Apply rule to not allow BSQ outputs after BTC output for regular txs

core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java

@@ -573,19 +573,99 @@ 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 getPreparedTxWithMandatoryBsqChangeOutput(fee);
     }
 
-    public Transaction getPreparedBurnFeeTx(Coin fee) throws InsufficientBsqException {
+    public Transaction getPreparedIssuanceTx(Coin fee) throws InsufficientBsqException {
+        return getPreparedTxWithMandatoryBsqChangeOutput(fee);
+    }
+
+    public Transaction getPreparedTradeFeeTx(Coin fee) throws InsufficientBsqException {
         return getPreparedBurnFeeTx(fee, false);
     }
 
+    public Transaction getPreparedProofOfBurnTx(Coin fee) throws InsufficientBsqException {
+        return getPreparedTxWithMandatoryBsqChangeOutput(fee);
+    }
+
+    public Transaction getPreparedBurnFeeTxForAssetListing(Coin fee) throws InsufficientBsqException {
+        return getPreparedTxWithMandatoryBsqChangeOutput(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. 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.
+
+    // 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)
+
+    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.
+        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);
+                String info = "Available BSQ balance=" + coinSelection.valueGathered.value / 100 + " BSQ. " +
+                        "Intended fee to burn=" + fee.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);
+
+                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). " +
+                                info);
+            }
+
+            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);
+        }
+    }
+
+    //todo
     private Transaction getPreparedBurnFeeTx(Coin fee, boolean requireChangeOutput) throws InsufficientBsqException {
         daoKillSwitch.assertDaoIsNotDisabled();
         final Transaction tx = new Transaction(params);
@@ -626,7 +706,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);
         }
     }

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);

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);

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);