Skip to content

Commit

Permalink
Fix warning/redirect/claim tx fee calculations
Browse files Browse the repository at this point in the history
Also make claim delay 5 blocks on regtest, and add missing TradeMessage
constructors to provide a default argument for the P2P message version,
for consistency with the other trade messages.
  • Loading branch information
stejbac committed Jul 17, 2024
1 parent 854bfd8 commit d184bed
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ public static boolean isProposal412Activated() {
// spike when opening arbitration.
private static final long DPT_MIN_TX_FEE_RATE = 10;

// The DPT weight (= 4 * size) without any outputs. This should actually be higher (426 wu, once the
// witness data is taken into account), but must be kept at this value for compatibility with the peer.
private static final long DPT_MIN_WEIGHT = 204;

private final DaoStateService daoStateService;
private final BurningManService burningManService;
private int currentChainHeight;
Expand Down Expand Up @@ -131,12 +135,20 @@ public int getBurningManSelectionHeight() {
public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
long inputAmount,
long tradeTxFee) {
return getReceivers(burningManSelectionHeight, inputAmount, tradeTxFee, isBugfix6699Activated());
return getReceivers(burningManSelectionHeight, inputAmount, tradeTxFee, DPT_MIN_WEIGHT, isBugfix6699Activated());
}

public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
long inputAmount,
long tradeTxFee,
boolean isBugfix6699Activated) {
return getReceivers(burningManSelectionHeight, inputAmount, tradeTxFee, DPT_MIN_WEIGHT, isBugfix6699Activated);
}

public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
long inputAmount,
long tradeTxFee,
long minTxWeight,
boolean isBugfix6699Activated) {
checkArgument(burningManSelectionHeight >= MIN_SNAPSHOT_HEIGHT, "Selection height must be >= " + MIN_SNAPSHOT_HEIGHT);
Collection<BurningManCandidate> burningManCandidates = burningManService.getActiveBurningManCandidates(burningManSelectionHeight);
Expand All @@ -157,13 +169,15 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,

if (burningManCandidates.isEmpty()) {
// If there are no compensation requests (e.g. at dev testing) we fall back to the legacy BM
long spendableAmount = getSpendableAmount(1, inputAmount, txFeePerVbyte);
return List.of(new Tuple2<>(spendableAmount, burningManService.getLegacyBurningManAddress(burningManSelectionHeight)));
long spendableAmount = getSpendableAmount(1, inputAmount, txFeePerVbyte, minTxWeight);
return spendableAmount > DPT_MIN_REMAINDER_TO_LEGACY_BM
? List.of(new Tuple2<>(spendableAmount, burningManService.getLegacyBurningManAddress(burningManSelectionHeight)))
: List.of();
}

long spendableAmount = getSpendableAmount(burningManCandidates.size(), inputAmount, txFeePerVbyte);
// We only use outputs > 1000 sat or at least 2 times the cost for the output (32 bytes).
// If we remove outputs it will be spent as miner fee.
long spendableAmount = getSpendableAmount(burningManCandidates.size(), inputAmount, txFeePerVbyte, minTxWeight);
// We only use outputs >= 1000 sat or at least 2 times the cost for the output (32 bytes).
// If we remove outputs it will be distributed to the remaining receivers.
long minOutputAmount = Math.max(DPT_MIN_OUTPUT_AMOUNT, txFeePerVbyte * 32 * 2);
// Sanity check that max share of a non-legacy BM is 20% over MAX_BURN_SHARE (taking into account potential increase due adjustment)
long maxOutputAmount = Math.round(spendableAmount * (BurningManService.MAX_BURN_SHARE * 1.2));
Expand All @@ -178,6 +192,9 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
})
.sum();

