From dfddc9009b786cbf995eacac4a178946f931b84f Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 15 Oct 2019 13:35:59 -0500 Subject: [PATCH 01/21] Apply rule to not allow BSQ outputs after BTC output for regular txs --- .../core/dao/node/parser/TxOutputParser.java | 67 ++++++++++++++++++- .../bisq/core/dao/node/parser/TxParser.java | 24 +++++-- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index e520e191015..1faf0070523 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -17,6 +17,7 @@ package bisq.core.dao.node.parser; +import bisq.core.app.BisqEnvironment; import bisq.core.dao.governance.bond.BondConsensus; import bisq.core.dao.state.DaoStateService; import bisq.core.dao.state.model.blockchain.OpReturnType; @@ -41,8 +42,46 @@ */ @Slf4j public class TxOutputParser { - private final DaoStateService daoStateService; + // With block 602500 (about 2 weeks after v1.2.0 release) we enforce a new rule which represents a + // hard fork. Not updated nodes would see an out of sync dao state hash if a relevant transaction would + // happen again. + // Further (highly unlikely) consequences could be: + // If the BSQ output would be sent to a BSQ address the old client would accept that even it is + // invalid according to the new rules. But sending such a output would require a manually crafted tx + // (not possible in the UI). Worst case a not updated user would buy invalid BSQ but that is not possible as we + // enforce update to 1.2.0 for trading a few days after release as that release introduced the new trade protocol + // and protection tool. Only of both both traders would have deactivated filter messages they could trade. + + // Problem description: + // We did not apply the check to not allow BSQ outputs after we had detected a BTC output. + // The supported BSQ transactions did not support such cases anyway but we missed an edge case: + // A trade fee tx in case when the BTC input matches exactly the BTC output + // (or BTC change was <= the miner fee) and the BSQ fee was > the miner fee. Then we + // create a change output after the BTC output (using an address from the BTC wallet) and as + // available BSQ was >= as spent BSQ it was considered a valid BSQ output. + // There have been observed 5 such transactions where 4 got spent later to a BTC address and by that burned + // the pending BSQ (spending amount was higher than sending amount). One was still unspent. + // The BSQ was sitting in the BTC wallet so not even visible as BSQ to the user. + // If the user would have crafted a custom BSQ tx he could have avoided that the full trade fee was burned. + + // Not an universal rule: + // We cannot enforce the rule that no BSQ output is permitted to all possible transactions because there can be cases + // where we need to permit this case. + // For instance in case we confiscate a lockupTx we have usually 2 BSQ outputs: The first one is the bond which + // should be confiscated and the second one is the BSQ change output. + // At confiscating we set the first to TxOutputType.BTC_OUTPUT but we do not want to confiscate + // the second BSQ change output as well. So we do not apply the rule that no BSQ is allowed once a BTC output is + // found. Theoretically other transactions could be confiscated as well and all BSQ tx which allow > 1 BSQ outputs + // would have the same issue as well if the first output gets confiscated. + // We also don't enforce the rule for irregular or invalid txs which are usually set and detected at the end of + // the tx parsing which is done in the TxParser. Blind vote and LockupTx with invalid OpReturn would be such cases + // where we don't want to invalidate the change output (See comments in TxParser). + + private static int ACTIVATE_HARD_FORK_1_HEIGHT_MAINNET = 602500; + private static int ACTIVATE_HARD_FORK_1_HEIGHT_TESTNET = 1583054; + private static int ACTIVATE_HARD_FORK_1_HEIGHT_REGTEST = 1; + private final DaoStateService daoStateService; // Setters @Getter @Setter @@ -70,6 +109,7 @@ public class TxOutputParser { // Private private int lockTime; private final List utxoCandidates = new ArrayList<>(); + private boolean prohibitMoreBsqOutputs = false; /////////////////////////////////////////////////////////////////////////////////////////// @@ -119,7 +159,11 @@ void processTxOutput(TempTxOutput tempTxOutput) { // In case we have the opReturn for a burn fee tx all outputs after 1st output are considered BTC handleBtcOutput(tempTxOutput, index); } else if (availableInputValue > 0 && availableInputValue >= txOutputValue) { - handleBsqOutput(tempTxOutput, index, txOutputValue); + if (tempTxOutput.getBlockHeight() >= getActivateHardFork1Height() && prohibitMoreBsqOutputs) { + handleBtcOutput(tempTxOutput, index); + } else { + handleBsqOutput(tempTxOutput, index, txOutputValue); + } } else { handleBtcOutput(tempTxOutput, index); } @@ -127,6 +171,9 @@ void processTxOutput(TempTxOutput tempTxOutput) { log.warn("TxOutput {} is confiscated ", tempTxOutput.getKey()); // We only burn that output availableInputValue -= tempTxOutput.getValue(); + + // We must not set prohibitMoreBsqOutputs at confiscation transactions as optional + // BSQ change output (output 2) must not be confiscated. tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); } } @@ -139,6 +186,7 @@ void commitUTXOCandidates() { * This sets all outputs to BTC_OUTPUT and doesn't add any txOutputs to the unspentTxOutput map in daoStateService */ void invalidateUTXOCandidates() { + // We do not need to apply prohibitMoreBsqOutputs as all spendable outputs are set to BTC_OUTPUT anyway. utxoCandidates.forEach(output -> output.setTxOutputType(TxOutputType.BTC_OUTPUT)); } @@ -171,6 +219,9 @@ private void handleUnlockBondTx(TempTxOutput txOutput) { utxoCandidates.add(txOutput); bsqOutputFound = true; + + // We do not permit more BSQ outputs after the unlock txo as we don't expect additional BSQ outputs. + prohibitMoreBsqOutputs = true; } private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { @@ -228,11 +279,23 @@ private void handleBtcOutput(TempTxOutput txOutput, int index) { (optionalOpReturnType.get() == OpReturnType.COMPENSATION_REQUEST || optionalOpReturnType.get() == OpReturnType.REIMBURSEMENT_REQUEST)) { optionalIssuanceCandidate = Optional.of(txOutput); + + // We do not permit more BSQ outputs after the issuance candidate. + prohibitMoreBsqOutputs = true; } else { txOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + + // For regular transactions we don't permit BSQ outputs after a BTC output was detected. + prohibitMoreBsqOutputs = true; } } + private int getActivateHardFork1Height() { + return BisqEnvironment.getBaseCurrencyNetwork().isMainnet() ? ACTIVATE_HARD_FORK_1_HEIGHT_MAINNET : + BisqEnvironment.getBaseCurrencyNetwork().isTestnet() ? ACTIVATE_HARD_FORK_1_HEIGHT_TESTNET : + ACTIVATE_HARD_FORK_1_HEIGHT_REGTEST; + } + /////////////////////////////////////////////////////////////////////////////////////////// // Static diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxParser.java index 42be945a632..1b518de77d1 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxParser.java @@ -217,19 +217,33 @@ private void applyTxTypeAndTxOutputType(int blockHeight, TempTx tempTx, long bsq // We need to check if any tempTxOutput is available and if so and the OpReturn data is invalid we // set the output to a BTC output. We must not use `if else` cases here! if (opReturnType != OpReturnType.COMPENSATION_REQUEST && opReturnType != OpReturnType.REIMBURSEMENT_REQUEST) { + // We applied already the check to not permit further BSQ outputs after the issuanceCandidate in the + // txOutputParser so we don't need to do any additional check here when we change to BTC_OUTPUT. txOutputParser.getOptionalIssuanceCandidate().ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); } if (opReturnType != OpReturnType.BLIND_VOTE) { - txOutputParser.getOptionalBlindVoteLockStakeOutput().ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); + txOutputParser.getOptionalBlindVoteLockStakeOutput().ifPresent(tempTxOutput -> { + // We cannot apply the rule to not allow BSQ outputs after a BTC output as the 2nd output is an + // optional BSQ change output and we don't want to burn that in case the opReturn is invalid. + tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + }); } if (opReturnType != OpReturnType.VOTE_REVEAL) { - txOutputParser.getOptionalVoteRevealUnlockStakeOutput().ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); + txOutputParser.getOptionalVoteRevealUnlockStakeOutput().ifPresent(tempTxOutput -> { + // We do not apply the rule to not allow BSQ outputs after a BTC output here because we expect only + // one BSQ output anyway. + tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + }); } if (opReturnType != OpReturnType.LOCKUP) { - txOutputParser.getOptionalLockupOutput().ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); + txOutputParser.getOptionalLockupOutput().ifPresent(tempTxOutput -> { + // We cannot apply the rule to not allow BSQ outputs after a BTC output as the 2nd output is an + // optional BSQ change output and we don't want to burn that in case the opReturn is invalid. + tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + }); } } @@ -259,12 +273,14 @@ private void processIssuance(int blockHeight, TempTx tempTx, long bsqFee) { } } else { // This could be a valid compensation request that failed to be included in a block during the - // correct phase due to no fault of the user. Better not burn the change as long as the BSQ inputs + // correct phase due to no fault of the user. We must not burn the change as long as the BSQ inputs // cover the value of the outputs. // We tolerate such an incorrect tx and do not burn the BSQ tempTx.setTxType(TxType.IRREGULAR); // Make sure the optionalIssuanceCandidate is set to BTC + // We applied already the check to not permit further BSQ outputs after the issuanceCandidate in the + // txOutputParser so we don't need to do any additional check here when we change to BTC_OUTPUT. optionalIssuanceCandidate.ifPresent(tempTxOutput -> tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT)); // Empty Optional case is a possible valid case where a random tx matches our opReturn rules but it is not a // valid BSQ tx. From 6ec853ecfe6fa785c24c066d37ed04d8c6845cdc Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Tue, 15 Oct 2019 21:30:29 -0500 Subject: [PATCH 02/21] Enforce exactly 1 BSQ output for vote reveal tx --- .../core/dao/node/parser/TxOutputParser.java | 85 +++++++++++-------- 1 file changed, 48 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 1faf0070523..6fe4d393a4a 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -39,45 +39,53 @@ /** * Checks if an output is a BSQ output and apply state change. + * + * With block 602500 (about 4 weeks after v1.2.0 release) we enforce a new rule which represents a + * hard fork. Not updated nodes would see an out of sync dao state hash if a relevant transaction would + * happen again. + * Further (highly unlikely) consequences could be: + * If the BSQ output would be sent to a BSQ address the old client would accept that even it is + * invalid according to the new rules. But sending such a output would require a manually crafted tx + * (not possible in the UI). Worst case a not updated user would buy invalid BSQ but that is not possible as we + * enforce update to 1.2.0 for trading a few days after release as that release introduced the new trade protocol + * and protection tool. Only of both both traders would have deactivated filter messages they could trade. + * + * Problem description: + * We did not apply the check to not allow BSQ outputs after we had detected a BTC output. + * The supported BSQ transactions did not support such cases anyway but we missed an edge case: + * A trade fee tx in case when the BTC input matches exactly the BTC output + * (or BTC change was <= the miner fee) and the BSQ fee was > the miner fee. Then we + * create a change output after the BTC output (using an address from the BTC wallet) and as + * available BSQ was >= as spent BSQ it was considered a valid BSQ output. + * There have been observed 5 such transactions where 4 got spent later to a BTC address and by that burned + * the pending BSQ (spending amount was higher than sending amount). One was still unspent. + * The BSQ was sitting in the BTC wallet so not even visible as BSQ to the user. + * If the user would have crafted a custom BSQ tx he could have avoided that the full trade fee was burned. + * + * Not an universal rule: + * We cannot enforce the rule that no BSQ output is permitted to all possible transactions because there can be cases + * where we need to permit this case. + * For instance in case we confiscate a lockupTx we have usually 2 BSQ outputs: The first one is the bond which + * should be confiscated and the second one is the BSQ change output. + * At confiscating we set the first to TxOutputType.BTC_OUTPUT but we do not want to confiscate + * the second BSQ change output as well. So we do not apply the rule that no BSQ is allowed once a BTC output is + * found. Theoretically other transactions could be confiscated as well and all BSQ tx which allow > 1 BSQ outputs + * would have the same issue as well if the first output gets confiscated. + * We also don't enforce the rule for irregular or invalid txs which are usually set and detected at the end of + * the tx parsing which is done in the TxParser. Blind vote and LockupTx with invalid OpReturn would be such cases + * where we don't want to invalidate the change output (See comments in TxParser). + * + * Most transactions created in Bisq (Proposal, blind vote and lockup,...) have only 1 or 2 BSQ + * outputs but we do not enforce a limit of max. 2 transactions in the parser. + * We leave for now that flexibility but it should not be considered as a rule. We might strengthen + * it any time if we find a reason for that (e.g. attack risk) and add checks that no more + * BSQ outputs are permitted for those txs. + * Some transactions like issuance, vote reveal and unlock have exactly 1 BSQ output and that rule + * is enforced. */ @Slf4j -public class TxOutputParser { - // With block 602500 (about 2 weeks after v1.2.0 release) we enforce a new rule which represents a - // hard fork. Not updated nodes would see an out of sync dao state hash if a relevant transaction would - // happen again. - // Further (highly unlikely) consequences could be: - // If the BSQ output would be sent to a BSQ address the old client would accept that even it is - // invalid according to the new rules. But sending such a output would require a manually crafted tx - // (not possible in the UI). Worst case a not updated user would buy invalid BSQ but that is not possible as we - // enforce update to 1.2.0 for trading a few days after release as that release introduced the new trade protocol - // and protection tool. Only of both both traders would have deactivated filter messages they could trade. - - // Problem description: - // We did not apply the check to not allow BSQ outputs after we had detected a BTC output. - // The supported BSQ transactions did not support such cases anyway but we missed an edge case: - // A trade fee tx in case when the BTC input matches exactly the BTC output - // (or BTC change was <= the miner fee) and the BSQ fee was > the miner fee. Then we - // create a change output after the BTC output (using an address from the BTC wallet) and as - // available BSQ was >= as spent BSQ it was considered a valid BSQ output. - // There have been observed 5 such transactions where 4 got spent later to a BTC address and by that burned - // the pending BSQ (spending amount was higher than sending amount). One was still unspent. - // The BSQ was sitting in the BTC wallet so not even visible as BSQ to the user. - // If the user would have crafted a custom BSQ tx he could have avoided that the full trade fee was burned. - - // Not an universal rule: - // We cannot enforce the rule that no BSQ output is permitted to all possible transactions because there can be cases - // where we need to permit this case. - // For instance in case we confiscate a lockupTx we have usually 2 BSQ outputs: The first one is the bond which - // should be confiscated and the second one is the BSQ change output. - // At confiscating we set the first to TxOutputType.BTC_OUTPUT but we do not want to confiscate - // the second BSQ change output as well. So we do not apply the rule that no BSQ is allowed once a BTC output is - // found. Theoretically other transactions could be confiscated as well and all BSQ tx which allow > 1 BSQ outputs - // would have the same issue as well if the first output gets confiscated. - // We also don't enforce the rule for irregular or invalid txs which are usually set and detected at the end of - // the tx parsing which is done in the TxParser. Blind vote and LockupTx with invalid OpReturn would be such cases - // where we don't want to invalidate the change output (See comments in TxParser). - - private static int ACTIVATE_HARD_FORK_1_HEIGHT_MAINNET = 602500; +class TxOutputParser { + private static int ACTIVATE_HARD_FORK_1_HEIGHT_MAINNET = 605000; private static int ACTIVATE_HARD_FORK_1_HEIGHT_TESTNET = 1583054; private static int ACTIVATE_HARD_FORK_1_HEIGHT_REGTEST = 1; @@ -252,6 +260,9 @@ private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValu } else if (isFirstOutput && opReturnTypeCandidate == OpReturnType.VOTE_REVEAL) { txOutputType = TxOutputType.VOTE_REVEAL_UNLOCK_STAKE_OUTPUT; optionalVoteRevealUnlockStakeOutput = Optional.of(txOutput); + + // We do not permit more BSQ outputs after the VOTE_REVEAL_UNLOCK_STAKE_OUTPUT. + prohibitMoreBsqOutputs = true; } else if (isFirstOutput && opReturnTypeCandidate == OpReturnType.LOCKUP) { txOutputType = TxOutputType.LOCKUP_OUTPUT; From 2a2adf67d1c1ade766969145a6afdfd7e92c34e9 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 16:04:35 -0500 Subject: [PATCH 03/21] Fix missing balance and button state update --- .../desktop/main/dao/burnbsq/assetfee/AssetFeeView.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/assetfee/AssetFeeView.java b/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/assetfee/AssetFeeView.java index 5988bf6f28e..1a40b35fa49 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/assetfee/AssetFeeView.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/assetfee/AssetFeeView.java @@ -162,6 +162,7 @@ protected void activate() { assetService.getUpdateFlag().addListener(updateListener); bsqWalletService.addBsqBalanceListener(this); + onUpdateAvailableConfirmedBalance(bsqWalletService.getAvailableConfirmedBalance()); payFeeButton.setOnAction((event) -> { Coin listingFee = getListingFee(); @@ -225,7 +226,8 @@ public void onUpdateBalances(Coin availableConfirmedBalance, Coin lockedForVotingBalance, Coin lockupBondsBalance, Coin unlockingBondsBalance) { - bsqValidator.setAvailableBalance(availableConfirmedBalance); + + onUpdateAvailableConfirmedBalance(availableConfirmedBalance); } @@ -248,6 +250,11 @@ private void createListeners() { updateListener = observable -> updateList(); } + private void onUpdateAvailableConfirmedBalance(Coin availableConfirmedBalance) { + bsqValidator.setAvailableBalance(availableConfirmedBalance); + updateButtonState(); + } + private long getDays() { return getListingFee().value / assetService.getFeePerDay().value; } From d80558a9543ca325bb94e671f946eda9b92697e3 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 16:05:56 -0500 Subject: [PATCH 04/21] Refactor isBtcOutputOfBurnFeeTx method and add comments and TODOs No functional change. --- .../core/dao/node/parser/TxOutputParser.java | 51 ++++++++++++++++--- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 6fe4d393a4a..dd7042b39b3 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -233,14 +233,49 @@ private void handleUnlockBondTx(TempTxOutput txOutput) { } private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { - // If we get a asset listing or proof of burn tx we have only 1 BSQ output and if the - // burned amount is larger than the miner fee we might have a BTC output for receiving the burned funds. - // If the burned funds are less than the miner fee a BTC input is used for miner fee and a BTC change output for - // the remaining funds. In any case only the first output is BSQ all the others are BTC. - return optionalOpReturnType.isPresent() && - (optionalOpReturnType.get() == OpReturnType.ASSET_LISTING_FEE || - optionalOpReturnType.get() == OpReturnType.PROOF_OF_BURN) && - tempTxOutput.getIndex() >= 1; + if (optionalOpReturnType.isPresent()) { + switch (optionalOpReturnType.get()) { + case UNDEFINED: + break; + case PROPOSAL: + // TODO + break; + case COMPENSATION_REQUEST: + break; + case REIMBURSEMENT_REQUEST: + break; + case BLIND_VOTE: + // TODO + break; + case VOTE_REVEAL: + break; + case LOCKUP: + break; + case ASSET_LISTING_FEE: + // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 + // structurally same transactions where only the BSQ fee is different and the asset listing fee is + // a user input when creating the asset listing, so it is not know to the parser. + // Instead we derive the asset listing fee from the parser. + + // Case 1: 10 BSQ asset listing fee + // In: 15 BSQ + // Out: BSQ change 5 BSQ -> valid BSQ + // Out: OpReturn + // Miner fee: 1000 sat (10 BSQ burned) + + + // Case 2: 15 BSQ asset listing fee + // In: 15 BSQ + // Out: burned BSQ change 5 BSQ -> BTC (5 BSQ burned) + // Out: OpReturn + // Miner fee: 1000 sat (10 BSQ burned) + return tempTxOutput.getIndex() >= 1; + case PROOF_OF_BURN: + // TODO + return tempTxOutput.getIndex() >= 1; + } + } + return false; } private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValue) { From e9d31e5f1ee4944c58dea0efcbed247d8dbf10e0 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 16:07:20 -0500 Subject: [PATCH 05/21] Handle asset listing fee in custom method We need to enforce a BSQ change output As this is just tx creation code it has no consequences for the hard fork. --- .../core/btc/wallet/BsqWalletService.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index 300771460f4..a9017c440e4 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -586,6 +586,68 @@ public Transaction getPreparedBurnFeeTx(Coin fee) throws InsufficientBsqExceptio return getPreparedBurnFeeTx(fee, false); } + public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws InsufficientBsqException { + daoKillSwitch.assertDaoIsNotDisabled(); + + // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 + // structurally same transactions where only the BSQ fee is different and the asset listing fee is + // a user input when creating the asset listing, so it is not know to the parser. + // Instead we derive the asset listing fee from the parser. + + // Case 1: 10 BSQ asset listing fee + // In: 15 BSQ + // Out: BSQ change 5 BSQ -> valid BSQ + // Out: OpReturn + // Miner fee: 1000 sat (10 BSQ burned) + + + // Case 2: 15 BSQ asset listing fee + // In: 15 BSQ + // Out: burned BSQ change 5 BSQ -> BTC (5 BSQ burned) + // Out: OpReturn + // Miner fee: 1000 sat (10 BSQ burned) + + Transaction tx = new Transaction(params); + // We look for inputs covering out BSQ fee we want to pay. + CoinSelection coinSelection = bsqCoinSelector.select(fee, wallet.calculateAllSpendCandidates()); + try { + Coin change = bsqCoinSelector.getChange(fee, coinSelection); + if (change.isZero() || Restrictions.isDust(change)) { + // If change is zero or below dust we increase required input amount to enforce a BSQ change output. + // All outputs after that are considered BTC and therefore would be burned BSQ if BSQ is left from what + // we use for miner fee. + + Coin minDustThreshold = Coin.valueOf(preferences.getIgnoreDustThreshold()); + Coin increasedRequiredInput = fee.add(minDustThreshold); + coinSelection = bsqCoinSelector.select(increasedRequiredInput, wallet.calculateAllSpendCandidates()); + change = bsqCoinSelector.getChange(fee, coinSelection); + + log.warn("We increased required input as change output was zero or dust: New change value={}", change); + checkArgument(coinSelection.valueGathered.compareTo(fee) > 0, + "This transaction require a change output of at least " + minDustThreshold.value / 100 + " BSQ (dust limit). " + + "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " + + "Intended asset listing fee=" + fee.value / 100 + " BSQ. " + + "Please reduce the asset listing fee to " + (coinSelection.valueGathered.value - minDustThreshold.value) / 100 + " BSQ."); + + checkArgument(!Restrictions.isDust(change), + "This transaction would create a dust output of " + change.value / 100 + " BSQ. " + + "It requires a change output of at least " + minDustThreshold.value / 100 + " BSQ (dust limit). " + + "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " + + "Intended asset listing fee=" + fee.value / 100 + " BSQ. " + + "Please reduce the asset listing fee to " + (coinSelection.valueGathered.value - minDustThreshold.value) / 100 + " BSQ."); + } + + coinSelection.gathered.forEach(tx::addInput); + tx.addOutput(change, getChangeAddress()); + + return tx; + + } catch (InsufficientMoneyException e) { + log.error("coinSelection.gathered={}", coinSelection.gathered); + throw new InsufficientBsqException(e.missing); + } + } + private Transaction getPreparedBurnFeeTx(Coin fee, boolean requireChangeOutput) throws InsufficientBsqException { daoKillSwitch.assertDaoIsNotDisabled(); final Transaction tx = new Transaction(params); From 7f44f2226f0eeabad85ba8e64f7b3e273f2b50df Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 16:07:39 -0500 Subject: [PATCH 06/21] Use getPreparedBurnFeeTxForAssetListing --- .../java/bisq/core/dao/governance/asset/AssetService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/governance/asset/AssetService.java b/core/src/main/java/bisq/core/dao/governance/asset/AssetService.java index 7b9390da6cd..c765697a704 100644 --- a/core/src/main/java/bisq/core/dao/governance/asset/AssetService.java +++ b/core/src/main/java/bisq/core/dao/governance/asset/AssetService.java @@ -322,11 +322,11 @@ public Transaction payFee(StatefulAsset statefulAsset, long listingFee) throws I checkArgument(listingFee % 100 == 0, "Fee must be a multiple of 1 BSQ (100 satoshi)."); try { // We create a prepared Bsq Tx for the listing fee. - final Transaction preparedBurnFeeTx = bsqWalletService.getPreparedBurnFeeTx(Coin.valueOf(listingFee)); + Transaction preparedBurnFeeTx = bsqWalletService.getPreparedBurnFeeTxForAssetListing(Coin.valueOf(listingFee)); byte[] hash = AssetConsensus.getHash(statefulAsset); byte[] opReturnData = AssetConsensus.getOpReturnData(hash); // We add the BTC inputs for the miner fee. - final Transaction txWithBtcFee = btcWalletService.completePreparedBurnBsqTx(preparedBurnFeeTx, opReturnData); + Transaction txWithBtcFee = btcWalletService.completePreparedBurnBsqTx(preparedBurnFeeTx, opReturnData); // We sign the BSQ inputs of the final tx. Transaction transaction = bsqWalletService.signTx(txWithBtcFee); log.info("Asset listing fee tx: " + transaction); From 31bfb4e7996495227591c4a2c40878529f975a3a Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 16:10:37 -0500 Subject: [PATCH 07/21] Update comments to not use dust output values --- .../java/bisq/core/btc/wallet/BsqWalletService.java | 10 +++++----- .../java/bisq/core/dao/node/parser/TxOutputParser.java | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index a9017c440e4..c56ac1f68cc 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -595,15 +595,15 @@ public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws Insuffic // Instead we derive the asset listing fee from the parser. // Case 1: 10 BSQ asset listing fee - // In: 15 BSQ - // Out: BSQ change 5 BSQ -> valid BSQ + // In: 17 BSQ + // Out: BSQ change 7 BSQ -> valid BSQ // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) - // Case 2: 15 BSQ asset listing fee - // In: 15 BSQ - // Out: burned BSQ change 5 BSQ -> BTC (5 BSQ burned) + // Case 2: 17 BSQ asset listing fee + // In: 17 BSQ + // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index dd7042b39b3..f0529478aa6 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -258,15 +258,15 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { // Instead we derive the asset listing fee from the parser. // Case 1: 10 BSQ asset listing fee - // In: 15 BSQ - // Out: BSQ change 5 BSQ -> valid BSQ + // In: 17 BSQ + // Out: BSQ change 7 BSQ -> valid BSQ // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) - // Case 2: 15 BSQ asset listing fee - // In: 15 BSQ - // Out: burned BSQ change 5 BSQ -> BTC (5 BSQ burned) + // Case 2: 17 BSQ asset listing fee + // In: 17 BSQ + // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) return tempTxOutput.getIndex() >= 1; From 4b19d7d9c137ee1eeeaa95ca80287e5ecedcdf78 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 17:03:47 -0500 Subject: [PATCH 08/21] Fix missing balance and button state update --- .../main/dao/burnbsq/proofofburn/ProofOfBurnView.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/proofofburn/ProofOfBurnView.java b/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/proofofburn/ProofOfBurnView.java index 7ac3097ce24..c3944e85ad6 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/proofofburn/ProofOfBurnView.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/burnbsq/proofofburn/ProofOfBurnView.java @@ -163,6 +163,7 @@ protected void activate() { proofOfBurnService.getUpdateFlag().addListener(updateListener); bsqWalletService.addBsqBalanceListener(this); + onUpdateAvailableConfirmedBalance(bsqWalletService.getAvailableConfirmedBalance()); burnButton.setOnAction((event) -> { Coin amount = getAmountFee(); @@ -221,7 +222,7 @@ public void onUpdateBalances(Coin availableConfirmedBalance, Coin lockedForVotingBalance, Coin lockupBondsBalance, Coin unlockingBondsBalance) { - bsqValidator.setAvailableBalance(availableConfirmedBalance); + onUpdateAvailableConfirmedBalance(availableConfirmedBalance); } @@ -253,6 +254,11 @@ private void createListeners() { updateListener = observable -> updateList(); } + private void onUpdateAvailableConfirmedBalance(Coin availableConfirmedBalance) { + bsqValidator.setAvailableBalance(availableConfirmedBalance); + updateButtonState(); + } + private void updateList() { myItemsObservableList.setAll(myProofOfBurnListService.getMyProofOfBurnList().stream() .map(myProofOfBurn -> new MyProofOfBurnListItem(myProofOfBurn, proofOfBurnService, bsqFormatter)) From 7699fbe0215b14175688394f6934098f14468ff2 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 17:05:30 -0500 Subject: [PATCH 09/21] Use same method for asset listing fee and proof of burn Use same method for asset listing fee and proof of burn as tx structure is same. Update comments to be more general. --- .../core/btc/wallet/BsqWalletService.java | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index c56ac1f68cc..ff88ec06dd8 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -586,22 +586,30 @@ public Transaction getPreparedBurnFeeTx(Coin fee) throws InsufficientBsqExceptio return getPreparedBurnFeeTx(fee, false); } + public Transaction getPreparedProofOfBurnTx(Coin fee) throws InsufficientBsqException { + return getPreparedBurnBsqFeeTx(fee); + } + public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws InsufficientBsqException { + return getPreparedBurnBsqFeeTx(fee); + } + + private Transaction getPreparedBurnBsqFeeTx(Coin fee) throws InsufficientBsqException { daoKillSwitch.assertDaoIsNotDisabled(); // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 - // structurally same transactions where only the BSQ fee is different and the asset listing fee is - // a user input when creating the asset listing, so it is not know to the parser. - // Instead we derive the asset listing fee from the parser. + // structurally same transactions where only the BSQ fee is different. In case of asset listing fee and proof of + // burn it is a user input, so it is not know to the parser, instead we derive the burned fee from the parser. + // In case of proposal fee we could derive it from the params. - // Case 1: 10 BSQ asset listing fee + // Case 1: 10 BSQ fee to burn // In: 17 BSQ // Out: BSQ change 7 BSQ -> valid BSQ // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) - // Case 2: 17 BSQ asset listing fee + // Case 2: 17 BSQ fee to burn // In: 17 BSQ // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) // Out: OpReturn @@ -623,18 +631,17 @@ public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws Insuffic change = bsqCoinSelector.getChange(fee, coinSelection); log.warn("We increased required input as change output was zero or dust: New change value={}", change); + String info = "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " + + "Intended fee to burn=" + fee.value / 100 + " BSQ. " + + "Please reduce the fee to burn to " + (coinSelection.valueGathered.value - minDustThreshold.value) / 100 + " BSQ."; checkArgument(coinSelection.valueGathered.compareTo(fee) > 0, "This transaction require a change output of at least " + minDustThreshold.value / 100 + " BSQ (dust limit). " + - "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " + - "Intended asset listing fee=" + fee.value / 100 + " BSQ. " + - "Please reduce the asset listing fee to " + (coinSelection.valueGathered.value - minDustThreshold.value) / 100 + " BSQ."); + info); checkArgument(!Restrictions.isDust(change), "This transaction would create a dust output of " + change.value / 100 + " BSQ. " + "It requires a change output of at least " + minDustThreshold.value / 100 + " BSQ (dust limit). " + - "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " + - "Intended asset listing fee=" + fee.value / 100 + " BSQ. " + - "Please reduce the asset listing fee to " + (coinSelection.valueGathered.value - minDustThreshold.value) / 100 + " BSQ."); + info); } coinSelection.gathered.forEach(tx::addInput); @@ -688,7 +695,10 @@ private void addInputsAndChangeOutputForTx(Transaction tx, "BSQ is needed for this transaction"); tx.addOutput(change, getChangeAddress()); } + + log.error(tx.toString()); } catch (InsufficientMoneyException e) { + log.error(tx.toString()); throw new InsufficientBsqException(e.missing); } } From a428caebf1acabbe3f2a05b145bf52e294ca04cd Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 17:05:39 -0500 Subject: [PATCH 10/21] Use getPreparedProofOfBurnTx --- .../core/dao/governance/proofofburn/ProofOfBurnService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/governance/proofofburn/ProofOfBurnService.java b/core/src/main/java/bisq/core/dao/governance/proofofburn/ProofOfBurnService.java index e347656a597..cca3185a507 100644 --- a/core/src/main/java/bisq/core/dao/governance/proofofburn/ProofOfBurnService.java +++ b/core/src/main/java/bisq/core/dao/governance/proofofburn/ProofOfBurnService.java @@ -134,11 +134,11 @@ public void onParseBlockCompleteAfterBatchProcessing(Block block) { public Transaction burn(String preImageAsString, long amount) throws InsufficientMoneyException, TxException { try { // We create a prepared Bsq Tx for the burn amount - final Transaction preparedBurnFeeTx = bsqWalletService.getPreparedBurnFeeTx(Coin.valueOf(amount)); + Transaction preparedBurnFeeTx = bsqWalletService.getPreparedProofOfBurnTx(Coin.valueOf(amount)); byte[] hash = getHashFromPreImage(preImageAsString); byte[] opReturnData = ProofOfBurnConsensus.getOpReturnData(hash); // We add the BTC inputs for the miner fee. - final Transaction txWithBtcFee = btcWalletService.completePreparedBurnBsqTx(preparedBurnFeeTx, opReturnData); + Transaction txWithBtcFee = btcWalletService.completePreparedBurnBsqTx(preparedBurnFeeTx, opReturnData); // We sign the BSQ inputs of the final tx. Transaction transaction = bsqWalletService.signTx(txWithBtcFee); log.info("Proof of burn tx: " + transaction); From 0a51f32656b82bd20774dabe3893428e127d6668 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 20:59:33 -0500 Subject: [PATCH 11/21] Require mandatory BSQ change output for proposal fee tx. We had in the doc stated that we require a mandatory BSQ change output but it was not enforced in the implementation, causing similar issues as in Asset listing and proof of burn txs. --- .../core/btc/wallet/BsqWalletService.java | 16 ++++++---- .../proposal/BaseProposalFactory.java | 5 ++-- .../core/dao/node/parser/TxOutputParser.java | 30 ++++++++++++------- .../placeoffer/tasks/CreateMakerFeeTx.java | 2 +- .../tasks/taker/CreateTakerFeeTx.java | 2 +- 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index ff88ec06dd8..be209684ea8 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -573,16 +573,21 @@ private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmoun /////////////////////////////////////////////////////////////////////////////////////////// - // Burn fee tx + // Burn fee txs /////////////////////////////////////////////////////////////////////////////////////////// // We create a tx with Bsq inputs for the fee and optional BSQ change output. // As the fee amount will be missing in the output those BSQ fees are burned. - public Transaction getPreparedProposalTx(Coin fee, boolean requireChangeOutput) throws InsufficientBsqException { - return getPreparedBurnFeeTx(fee, requireChangeOutput); + public Transaction getPreparedProposalTx(Coin fee) throws InsufficientBsqException { + return getPreparedBurnBsqFeeTx(fee); + } + + //todo + public Transaction getPreparedIssuanceTx(Coin fee) throws InsufficientBsqException { + return getPreparedBurnBsqFeeTx(fee); } - public Transaction getPreparedBurnFeeTx(Coin fee) throws InsufficientBsqException { + public Transaction getPreparedTradeFeeTx(Coin fee) throws InsufficientBsqException { return getPreparedBurnFeeTx(fee, false); } @@ -633,7 +638,7 @@ private Transaction getPreparedBurnBsqFeeTx(Coin fee) throws InsufficientBsqExce log.warn("We increased required input as change output was zero or dust: New change value={}", change); String info = "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " + "Intended fee to burn=" + fee.value / 100 + " BSQ. " + - "Please reduce the fee to burn to " + (coinSelection.valueGathered.value - minDustThreshold.value) / 100 + " BSQ."; + "Please increase your balance to at least " + (coinSelection.valueGathered.value + minDustThreshold.value) / 100 + " BSQ."; checkArgument(coinSelection.valueGathered.compareTo(fee) > 0, "This transaction require a change output of at least " + minDustThreshold.value / 100 + " BSQ (dust limit). " + info); @@ -655,6 +660,7 @@ private Transaction getPreparedBurnBsqFeeTx(Coin fee) throws InsufficientBsqExce } } + //todo private Transaction getPreparedBurnFeeTx(Coin fee, boolean requireChangeOutput) throws InsufficientBsqException { daoKillSwitch.assertDaoIsNotDisabled(); final Transaction tx = new Transaction(params); diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/BaseProposalFactory.java b/core/src/main/java/bisq/core/dao/governance/proposal/BaseProposalFactory.java index 4c3cb5f78f7..abb58e32c2d 100644 --- a/core/src/main/java/bisq/core/dao/governance/proposal/BaseProposalFactory.java +++ b/core/src/main/java/bisq/core/dao/governance/proposal/BaseProposalFactory.java @@ -87,8 +87,9 @@ private Transaction createTransaction(R proposal) throws InsufficientMoneyExcept try { Coin fee = ProposalConsensus.getFee(daoStateService, daoStateService.getChainHeight()); // We create a prepared Bsq Tx for the proposal fee. - boolean requireChangeOutput = proposal instanceof IssuanceProposal; - Transaction preparedBurnFeeTx = bsqWalletService.getPreparedProposalTx(fee, requireChangeOutput); + Transaction preparedBurnFeeTx = proposal instanceof IssuanceProposal ? + bsqWalletService.getPreparedIssuanceTx(fee) : + bsqWalletService.getPreparedProposalTx(fee); // payload does not have txId at that moment byte[] hashOfPayload = ProposalConsensus.getHashOfPayload(proposal); diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index f0529478aa6..9cf8e5e838c 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -167,7 +167,7 @@ void processTxOutput(TempTxOutput tempTxOutput) { // In case we have the opReturn for a burn fee tx all outputs after 1st output are considered BTC handleBtcOutput(tempTxOutput, index); } else if (availableInputValue > 0 && availableInputValue >= txOutputValue) { - if (tempTxOutput.getBlockHeight() >= getActivateHardFork1Height() && prohibitMoreBsqOutputs) { + if (isHardForkActivated(tempTxOutput) && prohibitMoreBsqOutputs) { handleBtcOutput(tempTxOutput, index); } else { handleBsqOutput(tempTxOutput, index, txOutputValue); @@ -238,7 +238,13 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { case UNDEFINED: break; case PROPOSAL: - // TODO + if (isHardForkActivated(tempTxOutput)) { + // We enforce a mandatory BSQ change output. + // We need that as similar to ASSET_LISTING_FEE and PROOF_OF_BURN + // we could not distinguish between 2 structurally same transactions otherwise (only way here + // would be to check the proposal fee as that is known from the params). + return tempTxOutput.getIndex() >= 1; + } break; case COMPENSATION_REQUEST: break; @@ -252,27 +258,27 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { case LOCKUP: break; case ASSET_LISTING_FEE: + case PROOF_OF_BURN: + // Asset listing fee and proof of burn tx are structurally the same. + // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 - // structurally same transactions where only the BSQ fee is different and the asset listing fee is - // a user input when creating the asset listing, so it is not know to the parser. - // Instead we derive the asset listing fee from the parser. + // structurally same transactions where only the BSQ fee is different. In case of asset listing fee and proof of + // burn it is a user input, so it is not know to the parser, instead we derive the burned fee from the parser. + // In case of proposal fee we could derive it from the params. - // Case 1: 10 BSQ asset listing fee + // Case 1: 10 BSQ fee to burn // In: 17 BSQ // Out: BSQ change 7 BSQ -> valid BSQ // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) - // Case 2: 17 BSQ asset listing fee + // Case 2: 17 BSQ fee to burn // In: 17 BSQ // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) return tempTxOutput.getIndex() >= 1; - case PROOF_OF_BURN: - // TODO - return tempTxOutput.getIndex() >= 1; } } return false; @@ -336,6 +342,10 @@ private void handleBtcOutput(TempTxOutput txOutput, int index) { } } + private boolean isHardForkActivated(TempTxOutput tempTxOutput) { + return tempTxOutput.getBlockHeight() >= getActivateHardFork1Height(); + } + private int getActivateHardFork1Height() { return BisqEnvironment.getBaseCurrencyNetwork().isMainnet() ? ACTIVATE_HARD_FORK_1_HEIGHT_MAINNET : BisqEnvironment.getBaseCurrencyNetwork().isTestnet() ? ACTIVATE_HARD_FORK_1_HEIGHT_TESTNET : diff --git a/core/src/main/java/bisq/core/offer/placeoffer/tasks/CreateMakerFeeTx.java b/core/src/main/java/bisq/core/offer/placeoffer/tasks/CreateMakerFeeTx.java index 70bf6d5cf25..da2b60a0ace 100644 --- a/core/src/main/java/bisq/core/offer/placeoffer/tasks/CreateMakerFeeTx.java +++ b/core/src/main/java/bisq/core/offer/placeoffer/tasks/CreateMakerFeeTx.java @@ -109,7 +109,7 @@ public void onFailure(TxBroadcastException exception) { }); } else { final BsqWalletService bsqWalletService = model.getBsqWalletService(); - Transaction preparedBurnFeeTx = model.getBsqWalletService().getPreparedBurnFeeTx(offer.getMakerFee()); + Transaction preparedBurnFeeTx = model.getBsqWalletService().getPreparedTradeFeeTx(offer.getMakerFee()); Transaction txWithBsqFee = tradeWalletService.completeBsqTradingFeeTx(preparedBurnFeeTx, fundingAddress, reservedForTradeAddress, diff --git a/core/src/main/java/bisq/core/trade/protocol/tasks/taker/CreateTakerFeeTx.java b/core/src/main/java/bisq/core/trade/protocol/tasks/taker/CreateTakerFeeTx.java index 15ede07092e..1e9f1a561c8 100644 --- a/core/src/main/java/bisq/core/trade/protocol/tasks/taker/CreateTakerFeeTx.java +++ b/core/src/main/java/bisq/core/trade/protocol/tasks/taker/CreateTakerFeeTx.java @@ -79,7 +79,7 @@ protected void run() { false, null); } else { - Transaction preparedBurnFeeTx = processModel.getBsqWalletService().getPreparedBurnFeeTx(trade.getTakerFee()); + Transaction preparedBurnFeeTx = processModel.getBsqWalletService().getPreparedTradeFeeTx(trade.getTakerFee()); Transaction txWithBsqFee = tradeWalletService.completeBsqTradingFeeTx(preparedBurnFeeTx, fundingAddress, reservedForTradeAddress, From 5870f921ce7c3c6333e770da63a8902d60bd6b70 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 22:26:51 -0500 Subject: [PATCH 12/21] Add fix for not correctly handled issuance tx --- .../core/dao/node/parser/TxOutputParser.java | 57 ++++++++++++++----- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 9cf8e5e838c..a2ff1f95d9d 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -166,6 +166,12 @@ void processTxOutput(TempTxOutput tempTxOutput) { } else if (isBtcOutputOfBurnFeeTx(tempTxOutput)) { // In case we have the opReturn for a burn fee tx all outputs after 1st output are considered BTC handleBtcOutput(tempTxOutput, index); + } else if (isHardForkActivated(tempTxOutput) && isIssuanceCandidateTxOutput(tempTxOutput)) { + // After the hard fork activation we fix a bug with a transaction which would have interpreted the + // issuance output as BSQ if the availableInputValue was >= issuance amount. + // Such a tx was never created but as we don't know if it will happen before activation date we cannot + // enforce the bugfix which represents a rule change before the activation date. + handleIssuanceCandidateOutput(tempTxOutput); } else if (availableInputValue > 0 && availableInputValue >= txOutputValue) { if (isHardForkActivated(tempTxOutput) && prohibitMoreBsqOutputs) { handleBtcOutput(tempTxOutput, index); @@ -284,6 +290,24 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { return false; } + private boolean isIssuanceCandidateTxOutput(TempTxOutput tempTxOutput) { + // If we have BSQ left as fee and we are at the second output we interpret it as a compensation request output. + return availableInputValue > 0 && + tempTxOutput.getIndex() == 1 && + optionalOpReturnType.isPresent() && + (optionalOpReturnType.get() == OpReturnType.COMPENSATION_REQUEST || + optionalOpReturnType.get() == OpReturnType.REIMBURSEMENT_REQUEST); + } + + private void handleIssuanceCandidateOutput(TempTxOutput tempTxOutput) { + // We do not permit more BSQ outputs after the issuance candidate. + prohibitMoreBsqOutputs = true; + + // We store the candidate but we don't apply the TxOutputType yet as we need to verify the fee after all + // outputs are parsed and check the phase. The TxParser will do that.... + optionalIssuanceCandidate = Optional.of(tempTxOutput); + } + private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValue) { // Update the input balance. availableInputValue -= txOutputValue; @@ -322,23 +346,30 @@ private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValu } private void handleBtcOutput(TempTxOutput txOutput, int index) { - // If we have BSQ left as fee and we are at the second output it might be a compensation request output. - // We store the candidate but we don't apply the TxOutputType yet as we need to verify the fee after all - // outputs are parsed and check the phase. The TxParser will do that.... - if (availableInputValue > 0 && - index == 1 && - optionalOpReturnType.isPresent() && - (optionalOpReturnType.get() == OpReturnType.COMPENSATION_REQUEST || - optionalOpReturnType.get() == OpReturnType.REIMBURSEMENT_REQUEST)) { - optionalIssuanceCandidate = Optional.of(txOutput); - - // We do not permit more BSQ outputs after the issuance candidate. - prohibitMoreBsqOutputs = true; - } else { + if (isHardForkActivated(txOutput)) { txOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); // For regular transactions we don't permit BSQ outputs after a BTC output was detected. prohibitMoreBsqOutputs = true; + } else { + // If we have BSQ left as fee and we are at the second output it might be a compensation request output. + // We store the candidate but we don't apply the TxOutputType yet as we need to verify the fee after all + // outputs are parsed and check the phase. The TxParser will do that.... + if (availableInputValue > 0 && + index == 1 && + optionalOpReturnType.isPresent() && + (optionalOpReturnType.get() == OpReturnType.COMPENSATION_REQUEST || + optionalOpReturnType.get() == OpReturnType.REIMBURSEMENT_REQUEST)) { + optionalIssuanceCandidate = Optional.of(txOutput); + + // We do not permit more BSQ outputs after the issuance candidate. + prohibitMoreBsqOutputs = true; + } else { + txOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + + // For regular transactions we don't permit BSQ outputs after a BTC output was detected. + prohibitMoreBsqOutputs = true; + } } } From 51955dcbed38bffac9a01d7418f4c9ee38c12e88 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Thu, 17 Oct 2019 22:33:43 -0500 Subject: [PATCH 13/21] Use new method for issuance tx // For issuance txs we also require a BSQ change output before the issuance output gets added. There was a // minor bug with the old version that multiple inputs would have caused an exception in case there was no // change output (e.g. inputs of 21 and 6 BSQ for BSQ fee of 21 BSQ would have caused that only 1 input was used // and then caused an error as we enforced a change output. This new version handles such cases correctly. --- .../core/btc/wallet/BsqWalletService.java | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index be209684ea8..f35a7898371 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -579,12 +579,11 @@ private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmoun // We create a tx with Bsq inputs for the fee and optional BSQ change output. // As the fee amount will be missing in the output those BSQ fees are burned. public Transaction getPreparedProposalTx(Coin fee) throws InsufficientBsqException { - return getPreparedBurnBsqFeeTx(fee); + return getPreparedTxWithMandatoryBsqChangeOutput(fee); } - //todo public Transaction getPreparedIssuanceTx(Coin fee) throws InsufficientBsqException { - return getPreparedBurnBsqFeeTx(fee); + return getPreparedTxWithMandatoryBsqChangeOutput(fee); } public Transaction getPreparedTradeFeeTx(Coin fee) throws InsufficientBsqException { @@ -592,33 +591,39 @@ public Transaction getPreparedTradeFeeTx(Coin fee) throws InsufficientBsqExcepti } public Transaction getPreparedProofOfBurnTx(Coin fee) throws InsufficientBsqException { - return getPreparedBurnBsqFeeTx(fee); + return getPreparedTxWithMandatoryBsqChangeOutput(fee); } public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws InsufficientBsqException { - return getPreparedBurnBsqFeeTx(fee); + return getPreparedTxWithMandatoryBsqChangeOutput(fee); } - private Transaction getPreparedBurnBsqFeeTx(Coin fee) throws InsufficientBsqException { - daoKillSwitch.assertDaoIsNotDisabled(); + // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 + // structurally same transactions where only the BSQ fee is different. In case of asset listing fee and proof of + // burn it is a user input, so it is not know to the parser, instead we derive the burned fee from the parser. - // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 - // structurally same transactions where only the BSQ fee is different. In case of asset listing fee and proof of - // burn it is a user input, so it is not know to the parser, instead we derive the burned fee from the parser. - // In case of proposal fee we could derive it from the params. + // In case of proposal fee we could derive it from the params. - // Case 1: 10 BSQ fee to burn - // In: 17 BSQ - // Out: BSQ change 7 BSQ -> valid BSQ - // Out: OpReturn - // Miner fee: 1000 sat (10 BSQ burned) + // For issuance txs we also require a BSQ change output before the issuance output gets added. There was a + // minor bug with the old version that multiple inputs would have caused an exception in case there was no + // change output (e.g. inputs of 21 and 6 BSQ for BSQ fee of 21 BSQ would have caused that only 1 input was used + // and then caused an error as we enforced a change output. This new version handles such cases correctly. + // Examples for the structurally indistinguishable transactions: + // Case 1: 10 BSQ fee to burn + // In: 17 BSQ + // Out: BSQ change 7 BSQ -> valid BSQ + // Out: OpReturn + // Miner fee: 1000 sat (10 BSQ burned) - // Case 2: 17 BSQ fee to burn - // In: 17 BSQ - // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) - // Out: OpReturn - // Miner fee: 1000 sat (10 BSQ burned) + // Case 2: 17 BSQ fee to burn + // In: 17 BSQ + // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) + // Out: OpReturn + // Miner fee: 1000 sat (10 BSQ burned) + + private Transaction getPreparedTxWithMandatoryBsqChangeOutput(Coin fee) throws InsufficientBsqException { + daoKillSwitch.assertDaoIsNotDisabled(); Transaction tx = new Transaction(params); // We look for inputs covering out BSQ fee we want to pay. From 8feba90e56af0b0728f959839a1b965759454e32 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 19 Oct 2019 17:14:47 -0500 Subject: [PATCH 14/21] Handle all possible blind vote fee transactions --- .../core/dao/node/parser/TxOutputParser.java | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index a2ff1f95d9d..a8e47ce81db 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -19,6 +19,7 @@ import bisq.core.app.BisqEnvironment; import bisq.core.dao.governance.bond.BondConsensus; +import bisq.core.dao.governance.param.Param; import bisq.core.dao.state.DaoStateService; import bisq.core.dao.state.model.blockchain.OpReturnType; import bisq.core.dao.state.model.blockchain.TxOutput; @@ -113,6 +114,7 @@ class TxOutputParser { private Optional optionalVoteRevealUnlockStakeOutput = Optional.empty(); @Getter private Optional optionalLockupOutput = Optional.empty(); + private Optional optionalOpReturnIndex = Optional.empty(); // Private private int lockTime; @@ -141,6 +143,8 @@ void processOpReturnOutput(TempTxOutput tempTxOutput) { optionalOpReturnType = getMappedOpReturnType(txOutputType); + optionalOpReturnType.ifPresent(e -> optionalOpReturnIndex = Optional.of(tempTxOutput.getIndex())); + // If we have a LOCKUP opReturn output we save the lockTime to apply it later to the LOCKUP output. // We keep that data in that other output as it makes parsing of the UNLOCK tx easier. optionalOpReturnType.filter(opReturnType -> opReturnType == OpReturnType.LOCKUP) @@ -168,10 +172,16 @@ void processTxOutput(TempTxOutput tempTxOutput) { handleBtcOutput(tempTxOutput, index); } else if (isHardForkActivated(tempTxOutput) && isIssuanceCandidateTxOutput(tempTxOutput)) { // After the hard fork activation we fix a bug with a transaction which would have interpreted the - // issuance output as BSQ if the availableInputValue was >= issuance amount. + // vote fee output as BSQ if the vote fee was >= miner fee. // Such a tx was never created but as we don't know if it will happen before activation date we cannot - // enforce the bugfix which represents a rule change before the activation date. + // enforce the bug fix which represents a rule change before the activation date. handleIssuanceCandidateOutput(tempTxOutput); + } else if (isHardForkActivated(tempTxOutput) && isBlindVoteFeeOutput(tempTxOutput)) { + // After the hard fork activation we fix a bug with a transaction which would have interpreted the + // issuance output as BSQ if the availableInputValue was >= issuance amount. + // Such a tx was never created but as we don't know if it will happen before activation date we cannot + // enforce the bug fix which represents a rule change before the activation date. + handleBlindVoteFeeOutput(tempTxOutput); } else if (availableInputValue > 0 && availableInputValue >= txOutputValue) { if (isHardForkActivated(tempTxOutput) && prohibitMoreBsqOutputs) { handleBtcOutput(tempTxOutput, index); @@ -192,6 +202,7 @@ void processTxOutput(TempTxOutput tempTxOutput) { } } + void commitUTXOCandidates() { utxoCandidates.forEach(output -> daoStateService.addUnspentTxOutput(TxOutput.fromTempOutput(output))); } @@ -308,6 +319,45 @@ private void handleIssuanceCandidateOutput(TempTxOutput tempTxOutput) { optionalIssuanceCandidate = Optional.of(tempTxOutput); } + private void handleBlindVoteFeeOutput(TempTxOutput tempTxOutput) { + tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); + prohibitMoreBsqOutputs = true; + } + + private boolean isBlindVoteFeeOutput(TempTxOutput tempTxOutput) { + if (!optionalOpReturnType.isPresent()) { + return false; + } + + if (optionalOpReturnType.get() != OpReturnType.BLIND_VOTE) { + return false; + } + + // If it is the vote stake output we return false. + if (tempTxOutput.getIndex() == 0) { + return false; + } + + // There must be a vote fee left + if (availableInputValue <= 0) { + return false; + } + + // Burned BSQ output is last output before opReturn. + // We could have also a BSQ change output as last output before opReturn but that will + // be detected at blindVoteFee check. + // We always have the BSQ change before the burned BSQ output if both are present. + checkArgument(optionalOpReturnIndex.isPresent()); + if (tempTxOutput.getIndex() != optionalOpReturnIndex.get() - 1) { + return false; + } + + // Without checking the fee we would not be able to distinguish between 2 structurally same transactions, one + // where the output is burned BSQ and one where it is a BSQ change output. + long blindVoteFee = daoStateService.getParamValueAsCoin(Param.BLIND_VOTE_FEE, tempTxOutput.getBlockHeight()).value; + return availableInputValue == blindVoteFee; + } + private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValue) { // Update the input balance. availableInputValue -= txOutputValue; From c39ae974824c799b81562f6cf58bbc551adcdaa6 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 19 Oct 2019 17:23:06 -0500 Subject: [PATCH 15/21] Move check for invalid opReturn output up --- .../bisq/core/dao/node/parser/TxOutputParser.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index a8e47ce81db..57df1facdd5 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -152,14 +152,14 @@ void processOpReturnOutput(TempTxOutput tempTxOutput) { } void processTxOutput(TempTxOutput tempTxOutput) { - if (!daoStateService.isConfiscatedOutput(tempTxOutput.getKey())) { - // We don not expect here an opReturn output as we do not get called on the last output. Any opReturn at - // another output index is invalid. - if (tempTxOutput.isOpReturnOutput()) { - tempTxOutput.setTxOutputType(TxOutputType.INVALID_OUTPUT); - return; - } + // We don not expect here an opReturn output as we do not get called on the last output. Any opReturn at + // another output index is invalid. + if (tempTxOutput.isOpReturnOutput()) { + tempTxOutput.setTxOutputType(TxOutputType.INVALID_OUTPUT); + return; + } + if (!daoStateService.isConfiscatedOutput(tempTxOutput.getKey())) { long txOutputValue = tempTxOutput.getValue(); int index = tempTxOutput.getIndex(); if (isUnlockBondTx(tempTxOutput.getValue(), index)) { From 3c60e1f3b65b703898cd988c1c3c9ac98a90cdff Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 19 Oct 2019 19:36:12 -0500 Subject: [PATCH 16/21] Add dust check at final sign method --- .../bisq/core/btc/wallet/BsqWalletService.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index f35a7898371..0d8714c9285 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -493,6 +493,15 @@ public Transaction signTx(Transaction tx) throws WalletException, TransactionVer } } + for (TransactionOutput txo : tx.getOutputs()) { + Coin value = txo.getValue(); + // OpReturn outputs have value 0 + if (value.isPositive()) { + checkArgument(Restrictions.isAboveDust(txo.getValue()), + "An output value is below dust limit. Transaction=" + tx); + } + } + checkWalletConsistency(wallet); verifyTransaction(tx); printTx("BSQ wallet: Signed Tx", tx); @@ -556,11 +565,11 @@ private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmoun // Tx has as first output BSQ and an optional second BSQ change output. // At that stage we do not have added the BTC inputs so there is no BTC change output here. if (tx.getOutputs().size() == 2) { - TransactionOutput bsqChangeOutput = tx.getOutputs().get(1); - if (!Restrictions.isAboveDust(bsqChangeOutput.getValue())) { - String msg = "BSQ change output is below dust limit. outputValue=" + bsqChangeOutput.getValue().toFriendlyString(); + Coin bsqChangeOutputValue = tx.getOutputs().get(1).getValue(); + if (!Restrictions.isAboveDust(bsqChangeOutputValue)) { + String msg = "BSQ change output is below dust limit. outputValue=" + bsqChangeOutputValue.value / 100 + " BSQ"; log.warn(msg); - throw new BsqChangeBelowDustException(msg, bsqChangeOutput.getValue()); + throw new BsqChangeBelowDustException(msg, bsqChangeOutputValue); } } From 253db9bd80373d018b855e65ccfe72e78fbba459 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 19 Oct 2019 19:36:31 -0500 Subject: [PATCH 17/21] Fix incorrect comments --- .../main/java/bisq/core/btc/wallet/BtcWalletService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java index 4841551cb9a..7e1ed9c9c74 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java @@ -408,13 +408,13 @@ public Transaction completePreparedSendBsqTx(Transaction preparedBsqTx, boolean TransactionVerificationException, WalletException, InsufficientMoneyException { // preparedBsqTx has following structure: // inputs [1-n] BSQ inputs - // outputs [0-1] BSQ receivers output + // outputs [1] BSQ receivers output // outputs [0-1] BSQ change output // We add BTC mining fee. Result tx looks like: // inputs [1-n] BSQ inputs // inputs [1-n] BTC inputs - // outputs [0-1] BSQ receivers output + // outputs [1] BSQ receivers output // outputs [0-1] BSQ change output // outputs [0-1] BTC change output // mining fee: BTC mining fee @@ -426,7 +426,7 @@ public Transaction completePreparedBsqTx(Transaction preparedBsqTx, boolean useC // preparedBsqTx has following structure: // inputs [1-n] BSQ inputs - // outputs [0-1] BSQ receivers output + // outputs [1] BSQ receivers output // outputs [0-1] BSQ change output // mining fee: optional burned BSQ fee (only if opReturnData != null) From c76639c2f023a78ba58e915b019d61d205634ef4 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 19 Oct 2019 19:44:26 -0500 Subject: [PATCH 18/21] Refactor - Remove requireChangeOutput param which is always false - Remove method which is used only by one caller - Cleanup --- .../core/btc/wallet/BsqWalletService.java | 49 +++++++------------ 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index 0d8714c9285..b187aa25172 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -585,6 +585,14 @@ private Transaction getPreparedSendTx(String receiverAddress, Coin receiverAmoun // Burn fee txs /////////////////////////////////////////////////////////////////////////////////////////// + public Transaction getPreparedTradeFeeTx(Coin fee) throws InsufficientBsqException { + daoKillSwitch.assertDaoIsNotDisabled(); + + Transaction tx = new Transaction(params); + addInputsAndChangeOutputForTx(tx, fee, bsqCoinSelector); + return tx; + } + // We create a tx with Bsq inputs for the fee and optional BSQ change output. // As the fee amount will be missing in the output those BSQ fees are burned. public Transaction getPreparedProposalTx(Coin fee) throws InsufficientBsqException { @@ -595,10 +603,6 @@ public Transaction getPreparedIssuanceTx(Coin fee) throws InsufficientBsqExcepti return getPreparedTxWithMandatoryBsqChangeOutput(fee); } - public Transaction getPreparedTradeFeeTx(Coin fee) throws InsufficientBsqException { - return getPreparedBurnFeeTx(fee, false); - } - public Transaction getPreparedProofOfBurnTx(Coin fee) throws InsufficientBsqException { return getPreparedTxWithMandatoryBsqChangeOutput(fee); } @@ -674,49 +678,30 @@ private Transaction getPreparedTxWithMandatoryBsqChangeOutput(Coin fee) throws I } } - //todo - private Transaction getPreparedBurnFeeTx(Coin fee, boolean requireChangeOutput) throws InsufficientBsqException { - daoKillSwitch.assertDaoIsNotDisabled(); - final Transaction tx = new Transaction(params); - addInputsAndChangeOutputForTx(tx, fee, bsqCoinSelector, requireChangeOutput); - // printTx("getPreparedFeeTx", tx); - return tx; - } - private void addInputsAndChangeOutputForTx(Transaction tx, Coin fee, - BsqCoinSelector bsqCoinSelector, - boolean requireChangeOutput) + BsqCoinSelector bsqCoinSelector) throws InsufficientBsqException { Coin requiredInput; // If our fee is less then dust limit we increase it so we are sure to not get any dust output. - if (Restrictions.isDust(fee)) - requiredInput = Restrictions.getMinNonDustOutput().add(fee); - else + if (Restrictions.isDust(fee)) { + requiredInput = fee.add(Restrictions.getMinNonDustOutput()); + } else { requiredInput = fee; + } CoinSelection coinSelection = bsqCoinSelector.select(requiredInput, wallet.calculateAllSpendCandidates()); coinSelection.gathered.forEach(tx::addInput); try { - // TODO why is fee passed to getChange ??? Coin change = bsqCoinSelector.getChange(fee, coinSelection); - if (requireChangeOutput) { - checkArgument(change.isPositive(), - "This transaction requires a mandatory BSQ change output. " + - "At least " + Restrictions.getMinNonDustOutput().add(fee).value / 100d + " " + - "BSQ is needed for this transaction"); - } - if (change.isPositive()) { checkArgument(Restrictions.isAboveDust(change), "The change output of " + change.value / 100d + " BSQ is below the min. dust value of " + Restrictions.getMinNonDustOutput().value / 100d + - ". At least " + Restrictions.getMinNonDustOutput().add(fee).value / 100d + " " + - "BSQ is needed for this transaction"); + ". At least " + Restrictions.getMinNonDustOutput().add(fee).value / 100d + + " BSQ is needed for this transaction"); tx.addOutput(change, getChangeAddress()); } - - log.error(tx.toString()); } catch (InsufficientMoneyException e) { log.error(tx.toString()); throw new InsufficientBsqException(e.missing); @@ -734,7 +719,7 @@ public Transaction getPreparedBlindVoteTx(Coin fee, Coin stake) throws Insuffici daoKillSwitch.assertDaoIsNotDisabled(); Transaction tx = new Transaction(params); tx.addOutput(new TransactionOutput(params, tx, stake, getUnusedAddress())); - addInputsAndChangeOutputForTx(tx, fee.add(stake), bsqCoinSelector, false); + addInputsAndChangeOutputForTx(tx, fee.add(stake), bsqCoinSelector); //printTx("getPreparedBlindVoteTx", tx); return tx; } @@ -768,7 +753,7 @@ public Transaction getPreparedLockupTx(Coin lockupAmount) throws AddressFormatEx Transaction tx = new Transaction(params); checkArgument(Restrictions.isAboveDust(lockupAmount), "The amount is too low (dust limit)."); tx.addOutput(new TransactionOutput(params, tx, lockupAmount, getUnusedAddress())); - addInputsAndChangeOutputForTx(tx, lockupAmount, bsqCoinSelector, false); + addInputsAndChangeOutputForTx(tx, lockupAmount, bsqCoinSelector); printTx("prepareLockupTx", tx); return tx; } From b63178bb527bbe843d500f90d9796e88cebe0e31 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sat, 19 Oct 2019 22:10:10 -0500 Subject: [PATCH 19/21] Add comment --- .../main/java/bisq/core/btc/wallet/TradeWalletService.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java index 798ae449c5f..e0c57058548 100644 --- a/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java @@ -220,6 +220,10 @@ public Transaction completeBsqTradingFeeTx(Transaction preparedBsqTx, // 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) + // If BSQ trade fee > reservedFundsForOffer we would create a BSQ output instead of a BTC output. + // As the min. reservedFundsForOffer is 0.001 BTC which is 1000 BSQ this is an unrealistic scenario and not + // handled atm (if BTC price is 1M USD and BSQ price is 0.1 USD, then fee would be 10% which still is unrealistic). + // WalletService.printTx("preparedBsqTx", preparedBsqTx); SendRequest sendRequest = SendRequest.forTx(preparedBsqTx); sendRequest.shuffleOutputs = false; From ade296671756cc22e29c6fb486feac9da216b340 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 20 Oct 2019 10:16:20 -0500 Subject: [PATCH 20/21] Fix comments, rename methods --- .../java/bisq/core/btc/wallet/BsqWalletService.java | 2 +- .../bisq/core/dao/node/parser/TxOutputParser.java | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java index b187aa25172..915e00ca235 100644 --- a/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java +++ b/core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java @@ -613,7 +613,7 @@ public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws Insuffic // We need to require one BSQ change output as we could otherwise not be able to distinguish between 2 // structurally same transactions where only the BSQ fee is different. In case of asset listing fee and proof of - // burn it is a user input, so it is not know to the parser, instead we derive the burned fee from the parser. + // burn it is a user input, so it is not know to the parser, instead we derive the burned fee from the parser. // In case of proposal fee we could derive it from the params. diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 57df1facdd5..0beef7fbf4a 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -172,16 +172,17 @@ void processTxOutput(TempTxOutput tempTxOutput) { handleBtcOutput(tempTxOutput, index); } else if (isHardForkActivated(tempTxOutput) && isIssuanceCandidateTxOutput(tempTxOutput)) { // After the hard fork activation we fix a bug with a transaction which would have interpreted the - // vote fee output as BSQ if the vote fee was >= miner fee. + // issuance output as BSQ if the availableInputValue was >= issuance amount. // Such a tx was never created but as we don't know if it will happen before activation date we cannot // enforce the bug fix which represents a rule change before the activation date. handleIssuanceCandidateOutput(tempTxOutput); - } else if (isHardForkActivated(tempTxOutput) && isBlindVoteFeeOutput(tempTxOutput)) { + } else if (isHardForkActivated(tempTxOutput) && isBlindVoteBurnedFeeOutput(tempTxOutput)) { // After the hard fork activation we fix a bug with a transaction which would have interpreted the - // issuance output as BSQ if the availableInputValue was >= issuance amount. + // vote fee output as BSQ if the vote fee was >= miner fee. // Such a tx was never created but as we don't know if it will happen before activation date we cannot // enforce the bug fix which represents a rule change before the activation date. - handleBlindVoteFeeOutput(tempTxOutput); + + handleBlindVoteBurnedFeeOutput(tempTxOutput); } else if (availableInputValue > 0 && availableInputValue >= txOutputValue) { if (isHardForkActivated(tempTxOutput) && prohibitMoreBsqOutputs) { handleBtcOutput(tempTxOutput, index); @@ -319,12 +320,12 @@ private void handleIssuanceCandidateOutput(TempTxOutput tempTxOutput) { optionalIssuanceCandidate = Optional.of(tempTxOutput); } - private void handleBlindVoteFeeOutput(TempTxOutput tempTxOutput) { + private void handleBlindVoteBurnedFeeOutput(TempTxOutput tempTxOutput) { tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); prohibitMoreBsqOutputs = true; } - private boolean isBlindVoteFeeOutput(TempTxOutput tempTxOutput) { + private boolean isBlindVoteBurnedFeeOutput(TempTxOutput tempTxOutput) { if (!optionalOpReturnType.isPresent()) { return false; } From 562310eae771d353562f78e7cbed3b252a66bd48 Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Sun, 20 Oct 2019 11:29:09 -0500 Subject: [PATCH 21/21] Move code of isBlindVoteBurnedFeeOutput to isBtcOutputOfBurnFeeTx --- .../core/dao/node/parser/TxOutputParser.java | 84 ++++++++----------- 1 file changed, 33 insertions(+), 51 deletions(-) diff --git a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java index 0beef7fbf4a..0a9d09bf34d 100644 --- a/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java +++ b/core/src/main/java/bisq/core/dao/node/parser/TxOutputParser.java @@ -176,13 +176,6 @@ void processTxOutput(TempTxOutput tempTxOutput) { // Such a tx was never created but as we don't know if it will happen before activation date we cannot // enforce the bug fix which represents a rule change before the activation date. handleIssuanceCandidateOutput(tempTxOutput); - } else if (isHardForkActivated(tempTxOutput) && isBlindVoteBurnedFeeOutput(tempTxOutput)) { - // After the hard fork activation we fix a bug with a transaction which would have interpreted the - // vote fee output as BSQ if the vote fee was >= miner fee. - // Such a tx was never created but as we don't know if it will happen before activation date we cannot - // enforce the bug fix which represents a rule change before the activation date. - - handleBlindVoteBurnedFeeOutput(tempTxOutput); } else if (availableInputValue > 0 && availableInputValue >= txOutputValue) { if (isHardForkActivated(tempTxOutput) && prohibitMoreBsqOutputs) { handleBtcOutput(tempTxOutput, index); @@ -203,7 +196,6 @@ void processTxOutput(TempTxOutput tempTxOutput) { } } - void commitUTXOCandidates() { utxoCandidates.forEach(output -> daoStateService.addUnspentTxOutput(TxOutput.fromTempOutput(output))); } @@ -252,6 +244,7 @@ private void handleUnlockBondTx(TempTxOutput txOutput) { private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { if (optionalOpReturnType.isPresent()) { + int index = tempTxOutput.getIndex(); switch (optionalOpReturnType.get()) { case UNDEFINED: break; @@ -261,7 +254,7 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { // We need that as similar to ASSET_LISTING_FEE and PROOF_OF_BURN // we could not distinguish between 2 structurally same transactions otherwise (only way here // would be to check the proposal fee as that is known from the params). - return tempTxOutput.getIndex() >= 1; + return index >= 1; } break; case COMPENSATION_REQUEST: @@ -269,8 +262,36 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { case REIMBURSEMENT_REQUEST: break; case BLIND_VOTE: - // TODO - break; + if (isHardForkActivated(tempTxOutput)) { + // After the hard fork activation we fix a bug with a transaction which would have interpreted the + // burned vote fee output as BSQ if the vote fee was >= miner fee. + // Such a tx was never created but as we don't know if it will happen before activation date we cannot + // enforce the bug fix which represents a rule change before the activation date. + + // If it is the vote stake output we return false. + if (index == 0) { + return false; + } + + // There must be a vote fee left + if (availableInputValue <= 0) { + return false; + } + + // Burned BSQ output is last output before opReturn. + // We could have also a BSQ change output as last output before opReturn but that will + // be detected at blindVoteFee check. + // We always have the BSQ change before the burned BSQ output if both are present. + checkArgument(optionalOpReturnIndex.isPresent()); + if (index != optionalOpReturnIndex.get() - 1) { + return false; + } + + // Without checking the fee we would not be able to distinguish between 2 structurally same transactions, one + // where the output is burned BSQ and one where it is a BSQ change output. + long blindVoteFee = daoStateService.getParamValueAsCoin(Param.BLIND_VOTE_FEE, tempTxOutput.getBlockHeight()).value; + return availableInputValue == blindVoteFee; + } case VOTE_REVEAL: break; case LOCKUP: @@ -296,7 +317,7 @@ private boolean isBtcOutputOfBurnFeeTx(TempTxOutput tempTxOutput) { // Out: burned BSQ change 7 BSQ -> BTC (7 BSQ burned) // Out: OpReturn // Miner fee: 1000 sat (10 BSQ burned) - return tempTxOutput.getIndex() >= 1; + return index >= 1; } } return false; @@ -320,45 +341,6 @@ private void handleIssuanceCandidateOutput(TempTxOutput tempTxOutput) { optionalIssuanceCandidate = Optional.of(tempTxOutput); } - private void handleBlindVoteBurnedFeeOutput(TempTxOutput tempTxOutput) { - tempTxOutput.setTxOutputType(TxOutputType.BTC_OUTPUT); - prohibitMoreBsqOutputs = true; - } - - private boolean isBlindVoteBurnedFeeOutput(TempTxOutput tempTxOutput) { - if (!optionalOpReturnType.isPresent()) { - return false; - } - - if (optionalOpReturnType.get() != OpReturnType.BLIND_VOTE) { - return false; - } - - // If it is the vote stake output we return false. - if (tempTxOutput.getIndex() == 0) { - return false; - } - - // There must be a vote fee left - if (availableInputValue <= 0) { - return false; - } - - // Burned BSQ output is last output before opReturn. - // We could have also a BSQ change output as last output before opReturn but that will - // be detected at blindVoteFee check. - // We always have the BSQ change before the burned BSQ output if both are present. - checkArgument(optionalOpReturnIndex.isPresent()); - if (tempTxOutput.getIndex() != optionalOpReturnIndex.get() - 1) { - return false; - } - - // Without checking the fee we would not be able to distinguish between 2 structurally same transactions, one - // where the output is burned BSQ and one where it is a BSQ change output. - long blindVoteFee = daoStateService.getParamValueAsCoin(Param.BLIND_VOTE_FEE, tempTxOutput.getBlockHeight()).value; - return availableInputValue == blindVoteFee; - } - private void handleBsqOutput(TempTxOutput txOutput, int index, long txOutputValue) { // Update the input balance. availableInputValue -= txOutputValue;