From a3d23c94deaec71482ec8c83826d2e0508e27557 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 6 Sep 2021 20:05:07 +0000 Subject: [PATCH 1/2] wallet: regression test for the previous bugfix --- test/functional/feature_issuance.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/functional/feature_issuance.py b/test/functional/feature_issuance.py index d10e6e37dd..6a9794f7fb 100755 --- a/test/functional/feature_issuance.py +++ b/test/functional/feature_issuance.py @@ -480,5 +480,23 @@ class IssuanceTest(BitcoinTestFramework): self.nodes[0].generate(1) assert_equal(self.nodes[0].gettransaction(tx_id)["confirmations"], 1) + ## Regression for one form of https://github.com/bitcoin/bitcoin/issues/20347 + # 1. Leave node 1 with only a single small explicit output + self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), self.nodes[1].getbalance()['bitcoin'], "", "", True) + blind_addr = self.nodes[1].getnewaddress() + nonblind_addr = self.nodes[1].validateaddress(blind_addr)['unconfidential'] + self.nodes[0].sendtoaddress(nonblind_addr, 0.0005) + self.sync_all() + self.nodes[0].generate(2) + self.sync_all() + + # 2. Try to do an unblinded issuance with only tokens -- the result will + # be an entirely unblinded tx in the 3-output case but a blinded tx + # in the 4-output case. As the bug causes us to do coin selection for + # the former while attempting to produce the latter, we will trigger + # an "impossible" case and a confusing/generic error message. + txid = self.nodes[1].issueasset(0, 1, False)["txid"] + tx = self.nodes[1].getrawtransaction(txid, True) + if __name__ == '__main__': IssuanceTest ().main () From 2ec356a1851c72fb543c260ad84634e090d8d34b Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 6 Sep 2021 19:17:25 +0000 Subject: [PATCH 2/2] wallet: additional patch for https://github.com/bitcoin/bitcoin/issues/20347 How to review this PR: * Skim the description of https://github.com/bitcoin/bitcoin/issues/20347 to see that it's roughly "when pick_new_inputs is unset but bnb_used is set, one the next iteration of the loop an assumption may be wrong" * See that in this case, we just add another iteration to the loop... * ...in exactly the same way as the other place that pick_new_inputs is turned off, I just missed this one before * Observe that the regression test fails before the patch but passes after. --- src/wallet/wallet.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 70d658e323..d0809de4dc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3775,6 +3775,7 @@ bool CWallet::CreateTransactionInternal( CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { pick_new_inputs = false; + one_more_try_20347 = bnb_used; nFeeRet = fee_needed_with_change; continue; }