// FIXME: The small outputs should be filtered out before adjustment, not afterwards. Otherwise, outputs of
// amount just under 1000 sats or 64 * fee-rate could get erroneously included and lead to significant
// underpaying of the DPT (by perhaps around 5-10% per erroneously included output).
List<Tuple2<Long, String>> receivers = burningManCandidates.stream()
.filter(candidate -> candidate.getReceiverAddress(isBugfix6699Activated).isPresent())
.map(candidate -> {
Expand All @@ -191,6 +208,8 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
.thenComparing(tuple -> tuple.second))
.collect(Collectors.toList());
long totalOutputValue = receivers.stream().mapToLong(e -> e.first).sum();
// FIXME: The balance given to the LBM burning man needs to take into account the tx size increase due to
// the extra output, to avoid underpaying the tx fee.
if (totalOutputValue < spendableAmount) {
long available = spendableAmount - totalOutputValue;
// If the available is larger than DPT_MIN_REMAINDER_TO_LEGACY_BM we send it to legacy BM
Expand All @@ -202,14 +221,19 @@ public List<Tuple2<Long, String>> getReceivers(int burningManSelectionHeight,
return receivers;
}

private static long getSpendableAmount(int numOutputs, long inputAmount, long txFeePerVbyte) {
// TODO: For the v5 trade protocol, should we compute a more precise fee estimate taking into account the individual
// receiver output script types? (P2SH costs 32 bytes per output, P2WPKH costs 31, etc.) This has the advantage of
// avoiding the slight overestimate at present (32 vs 31) and could allow future support for P2TR receiver outputs,
// which cost 43 bytes each. (Note that bitcoinj has already added partial taproot support upstream, and recognises
// P2TR addresses & ScriptPubKey types.)
private static long getSpendableAmount(int numOutputs, long inputAmount, long txFeePerVbyte, long minTxWeight) {
// Output size: 32 bytes
// Tx size without outputs: 51 bytes
int txSize = 51 + numOutputs * 32; // Min value: txSize=83
long minerFee = txFeePerVbyte * txSize; // Min value: minerFee=830
long txWeight = minTxWeight + numOutputs * 128L; // Min value: txWeight=332 (for DPT with 1 output)
long minerFee = (txFeePerVbyte * txWeight + 3) / 4; // Min value: minerFee=830
// We need to make sure we have at least 1000 sat as defined in TradeWalletService
minerFee = Math.max(TradeWalletService.MIN_DELAYED_PAYOUT_TX_FEE.value, minerFee);
return inputAmount - minerFee;
return Math.max(inputAmount - minerFee, 0);
}

private static int getSnapshotHeight(int genesisHeight, int height, int grid) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import bisq.network.p2p.DirectMessage;
import bisq.network.p2p.NodeAddress;

import bisq.common.app.Version;
import bisq.common.util.Utilities;

import protobuf.NetworkEnvelope;
Expand All @@ -41,15 +42,37 @@ public class PreparedTxBuyerSignaturesMessage extends TradeMessage implements Di
private final byte[] buyersRedirectTxBuyerSignature;
private final byte[] sellersRedirectTxBuyerSignature;

public PreparedTxBuyerSignaturesMessage(int messageVersion,
String tradeId,
public PreparedTxBuyerSignaturesMessage(String tradeId,
String uid,
NodeAddress senderNodeAddress,
byte[] depositTxWithBuyerWitnesses,
byte[] buyersWarningTxBuyerSignature,
byte[] sellersWarningTxBuyerSignature,
byte[] buyersRedirectTxBuyerSignature,
byte[] sellersRedirectTxBuyerSignature) {
this(Version.getP2PMessageVersion(), tradeId, uid,
senderNodeAddress,
depositTxWithBuyerWitnesses,
buyersWarningTxBuyerSignature,
sellersWarningTxBuyerSignature,
buyersRedirectTxBuyerSignature,
sellersRedirectTxBuyerSignature);
}


///////////////////////////////////////////////////////////////////////////////////////////
// PROTO BUFFER
///////////////////////////////////////////////////////////////////////////////////////////

private PreparedTxBuyerSignaturesMessage(int messageVersion,
String tradeId,
String uid,
NodeAddress senderNodeAddress,
byte[] depositTxWithBuyerWitnesses,
byte[] buyersWarningTxBuyerSignature,
byte[] sellersWarningTxBuyerSignature,
byte[] buyersRedirectTxBuyerSignature,
byte[] sellersRedirectTxBuyerSignature) {
super(messageVersion, tradeId, uid);
this.senderNodeAddress = senderNodeAddress;
this.depositTxWithBuyerWitnesses = depositTxWithBuyerWitnesses;
Expand All @@ -59,11 +82,6 @@ public PreparedTxBuyerSignaturesMessage(int messageVersion,
this.sellersRedirectTxBuyerSignature = sellersRedirectTxBuyerSignature;
}


///////////////////////////////////////////////////////////////////////////////////////////
// PROTO BUFFER
///////////////////////////////////////////////////////////////////////////////////////////

@Override
public NetworkEnvelope toProtoNetworkEnvelope() {
protobuf.PreparedTxBuyerSignaturesMessage.Builder builder = protobuf.PreparedTxBuyerSignaturesMessage.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import bisq.network.p2p.DirectMessage;
import bisq.network.p2p.NodeAddress;

import bisq.common.app.Version;
import bisq.common.util.Utilities;

import protobuf.NetworkEnvelope;
Expand All @@ -40,14 +41,34 @@ public class PreparedTxBuyerSignaturesRequest extends TradeMessage implements Di
private final byte[] buyersRedirectTxSellerSignature;
private final byte[] sellersRedirectTxSellerSignature;

public PreparedTxBuyerSignaturesRequest(int messageVersion,
String tradeId,
public PreparedTxBuyerSignaturesRequest(String tradeId,
String uid,
NodeAddress senderNodeAddress,
byte[] buyersWarningTxSellerSignature,
byte[] sellersWarningTxSellerSignature,
byte[] buyersRedirectTxSellerSignature,
byte[] sellersRedirectTxSellerSignature) {
this(Version.getP2PMessageVersion(), tradeId, uid,
senderNodeAddress,
buyersWarningTxSellerSignature,
sellersWarningTxSellerSignature,
buyersRedirectTxSellerSignature,
sellersRedirectTxSellerSignature);
}


///////////////////////////////////////////////////////////////////////////////////////////
// PROTO BUFFER
///////////////////////////////////////////////////////////////////////////////////////////

private PreparedTxBuyerSignaturesRequest(int messageVersion,
String tradeId,
String uid,
NodeAddress senderNodeAddress,
byte[] buyersWarningTxSellerSignature,
byte[] sellersWarningTxSellerSignature,
byte[] buyersRedirectTxSellerSignature,
byte[] sellersRedirectTxSellerSignature) {
super(messageVersion, tradeId, uid);
this.senderNodeAddress = senderNodeAddress;
this.buyersWarningTxSellerSignature = buyersWarningTxSellerSignature;
Expand All @@ -56,11 +77,6 @@ public PreparedTxBuyerSignaturesRequest(int messageVersion,
this.sellersRedirectTxSellerSignature = sellersRedirectTxSellerSignature;
}


///////////////////////////////////////////////////////////////////////////////////////////
// PROTO BUFFER
///////////////////////////////////////////////////////////////////////////////////////////

@Override
public NetworkEnvelope toProtoNetworkEnvelope() {
protobuf.PreparedTxBuyerSignaturesRequest.Builder builder = protobuf.PreparedTxBuyerSignaturesRequest.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,34 @@

package bisq.core.trade.protocol.bisq_v5.model;

import bisq.common.config.Config;

public class StagedPayoutTxParameters {
// 10 days
public static final long CLAIM_DELAY = 144 * 10;
private static final long CLAIM_DELAY = 144 * 10;
//todo find what is min value (we filter dust values in the wallet, so better not go that low)
public static final long WARNING_TX_FEE_BUMP_OUTPUT_VALUE = 2000;
public static final long REDIRECT_TX_FEE_BUMP_OUTPUT_VALUE = 2000;

// todo find out size
private static final long WARNING_TX_EXPECTED_SIZE = 1000; // todo find out size
private static final long CLAIM_TX_EXPECTED_SIZE = 1000; // todo find out size
// Min. fee rate for DPT. If fee rate used at take offer time was higher we use that.
// We prefer a rather high fee rate to not risk that the DPT gets stuck if required fee rate would
private static final long WARNING_TX_EXPECTED_WEIGHT = 722; // 125 tx bytes, 220-222 witness bytes
private static final long CLAIM_TX_EXPECTED_WEIGHT = 520; // 82 tx bytes, 191-192 witness bytes
public static final long REDIRECT_TX_MIN_WEIGHT = 595; // 82 tx bytes, 265-267 witness bytes

// Min. fee rate for staged payout txs. If fee rate used at take offer time was higher we use that.
// We prefer a rather high fee rate to not risk that the tx gets stuck if required fee rate would
// spike when opening arbitration.
private static final long MIN_TX_FEE_RATE = 10;

public static long getClaimDelay() {
return Config.baseCurrencyNetwork().isRegtest() ? 5 : CLAIM_DELAY;
}

public static long getWarningTxMiningFee(long depositTxFeeRate) {
return getFeePerVByte(depositTxFeeRate) * StagedPayoutTxParameters.WARNING_TX_EXPECTED_SIZE;
return (getFeePerVByte(depositTxFeeRate) * StagedPayoutTxParameters.WARNING_TX_EXPECTED_WEIGHT + 3) / 4;
}

public static long getClaimTxMiningFee(long depositTxFeeRate) {
return getFeePerVByte(depositTxFeeRate) * StagedPayoutTxParameters.CLAIM_TX_EXPECTED_SIZE;
public static long getClaimTxMiningFee(long txFeePerVByte) {
return (txFeePerVByte * StagedPayoutTxParameters.CLAIM_TX_EXPECTED_WEIGHT + 3) / 4;
}

private static long getFeePerVByte(long depositTxFeeRate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ protected void run() {
"Different warningTx output amounts. Ours: {}; Peer's: {}", warningTxOutput.getValue().value, inputAmount);

long depositTxFee = trade.getTradeTxFeeAsLong(); // Used for fee rate calculation inside getDelayedPayoutTxReceiverService
long inputAmountMinusFeeForFeeBumpOutput = inputAmount - 32 * depositTxFee;
long inputAmountMinusFeeBumpAmount = inputAmount - StagedPayoutTxParameters.REDIRECT_TX_FEE_BUMP_OUTPUT_VALUE;
int selectionHeight = processModel.getBurningManSelectionHeight();
List<Tuple2<Long, String>> burningMen = processModel.getDelayedPayoutTxReceiverService().getReceivers(
selectionHeight,
inputAmountMinusFeeForFeeBumpOutput,
depositTxFee);
inputAmountMinusFeeBumpAmount,
depositTxFee,
StagedPayoutTxParameters.REDIRECT_TX_MIN_WEIGHT,
true);

log.info("Create redirectionTxs using selectionHeight {} and receivers {}", selectionHeight, burningMen);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected void run() {
long lockTime = trade.getLockTime();
byte[] buyerPubKey = amBuyer ? processModel.getMyMultiSigPubKey() : tradingPeer.getMultiSigPubKey();
byte[] sellerPubKey = amBuyer ? tradingPeer.getMultiSigPubKey() : processModel.getMyMultiSigPubKey();
long claimDelay = StagedPayoutTxParameters.CLAIM_DELAY; // FIXME: Make sure this is a low value off mainnet
long claimDelay = StagedPayoutTxParameters.getClaimDelay();
long miningFee = StagedPayoutTxParameters.getWarningTxMiningFee(trade.getDepositTxFeeRate());

// Create our warning tx.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected void run() {
boolean amBuyer = trade instanceof BuyerTrade;
byte[] buyerPubKey = amBuyer ? processModel.getMyMultiSigPubKey() : tradingPeer.getMultiSigPubKey();
byte[] sellerPubKey = amBuyer ? tradingPeer.getMultiSigPubKey() : processModel.getMyMultiSigPubKey();
long claimDelay = StagedPayoutTxParameters.CLAIM_DELAY; // FIXME: Make sure this is a low value off mainnet
long claimDelay = StagedPayoutTxParameters.getClaimDelay();

// Finalize our redirect tx.
TransactionOutput peersWarningTxOutput = tradingPeer.getWarningTx().getOutput(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ protected void run() {
TransactionOutput myWarningTxOutput = processModel.getWarningTx().getOutput(0);
AddressEntry addressEntry = processModel.getBtcWalletService().getOrCreateAddressEntry(tradeId, AddressEntry.Context.TRADE_PAYOUT);
Address payoutAddress = addressEntry.getAddress();
// long miningFee = StagedPayoutTxParameters.getClaimTxMiningFee(trade.getDepositTxFeeRate());
long miningFee = StagedPayoutTxParameters.getClaimTxMiningFee(feeService.getTxFeePerVbyte().value);
long claimDelay = StagedPayoutTxParameters.CLAIM_DELAY; // FIXME: Make sure this is a low value off mainnet
long claimDelay = StagedPayoutTxParameters.getClaimDelay();
byte[] myMultiSigPubKey = processModel.getMyMultiSigPubKey();
byte[] peersMultiSigPubKey = tradingPeer.getMultiSigPubKey();
DeterministicKey myMultiSigKeyPair = btcWalletService.getMultiSigKeyPair(tradeId, myMultiSigPubKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import bisq.network.p2p.NodeAddress;
import bisq.network.p2p.SendDirectMessageListener;

import bisq.common.app.Version;
import bisq.common.taskrunner.TaskRunner;

import java.util.UUID;
Expand All @@ -50,7 +49,7 @@ protected void run() {
byte[] buyersRedirectTxBuyerSignature = processModel.getRedirectTxBuyerSignature();
byte[] sellersRedirectTxBuyerSignature = processModel.getTradePeer().getRedirectTxBuyerSignature();

PreparedTxBuyerSignaturesMessage message = new PreparedTxBuyerSignaturesMessage(Version.getP2PMessageVersion(), // TODO: Add extra constructor
PreparedTxBuyerSignaturesMessage message = new PreparedTxBuyerSignaturesMessage(
processModel.getOfferId(),
UUID.randomUUID().toString(),
processModel.getMyNodeAddress(),
Expand Down
Loading

0 comments on commit d184bed

Please sign in to comment.