mirror of
https://github.com/bisq-network/bisq.git
synced 2024-11-19 18:03:12 +01:00
Fix wrong fee estimation for taker. Improve logs.
This commit is contained in:
parent
391844e41e
commit
43cc5598ca
@ -158,13 +158,14 @@ public class TradeWalletService {
|
||||
log.debug("fundingAddress " + fundingAddress.toString());
|
||||
log.debug("reservedForTradeAddress " + reservedForTradeAddress.toString());
|
||||
log.debug("changeAddress " + changeAddress.toString());
|
||||
log.debug("reservedFundsForOffer " + reservedFundsForOffer.toPlainString());
|
||||
log.info("reservedFundsForOffer " + reservedFundsForOffer.toPlainString());
|
||||
log.debug("useSavingsWallet " + useSavingsWallet);
|
||||
log.debug("tradingFee " + tradingFee.toPlainString());
|
||||
log.debug("txFee " + txFee.toPlainString());
|
||||
log.info("tradingFee " + tradingFee.toPlainString());
|
||||
log.info("txFee " + txFee.toPlainString());
|
||||
log.debug("feeReceiverAddresses " + feeReceiverAddresses);
|
||||
|
||||
Transaction tradingFeeTx = new Transaction(params);
|
||||
SendRequest sendRequest = null;
|
||||
try {
|
||||
tradingFeeTx.addOutput(tradingFee, Address.fromBase58(params, feeReceiverAddresses));
|
||||
// the reserved amount we need for the trade we send to our trade reservedForTradeAddress
|
||||
tradingFeeTx.addOutput(reservedFundsForOffer, reservedForTradeAddress);
|
||||
@ -172,7 +173,7 @@ public class TradeWalletService {
|
||||
// we allow spending of unconfirmed tx (double spend risk is low and usability would suffer if we need to
|
||||
// wait for 1 confirmation)
|
||||
// In case of double spend we will detect later in the trade process and use a ban score to penalize bad behaviour (not impl. yet)
|
||||
SendRequest sendRequest = SendRequest.forTx(tradingFeeTx);
|
||||
sendRequest = SendRequest.forTx(tradingFeeTx);
|
||||
sendRequest.shuffleOutputs = false;
|
||||
sendRequest.aesKey = aesKey;
|
||||
if (useSavingsWallet)
|
||||
@ -195,6 +196,15 @@ public class TradeWalletService {
|
||||
broadcastTx(tradingFeeTx, callback);
|
||||
|
||||
return tradingFeeTx;
|
||||
} catch (Throwable t) {
|
||||
if (wallet!=null && sendRequest != null && sendRequest.coinSelector != null)
|
||||
log.warn("Balance = {}; CoinSelector = {}",
|
||||
wallet.getBalance(sendRequest.coinSelector),
|
||||
sendRequest.coinSelector);
|
||||
|
||||
log.warn("createBtcTradingFeeTx failed: tradingFeeTx={}, txOutputs={}", tradingFeeTx.toString(), tradingFeeTx.getOutputs());
|
||||
throw t;
|
||||
}
|
||||
}
|
||||
|
||||
public Transaction estimateBtcTradingFeeTxSize(Address fundingAddress,
|
||||
@ -223,6 +233,7 @@ public class TradeWalletService {
|
||||
sendRequest.ensureMinRequiredFee = false;
|
||||
sendRequest.changeAddress = changeAddress;
|
||||
checkNotNull(wallet, "Wallet must not be null");
|
||||
log.info("estimateBtcTradingFeeTxSize");
|
||||
wallet.completeTx(sendRequest);
|
||||
return tradingFeeTx;
|
||||
}
|
||||
|
@ -193,8 +193,8 @@ class CreateOfferViewModel extends ActivatableWithDataModel<CreateOfferDataModel
|
||||
UserThread.runAfter(() -> {
|
||||
switch (BisqEnvironment.getBaseCurrencyNetwork().getCurrencyCode()) {
|
||||
case "BTC":
|
||||
amount.set("0.1");
|
||||
price.set("0.0001");
|
||||
amount.set("0.0001");
|
||||
price.set("14029");
|
||||
break;
|
||||
case "LTC":
|
||||
amount.set("50");
|
||||
|
@ -103,8 +103,11 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
Coin totalAvailableBalance;
|
||||
private Notification walletFundedNotification;
|
||||
Price tradePrice;
|
||||
private int feeTxSize = 320; // 260 kb is size of typical trade fee tx with 1 input but trade tx (deposit and payout) are larger so we adjust to 320
|
||||
private int feeTxSize = 320; // 260 kb is size of typical trade fee tx with 1 input but trade tx (deposit and payout)
|
||||
// are larger so we adjust to 320
|
||||
private int feeTxSizeEstimationRecursionCounter;
|
||||
private boolean freezeFee;
|
||||
private Coin txFeePerByteFromFeeService;
|
||||
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////////////////
|
||||
@ -187,12 +190,11 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
getBuyerSecurityDeposit() :
|
||||
getSellerSecurityDeposit();
|
||||
|
||||
// We request to get the actual estimated fee
|
||||
requestTxFee();
|
||||
|
||||
|
||||
// Taker pays 2 times the tx fee because the mining fee might be different when maker created the offer
|
||||
// and reserved his funds, so that would not work well with dynamic fees.
|
||||
// Taker pays 3 times the tx fee (taker fee, deposit, payout) because the mining fee might be different when maker created the offer
|
||||
// and reserved his funds. Taker creates at least taker fee and deposit tx at nearly the same moment. Just the payout will
|
||||
// be later and still could lead to issues if the required fee changed a lot in the meantime. using RBF and/or
|
||||
// multiple batch-signed payout tx with different fees might be an option but RBF is not supported yet in BitcoinJ
|
||||
// and batched txs would add more complexity to the trade protocol.
|
||||
|
||||
// A typical trade fee tx has about 260 bytes (if one input). The trade txs has about 336-414 bytes.
|
||||
// We use 320 as a average value.
|
||||
@ -204,7 +206,22 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
|
||||
// Set the default values (in rare cases if the fee request was not done yet we get the hard coded default values)
|
||||
// But the "take offer" happens usually after that so we should have already the value from the estimation service.
|
||||
txFeeFromFeeService = feeService.getTxFee(feeTxSize);
|
||||
txFeePerByteFromFeeService = feeService.getTxFeePerByte();
|
||||
txFeeFromFeeService = getTxFeeBySize(feeTxSize);
|
||||
|
||||
// We request to get the actual estimated fee
|
||||
log.info("Start requestTxFee: txFeeFromFeeService={}", txFeeFromFeeService);
|
||||
feeService.requestFees(() -> {
|
||||
if (!freezeFee) {
|
||||
txFeePerByteFromFeeService = feeService.getTxFeePerByte();
|
||||
txFeeFromFeeService = getTxFeeBySize(feeTxSize);
|
||||
calculateTotalToPay();
|
||||
log.info("Completed requestTxFee: txFeeFromFeeService={}", txFeeFromFeeService);
|
||||
} else {
|
||||
log.warn("We received the tx fee respnse after we have shown the funding screen and ignore that " +
|
||||
"to avoid that the total funds to pay changes due cahnged tx fees.");
|
||||
}
|
||||
}, null);
|
||||
|
||||
calculateVolume();
|
||||
calculateTotalToPay();
|
||||
@ -246,11 +263,11 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
priceFeedService.setCurrencyCode(offer.getCurrencyCode());
|
||||
}
|
||||
|
||||
void requestTxFee() {
|
||||
feeService.requestFees(() -> {
|
||||
txFeeFromFeeService = feeService.getTxFee(feeTxSize);
|
||||
// We don't want that the fee gets updated anymore after we show the funding screen.
|
||||
void onShowPayFundsScreen() {
|
||||
estimateTxSize();
|
||||
freezeFee = true;
|
||||
calculateTotalToPay();
|
||||
}, null);
|
||||
}
|
||||
|
||||
void onTabSelected(boolean isSelected) {
|
||||
@ -307,20 +324,20 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
|
||||
// This works only if have already funds in the wallet
|
||||
public void estimateTxSize() {
|
||||
txFeeFromFeeService = feeService.getTxFee(feeTxSize);
|
||||
txFeeFromFeeService = getTxFeeBySize(feeTxSize);
|
||||
Address fundingAddress = btcWalletService.getOrCreateAddressEntry(AddressEntry.Context.AVAILABLE).getAddress();
|
||||
Address reservedForTradeAddress = btcWalletService.getOrCreateAddressEntry(offer.getId(), AddressEntry.Context.RESERVED_FOR_TRADE).getAddress();
|
||||
Address changeAddress = btcWalletService.getOrCreateAddressEntry(AddressEntry.Context.AVAILABLE).getAddress();
|
||||
|
||||
Coin reservedFundsForOffer = getSecurityDeposit();
|
||||
if (!isBuyOffer())
|
||||
Coin reservedFundsForOffer = getSecurityDeposit().add(txFeeFromFeeService).add(txFeeFromFeeService);
|
||||
if (isBuyOffer())
|
||||
reservedFundsForOffer = reservedFundsForOffer.add(amount.get());
|
||||
|
||||
checkNotNull(user.getAcceptedArbitrators(), "user.getAcceptedArbitrators() must not be null");
|
||||
checkArgument(!user.getAcceptedArbitrators().isEmpty(), "user.getAcceptedArbitrators() must not be empty");
|
||||
String dummyArbitratorAddress = user.getAcceptedArbitrators().get(0).getBtcAddress();
|
||||
try {
|
||||
log.info("We create a dummy tx to see if our estimated size is in the accepted range. feeTxSize={}," +
|
||||
log.debug("We create a dummy tx to see if our estimated size is in the accepted range. feeTxSize={}," +
|
||||
" txFee based on feeTxSize: {}, recommended txFee is {} sat/byte",
|
||||
feeTxSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
|
||||
Transaction tradeFeeTx = tradeWalletService.estimateBtcTradingFeeTxSize(
|
||||
@ -335,28 +352,41 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
|
||||
final int txSize = tradeFeeTx.bitcoinSerialize().length;
|
||||
// use feeTxSizeEstimationRecursionCounter to avoid risk for endless loop
|
||||
// We use the tx size for the trade fee tx as target for the fees.
|
||||
// The deposit and payout txs are determined +/- 1 output but the trade fee tx can have either 1 or many inputs
|
||||
// so we need to make sure the trade fee tx gets the correct fee to not get stuck.
|
||||
// We use a 20% tolerance frm out default 320 byte size (typical for deposit and payout) and only if we get a
|
||||
// larger size we increase the fee. Worst case is that we overpay for the other follow up txs, but better than
|
||||
// use a too low fee and get stuck.
|
||||
if (txSize > feeTxSize * 1.2 && feeTxSizeEstimationRecursionCounter < 10) {
|
||||
feeTxSizeEstimationRecursionCounter++;
|
||||
log.info("txSize is {} bytes but feeTxSize used for txFee calculation was {} bytes. We try again with an " +
|
||||
"adjusted txFee to reach the target tx fee.", txSize, feeTxSize);
|
||||
feeTxSize = txSize;
|
||||
txFeeFromFeeService = feeService.getTxFee(feeTxSize);
|
||||
txFeeFromFeeService = getTxFeeBySize(feeTxSize);
|
||||
|
||||
// lets try again with the adjusted txSize and fee.
|
||||
estimateTxSize();
|
||||
} else {
|
||||
log.info("feeTxSize {} bytes", feeTxSize);
|
||||
log.info("txFee based on estimated size: {}, recommended txFee is {} sat/byte",
|
||||
txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
|
||||
// We are done with estimation iterations
|
||||
if (feeTxSizeEstimationRecursionCounter < 10)
|
||||
log.info("txFee based on estimated size of {} bytes. Actual tx size = {} bytes. " +
|
||||
"TxFee is {} BTC ({} sat/byte)",
|
||||
feeTxSize, txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
|
||||
else
|
||||
log.warn("We could not estimate the fee as the feeTxSizeEstimationRecursionCounter exceeded our limit of 10 recursions. txFee based on estimated size of {} bytes. Actual tx size = {} bytes. " +
|
||||
"TxFee is {} BTC ({} sat/byte)",
|
||||
feeTxSize, txSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
|
||||
}
|
||||
} catch (InsufficientMoneyException e) {
|
||||
// If we need to fund from an external wallet we can assume we only have 1 input (260 bytes).
|
||||
log.warn("We cannot do the fee estimation because there are not enough funds in the wallet. This is expected " +
|
||||
"if the user pays from an external wallet. In that case we use an estimated tx size of 260 bytes.");
|
||||
feeTxSize = 260;
|
||||
txFeeFromFeeService = feeService.getTxFee(feeTxSize);
|
||||
log.info("feeTxSize {} bytes", feeTxSize);
|
||||
log.info("txFee based on estimated size: {}, recommended txFee is {} sat/byte",
|
||||
txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
|
||||
log.info("We cannot do the fee estimation because there are not enough funds in the wallet. This is expected " +
|
||||
"if the user has not sufficient funds yet and need to fund from an external wallet. " +
|
||||
"In that case we use an estimated tx size of 320 bytes.");
|
||||
feeTxSize = 320;
|
||||
txFeeFromFeeService = getTxFeeBySize(feeTxSize);
|
||||
log.info("txFee based on estimated size of {} bytes. TxFee is {} BTC ({} sat/byte)",
|
||||
feeTxSize, txFeeFromFeeService.toFriendlyString(), feeService.getTxFeePerByte());
|
||||
}
|
||||
}
|
||||
|
||||
@ -548,6 +578,10 @@ class TakeOfferDataModel extends ActivatableDataModel {
|
||||
btcWalletService.resetAddressEntriesForOpenOffer(offer.getId());
|
||||
}
|
||||
|
||||
private Coin getTxFeeBySize(int sizeInBytes) {
|
||||
return txFeePerByteFromFeeService.multiply(sizeInBytes);
|
||||
}
|
||||
|
||||
/* private void setFeeFromFundingTx(Coin fee) {
|
||||
feeFromFundingTx = fee;
|
||||
isFeeFromFundingTxSufficient.set(feeFromFundingTx.compareTo(FeePolicy.getMinRequiredFeeForFundingTx()) >= 0);
|
||||
|
@ -216,8 +216,7 @@ class TakeOfferViewModel extends ActivatableWithDataModel<TakeOfferDataModel> im
|
||||
}
|
||||
|
||||
public void onShowPayFundsScreen() {
|
||||
dataModel.estimateTxSize();
|
||||
dataModel.requestTxFee();
|
||||
dataModel.onShowPayFundsScreen();
|
||||
showPayFundsScreenDisplayed.set(true);
|
||||
updateSpinnerInfo();
|
||||
}
|
||||
|
@ -611,6 +611,7 @@ public class PeerManager implements ConnectionListener, PersistedDataHost {
|
||||
// Delivers the live peers from the last 30 min (MAX_AGE_LIVE_PEERS)
|
||||
// We include older peers to avoid risks for network partitioning
|
||||
public Set<Peer> getLivePeers(NodeAddress excludedNodeAddress) {
|
||||
int oldNumLatestLivePeers = latestLivePeers.size();
|
||||
Set<Peer> currentLivePeers = new HashSet<>(getConnectedReportedPeers().stream()
|
||||
.filter(e -> !isSeedNode(e))
|
||||
.filter(e -> !e.getNodeAddress().equals(excludedNodeAddress))
|
||||
@ -620,6 +621,7 @@ public class PeerManager implements ConnectionListener, PersistedDataHost {
|
||||
latestLivePeers = latestLivePeers.stream()
|
||||
.filter(peer -> peer.getDate().getTime() > maxAge)
|
||||
.collect(Collectors.toSet());
|
||||
if (oldNumLatestLivePeers != latestLivePeers.size())
|
||||
log.info("Num of latestLivePeers={}, latestLivePeers={}", latestLivePeers.size(), latestLivePeers);
|
||||
return latestLivePeers;
|
||||
}
|
||||
|
@ -210,7 +210,7 @@ public class PeerExchangeManager implements MessageListener, ConnectionListener,
|
||||
|
||||
@Override
|
||||
public void onFault(String errorMessage, @Nullable Connection connection) {
|
||||
log.info("PeerExchangeHandshake of outbound connection failed.\n\terrorMessage={}\n\t" +
|
||||
log.debug("PeerExchangeHandshake of outbound connection failed.\n\terrorMessage={}\n\t" +
|
||||
"nodeAddress={}", errorMessage, nodeAddress);
|
||||
|
||||
peerManager.handleConnectionFault(nodeAddress);
|
||||
|
Loading…
Reference in New Issue
Block a user