From dfc3e891d7b24cde6f96b8d853ead439f21bb46c Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 11 Aug 2021 22:05:51 -0400 Subject: [PATCH 01/14] wallet: Assert that enough was selected to cover the fees When the fee is not subtracted from the outputs, the amount that has been reserved for the fee (change_and_fee - change_amount) must be enough to cover the fee that is needed. It would be a bug to not do so, so use an assert to make this obvious if such a situation were to occur. Github-Pull: bitcoin/bitcoin#22686 Rebased-From: d9262324e80da32725e21d4e0fbfee56f25490b1 --- src/wallet/spend.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index e0912d4acf..36516ab5b7 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1385,6 +1385,10 @@ bool CWallet::CreateTransactionInternal( fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); } + // The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when + // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug. + assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= change_and_fee - change_amount); + // Update nFeeRet in case fee_needed changed due to dropping the change output if (fee_needed <= map_change_and_fee.at(policyAsset) - change_amount) { nFeeRet = map_change_and_fee.at(policyAsset) - change_amount; From 2273079c450aa5f3cc9895c3085eae35f47f28f2 Mon Sep 17 00:00:00 2001 From: Glenn Willen Date: Wed, 27 Jul 2022 08:36:39 -0700 Subject: [PATCH 02/14] elements: Fix build by removing newly-added assertion from upstream that doesn't make sense with assets --- src/wallet/spend.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 36516ab5b7..e0912d4acf 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1385,10 +1385,6 @@ bool CWallet::CreateTransactionInternal( fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); } - // The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when - // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug. - assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= change_and_fee - change_amount); - // Update nFeeRet in case fee_needed changed due to dropping the change output if (fee_needed <= map_change_and_fee.at(policyAsset) - change_amount) { nFeeRet = map_change_and_fee.at(policyAsset) - change_amount; From 23e91d0ef88ffff45c443d03133dbbeda57c891d Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 Sep 2022 17:39:05 +0000 Subject: [PATCH 03/14] wallet: fix some fee calculation bugs First, this reverts commit ca2d72ae8b747f417de95f3cfa70e2e55325a688 to reinstate an assertion that was added in Bitcoin #22686. It did not compile because our `change_and_fee` variable is a map rather than number; I changed it to use `map_change_and_fee.at(policyAsset)` to match the equivalent change 2 lines down from a5d97b363b51 (merge of Bitcoin #22008). Then fix the following bugs: 1. Change the new test in rpc_fundrawtransaction.py to bump the -maxtxfee value, which we'd otherwise exceed, failing the test and masking actual failures. (This was just caused by the extreme fee settings of the test combined with Elements' large transactions.) 2. Change the fee-output size estimation for `tx_noinputs_size` to be 46 rather than 44 bytes; we forgot that even null surjection/rangeproofs need a 0 byte when output witnesses are present. This mistake triggered the new assertion. 3. Correct the logic in which change outputs are sometimes dropped even when they are the only blinded output in a transaction with blinded inputs. This would cause the new test to fail with `bad-txn-inputs-ne-outputs`; I'm very surprised that no existing tests hit this. (I have an existing comment block in this code where I "promise" that I had a good reason for doing something mysterious related to blinding. I was not able to reverse-engineer my intention here, though I think it is related to this, but since I couldn't understand it I just left this block intact and worked around it.) 4. This then triggered the assertion again since the coin selection code assumes that sufficiently-small change will always be dropped. If we prevent this drop we will have under-funded the transaction. To fix this we add Yet Another Flag `may_need_blinded_dummy` in which we add extra weight to `tx_noinputs_size` in the case that we're doing a blinded tx but have no blind destinations. We turn this off after coin selection if it turns out that we don't have any blinded inputs, though ofc at that point much of the damage/inefficiency has already been done.. 5. Fix some constants in other functional tests which assumed precise fee calculations; these precise values changed because of fixes (2) and (4). There is one new FIXME, which is that the "dummy change" value will now be a zero-valued OP_RETURN but we still put a full-size rangeproof and surjection proof on it. There is some plausible privacy benefit to this but not much, and wasting 5000+ bytes rather than the ~65 needed for an exact-value proof is not worth it. We will fix this in the future when we overhaul the wallet blinding logic. --- src/wallet/spend.cpp | 101 +++++++++++++------- test/functional/wallet_importdescriptors.py | 4 +- test/functional/wallet_keypool.py | 4 +- 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index e0912d4acf..316ac45b35 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1032,9 +1032,13 @@ bool CWallet::CreateTransactionInternal( if (!coin_selection_params.m_subtract_fee_outputs) { coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size) if (g_con_elementsmode) { - coin_selection_params.tx_noinputs_size += 44; // change output: 9 bytes value, 1 byte scriptPubKey, 33 bytes asset, 1 byte nonce + coin_selection_params.tx_noinputs_size += 46; // fee output: 9 bytes value, 1 byte scriptPubKey, 33 bytes asset, 1 byte nonce, 1 byte each for null rangeproof/surjectionproof } } + // ELEMENTS: If we have blinded inputs but no blinded outputs (which, since the wallet + // makes an effort to not produce change, is a common case) then we need to add a + // dummy output. + bool may_need_blinded_dummy = !!blind_details; for (const auto& recipient : vecSend) { CTxOut txout(recipient.asset, recipient.nAmount, recipient.scriptPubKey); @@ -1056,6 +1060,7 @@ bool CWallet::CreateTransactionInternal( if (blind_details) { blind_details->o_pubkeys.push_back(recipient.confidentiality_key); if (blind_details->o_pubkeys.back().IsFullyValid()) { + may_need_blinded_dummy = false; blind_details->num_to_blind++; blind_details->only_recipient_blind_index = txNew.vout.size()-1; if (!coin_selection_params.m_subtract_fee_outputs) { @@ -1064,6 +1069,13 @@ bool CWallet::CreateTransactionInternal( } } } + if (may_need_blinded_dummy && !coin_selection_params.m_subtract_fee_outputs) { + // dummy output: 33 bytes value, 2 byte scriptPubKey, 33 bytes asset, 1 byte nonce, 66 bytes dummy rangeproof, 1 byte null surjectionproof + // FIXME actually, we currently just hand off to BlindTransaction which will put + // a full rangeproof and surjectionproof. We should fix this when we overhaul + // the blinding logic. + coin_selection_params.tx_noinputs_size += 70 + 66 +(MAX_RANGEPROOF_SIZE + DEFAULT_SURJECTIONPROOF_SIZE + WITNESS_SCALE_FACTOR - 1)/WITNESS_SCALE_FACTOR; + } // Include the fees for things that aren't inputs, excluding the change output const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); @@ -1087,6 +1099,17 @@ bool CWallet::CreateTransactionInternal( return false; } + // If all of our inputs are explicit, we don't need a blinded dummy + if (may_need_blinded_dummy) { + may_need_blinded_dummy = false; + for (const auto& coin : setCoins) { + if (!coin.txout.nValue.IsExplicit()) { + may_need_blinded_dummy = true; + break; + } + } + } + // Always make a change output // We will reduce the fee from this change output later, and remove the output if it is too small. // ELEMENTS: wrap this all in a loop, set nChangePosInOut specifically for policy asset @@ -1342,36 +1365,46 @@ bool CWallet::CreateTransactionInternal( CAmount change_amount = change_position->nValue.GetAmount(); if (IsDust(*change_position, coin_selection_params.m_discard_feerate) || change_amount <= coin_selection_params.m_cost_of_change) { - txNew.vout.erase(change_position); + bool was_blinded = blind_details && blind_details->o_pubkeys[nChangePosInOut].IsValid(); + + // If the change was blinded, and was the only blinded output, we cannot drop it + // without causing the transaction to fail to balance. So keep it, and merely + // zero it out. + if (was_blinded && blind_details->num_to_blind == 1) { + assert (may_need_blinded_dummy); + change_position->scriptPubKey = CScript() << OP_RETURN; + change_position->nValue = 0; + } else { + txNew.vout.erase(change_position); - change_pos[nChangePosInOut] = std::nullopt; - tx_blinded.vout.erase(tx_blinded.vout.begin() + nChangePosInOut); - if (tx_blinded.witness.vtxoutwit.size() > (unsigned) nChangePosInOut) { - tx_blinded.witness.vtxoutwit.erase(tx_blinded.witness.vtxoutwit.begin() + nChangePosInOut); - } - if (blind_details) { - bool was_blinded = blind_details->o_pubkeys[nChangePosInOut].IsValid(); - - blind_details->o_amounts.erase(blind_details->o_amounts.begin() + nChangePosInOut); - blind_details->o_assets.erase(blind_details->o_assets.begin() + nChangePosInOut); - blind_details->o_pubkeys.erase(blind_details->o_pubkeys.begin() + nChangePosInOut); - // If change_amount == 0, we did not increment num_to_blind initially - // and therefore do not need to decrement it here. - if (was_blinded) { - blind_details->num_to_blind--; - blind_details->change_to_blind--; - - // FIXME: I promise this makes sense and fixes an actual problem - // with the wallet that users could encounter. But no human could - // follow the logic as to what this does or why it is safe. After - // the 22.0 rebase we need to double-back and replace the blinding - // logic to eliminate a bunch of edge cases and make this logic - // incomprehensible. But in the interest of minimizing diff during - // the rebase I am going to do this for now. - if (blind_details->num_to_blind == 1) { - resetBlindDetails(blind_details); - if (!fillBlindDetails(blind_details, this, txNew, selected_coins, error)) { - return false; + change_pos[nChangePosInOut] = std::nullopt; + tx_blinded.vout.erase(tx_blinded.vout.begin() + nChangePosInOut); + if (tx_blinded.witness.vtxoutwit.size() > (unsigned) nChangePosInOut) { + tx_blinded.witness.vtxoutwit.erase(tx_blinded.witness.vtxoutwit.begin() + nChangePosInOut); + } + if (blind_details) { + + blind_details->o_amounts.erase(blind_details->o_amounts.begin() + nChangePosInOut); + blind_details->o_assets.erase(blind_details->o_assets.begin() + nChangePosInOut); + blind_details->o_pubkeys.erase(blind_details->o_pubkeys.begin() + nChangePosInOut); + // If change_amount == 0, we did not increment num_to_blind initially + // and therefore do not need to decrement it here. + if (was_blinded) { + blind_details->num_to_blind--; + blind_details->change_to_blind--; + + // FIXME: I promise this makes sense and fixes an actual problem + // with the wallet that users could encounter. But no human could + // follow the logic as to what this does or why it is safe. After + // the 22.0 rebase we need to double-back and replace the blinding + // logic to eliminate a bunch of edge cases and make this logic + // incomprehensible. But in the interest of minimizing diff during + // the rebase I am going to do this for now. + if (blind_details->num_to_blind == 1) { + resetBlindDetails(blind_details); + if (!fillBlindDetails(blind_details, this, txNew, selected_coins, error)) { + return false; + } } } } @@ -1385,6 +1418,10 @@ bool CWallet::CreateTransactionInternal( fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); } + // The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when + // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug. + assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= map_change_and_fee.at(policyAsset) - change_amount); + // Update nFeeRet in case fee_needed changed due to dropping the change output if (fee_needed <= map_change_and_fee.at(policyAsset) - change_amount) { nFeeRet = map_change_and_fee.at(policyAsset) - change_amount; @@ -1479,9 +1516,9 @@ bool CWallet::CreateTransactionInternal( summary += strprintf("#%d: %s%s [%s] (%s [%s])\n", i, txNew.vout[i].IsFee() ? "[fee] " : "", unblinded.nValue.GetAmount(), - txNew.vout[i].nValue.IsExplicit() ? "explicit" : "blinded", + blind_details->o_pubkeys[i].IsValid() ? "blinded" : "explicit", unblinded.nAsset.GetAsset().GetHex(), - txNew.vout[i].nAsset.IsExplicit() ? "explicit" : "blinded" + blind_details->o_pubkeys[i].IsValid() ? "blinded" : "explicit" ); } WalletLogPrintf(summary+"\n"); diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index e2ad295b9b..1c38dd2a58 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -404,10 +404,10 @@ def run_test(self): address, solvable=True, ismine=True) - txid = w0.sendtoaddress(address, 49.99993240) + txid = w0.sendtoaddress(address, 49.99965520) w0.generatetoaddress(6, w0.getnewaddress()) self.sync_blocks() - tx = wpriv.createrawtransaction([{"txid": txid, "vout": 0}], [{w0.getnewaddress(): 49.999}, {"fee": 0.0009324}]) + tx = wpriv.createrawtransaction([{"txid": txid, "vout": 0}], [{w0.getnewaddress(): 49.999}, {"fee": 0.00065520}]) signed_tx = wpriv.signrawtransactionwithwallet(tx) w1.sendrawtransaction(signed_tx['hex']) diff --git a/test/functional/wallet_keypool.py b/test/functional/wallet_keypool.py index 0aee82c51d..97ea2e80c6 100755 --- a/test/functional/wallet_keypool.py +++ b/test/functional/wallet_keypool.py @@ -174,11 +174,11 @@ def run_test(self): res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00025000}], options={"subtractFeeFromOutputs": [0], "feeRate": 0.00010}) assert_equal("psbt" in res, True) # should work without subtractFeeFromOutputs if the exact fee is subtracted from the amount - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00021650}], options={"feeRate": 0.00010}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008570}], options={"feeRate": 0.00010}) assert_equal("psbt" in res, True) # dust change should be removed - res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00021000}], options={"feeRate": 0.00010}) + res = w2.walletcreatefundedpsbt(inputs=[], outputs=[{destination: 0.00008200}], options={"feeRate": 0.00010}) assert_equal("psbt" in res, True) # create a transaction without change at the maximum fee rate, such that the output is still spendable: From fac694be4c6539207cc771d549e8da4e5478b492 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 Sep 2022 17:40:47 +0000 Subject: [PATCH 04/14] wallet: don't clear out all the blinding data when dropping change The Elements 22 blinding logic has an edge case where when we drop change, leaving only a single blinded output, we recompute a bunch of blinding data to handle the potential for us to have 0 inputs and 1 output to blind. (BlindTransaction will fail in this case because it cannot make the transaction balance with only one output to mess with.) In this recomputation, we dropped more data than we meant to, causing us to incorrectly blind an output. --- src/wallet/spend.cpp | 35 +++++----- test/functional/elements_regression_1172.py | 73 +++++++++++++++++++++ test/functional/test_runner.py | 1 + 3 files changed, 94 insertions(+), 15 deletions(-) create mode 100755 test/functional/elements_regression_1172.py diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 316ac45b35..00c9dfc90a 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -718,25 +718,29 @@ static uint32_t GetLocktimeForNewTransaction(interfaces::Chain& chain, const uin } // Reset all non-global blinding details. -void resetBlindDetails(BlindDetails* det) { +static void resetBlindDetails(BlindDetails* det, bool preserve_output_data = false) { det->i_amount_blinds.clear(); det->i_asset_blinds.clear(); det->i_assets.clear(); det->i_amounts.clear(); det->o_amounts.clear(); - det->o_pubkeys.clear(); + if (!preserve_output_data) { + det->o_pubkeys.clear(); + } det->o_amount_blinds.clear(); det->o_assets.clear(); det->o_asset_blinds.clear(); - det->num_to_blind = 0; - det->change_to_blind = 0; - det->only_recipient_blind_index = -1; - det->only_change_pos = -1; + if (!preserve_output_data) { + det->num_to_blind = 0; + det->change_to_blind = 0; + det->only_recipient_blind_index = -1; + det->only_change_pos = -1; + } } -bool fillBlindDetails(BlindDetails* det, CWallet* wallet, CMutableTransaction& txNew, std::vector& selected_coins, bilingual_str& error) { +static bool fillBlindDetails(BlindDetails* det, CWallet* wallet, CMutableTransaction& txNew, std::vector& selected_coins, bilingual_str& error) { int num_inputs_blinded = 0; // Fill in input blinding details @@ -1393,15 +1397,15 @@ bool CWallet::CreateTransactionInternal( blind_details->num_to_blind--; blind_details->change_to_blind--; - // FIXME: I promise this makes sense and fixes an actual problem - // with the wallet that users could encounter. But no human could - // follow the logic as to what this does or why it is safe. After - // the 22.0 rebase we need to double-back and replace the blinding - // logic to eliminate a bunch of edge cases and make this logic - // incomprehensible. But in the interest of minimizing diff during - // the rebase I am going to do this for now. + // FIXME: If we drop the change *and* this means we have only one + // blinded output *and* we have no blinded inputs, then this puts + // us in a situation where BlindTransaction will fail. This is + // prevented in fillBlindDetails, which adds an OP_RETURN output + // to handle this case. So do this ludicrous hack to accomplish + // this. This whole lump of un-followable-logic needs to be replaced + // by a complete rewriting of the wallet blinding logic. if (blind_details->num_to_blind == 1) { - resetBlindDetails(blind_details); + resetBlindDetails(blind_details, true /* don't wipe output data */); if (!fillBlindDetails(blind_details, this, txNew, selected_coins, error)) { return false; } @@ -1535,6 +1539,7 @@ bool CWallet::CreateTransactionInternal( int ret = BlindTransaction(blind_details->i_amount_blinds, blind_details->i_asset_blinds, blind_details->i_assets, blind_details->i_amounts, blind_details->o_amount_blinds, blind_details->o_asset_blinds, blind_details->o_pubkeys, issuance_asset_keys, issuance_token_keys, txNew); assert(ret != -1); if (ret != blind_details->num_to_blind) { + WalletLogPrintf("ERROR: tried to blind %d outputs but only blinded %d\n", (int) blind_details->num_to_blind, (int) ret); error = _("Unable to blind the transaction properly. This should not happen."); return false; } diff --git a/test/functional/elements_regression_1172.py b/test/functional/elements_regression_1172.py new file mode 100755 index 0000000000..79d48dde86 --- /dev/null +++ b/test/functional/elements_regression_1172.py @@ -0,0 +1,73 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test blinding logic when change is dropped and we have only one other blinded input + +Constructs a transaction with a sufficiently small change output that it +gets dropped, in which there is only one other blinded input. In the case +that we have no blinded inputs, we would need to add an OP_RETURN output +to the transaction, neccessitating special logic. + +Check that this special logic still results in a correct transaction that +sends the money to the desired recipient (and that the recipient is able +to receive/spend the money). +""" + +from decimal import Decimal + +from test_framework.blocktools import COINBASE_MATURITY +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + satoshi_round, +) + +class WalletCtTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 3 + self.extra_args = [[ + "-blindedaddresses=1", + "-initialfreecoins=2100000000000000", + "-con_blocksubsidy=0", + "-con_connect_genesis_outputs=1", + "-txindex=1", + ]] * self.num_nodes + self.extra_args[0].append("-anyonecanspendaremine=1") # first node gets the coins + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + # Mine 101 blocks to get the initial coins out of IBD + self.nodes[0].generate(COINBASE_MATURITY + 1) + self.nodes[0].syncwithvalidationinterfacequeue() + self.sync_all() + + for i in range(self.num_nodes): + self.log.info(f"Starting with node {i} balance: {self.nodes[i].getbalance()}") + + # Send 1 coin to a new wallet + txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1) + self.log.info(f"Sent one coin to node 1 in {txid}") + self.nodes[0].generate(2) + self.sync_all() + + # Try to send those coins to yet another wallet, sending a large enough amount + # that the change output is dropped. + amt = satoshi_round(Decimal(0.9995)) + txid = self.nodes[1].sendtoaddress(self.nodes[2].getnewaddress(), amt) + self.log.info(f"Sent {amt} LBTC to node 2 in {txid}") + self.nodes[1].generate(2) + self.sync_all() + + for i in range(self.num_nodes): + self.log.info(f"Finished with node {i} balance: {self.nodes[i].getbalance()}") + + assert_equal(self.nodes[1].getbalance(), { "bitcoin": Decimal(0) }) + assert_equal(self.nodes[2].getbalance(), { "bitcoin": amt }) + +if __name__ == '__main__': + WalletCtTest().main() + diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ab0268f922..ef2c7865cf 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -109,6 +109,7 @@ 'feature_initial_reissuance_token.py', 'feature_progress.py', 'rpc_getnewblockhex.py', + 'elements_regression_1172.py', # Longest test should go first, to favor running tests in parallel 'wallet_hd.py --legacy-wallet', 'wallet_hd.py --descriptors', From 79fd90f064fc7c09ab41bced0225c7618a527277 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 Sep 2022 17:41:24 +0000 Subject: [PATCH 05/14] wallet: extend fix to "dropped change is the last blinded output" case --- src/wallet/spend.cpp | 2 +- test/functional/elements_regression_1172.py | 39 ++++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 00c9dfc90a..930e8ff204 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1404,7 +1404,7 @@ bool CWallet::CreateTransactionInternal( // to handle this case. So do this ludicrous hack to accomplish // this. This whole lump of un-followable-logic needs to be replaced // by a complete rewriting of the wallet blinding logic. - if (blind_details->num_to_blind == 1) { + if (blind_details->num_to_blind < 2) { resetBlindDetails(blind_details, true /* don't wipe output data */); if (!fillBlindDetails(blind_details, this, txNew, selected_coins, error)) { return false; diff --git a/test/functional/elements_regression_1172.py b/test/functional/elements_regression_1172.py index 79d48dde86..e070a4b1e8 100755 --- a/test/functional/elements_regression_1172.py +++ b/test/functional/elements_regression_1172.py @@ -39,6 +39,22 @@ def set_test_params(self): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_send(self, amt, from_idx, to_idx, confidential): + # Try to send those coins to yet another wallet, sending a large enough amount + # that the change output is dropped. + address = self.nodes[to_idx].getnewaddress() + if not confidential: + address = self.nodes[to_idx].getaddressinfo(address)['unconfidential'] + txid = self.nodes[from_idx].sendtoaddress(address, amt) + self.log.info(f"Sent {amt} LBTC to node {to_idx} in {txid}") + self.nodes[from_idx].generate(2) + self.sync_all() + + for i in range(self.num_nodes): + self.log.info(f"Finished with node {i} balance: {self.nodes[i].getbalance()}") + assert_equal(self.nodes[from_idx].getbalance(), { "bitcoin": Decimal(0) }) + assert_equal(self.nodes[to_idx].getbalance(), { "bitcoin": amt }) + def run_test(self): # Mine 101 blocks to get the initial coins out of IBD self.nodes[0].generate(COINBASE_MATURITY + 1) @@ -57,16 +73,23 @@ def run_test(self): # Try to send those coins to yet another wallet, sending a large enough amount # that the change output is dropped. amt = satoshi_round(Decimal(0.9995)) - txid = self.nodes[1].sendtoaddress(self.nodes[2].getnewaddress(), amt) - self.log.info(f"Sent {amt} LBTC to node 2 in {txid}") - self.nodes[1].generate(2) - self.sync_all() + self.test_send(amt, 1, 2, True) - for i in range(self.num_nodes): - self.log.info(f"Finished with node {i} balance: {self.nodes[i].getbalance()}") + # Repeat, sending to a non-confidential output + amt = satoshi_round(Decimal(amt - Decimal(0.00035))) + self.test_send(amt, 2, 1, False) + + # Again, sending from non-confidential to non-confidential + amt = satoshi_round(Decimal(amt - Decimal(0.00033))) + self.test_send(amt, 1, 2, False) + + # Finally sending from non-confidential to confidential + amt = satoshi_round(Decimal(amt - Decimal(0.0005))) + self.test_send(amt, 2, 1, True) - assert_equal(self.nodes[1].getbalance(), { "bitcoin": Decimal(0) }) - assert_equal(self.nodes[2].getbalance(), { "bitcoin": amt }) + # Then send the coins again to make sure they're spendable + amt = satoshi_round(Decimal(amt - Decimal(0.0005))) + self.test_send(amt, 1, 2, True) if __name__ == '__main__': WalletCtTest().main() From 3c896b11e061edf39c3e07ee18bcf3b463d1e6c9 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 19 Sep 2022 21:40:30 +0000 Subject: [PATCH 06/14] test: rename `elements_regression_1172` to follow naming convention --- test/functional/test_runner.py | 2 +- ...ts_regression_1172.py => wallet_elements_regression_1172.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/functional/{elements_regression_1172.py => wallet_elements_regression_1172.py} (100%) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ef2c7865cf..c8f2703bdd 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -109,7 +109,7 @@ 'feature_initial_reissuance_token.py', 'feature_progress.py', 'rpc_getnewblockhex.py', - 'elements_regression_1172.py', + 'wallet_elements_regression_1172.py', # Longest test should go first, to favor running tests in parallel 'wallet_hd.py --legacy-wallet', 'wallet_hd.py --descriptors', diff --git a/test/functional/elements_regression_1172.py b/test/functional/wallet_elements_regression_1172.py similarity index 100% rename from test/functional/elements_regression_1172.py rename to test/functional/wallet_elements_regression_1172.py From e5e3ec2700a1f72351e2d227da68a7cc07c7285a Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 22 Sep 2022 13:11:07 +0000 Subject: [PATCH 07/14] wallet: account for issuances during coin selection Prior to coin selection we need to indicate that the issuances will take extra space, otherwise we may fail to select enough coins to cover our fees, triggering the new "fee needed exceeds fees available" assertion. --- src/wallet/spend.cpp | 22 ++++++++++++++ .../wallet_elements_regression_1172.py | 30 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 930e8ff204..3bd0ff9fc8 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1080,6 +1080,28 @@ bool CWallet::CreateTransactionInternal( // the blinding logic. coin_selection_params.tx_noinputs_size += 70 + 66 +(MAX_RANGEPROOF_SIZE + DEFAULT_SURJECTIONPROOF_SIZE + WITNESS_SCALE_FACTOR - 1)/WITNESS_SCALE_FACTOR; } + // If we are going to issue an asset, add the issuance data to the noinputs_size so that + // we allocate enough coins for them. + if (issuance_details) { + size_t issue_count = 0; + for (unsigned int i = 0; i < txNew.vout.size(); i++) { + if (txNew.vout[i].nAsset.IsExplicit() && txNew.vout[i].nAsset.GetAsset() == CAsset(uint256S("1"))) { + issue_count++; + } else if (txNew.vout[i].nAsset.IsExplicit() && txNew.vout[i].nAsset.GetAsset() == CAsset(uint256S("2"))) { + issue_count++; + } + } + if (issue_count > 0) { + // Allocate space for blinding nonce, entropy, and whichever of nAmount/nInflationKeys is null + coin_selection_params.tx_noinputs_size += 2 * 32 + 2 * (2 - issue_count); + } + // Allocate non-null nAmount/nInflationKeys and rangeproofs + if (issuance_details->blind_issuance) { + coin_selection_params.tx_noinputs_size += issue_count * (33 * WITNESS_SCALE_FACTOR + MAX_RANGEPROOF_SIZE + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; + } else { + coin_selection_params.tx_noinputs_size += issue_count * 9; + } + } // Include the fees for things that aren't inputs, excluding the change output const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); diff --git a/test/functional/wallet_elements_regression_1172.py b/test/functional/wallet_elements_regression_1172.py index e070a4b1e8..0c9afa6db9 100755 --- a/test/functional/wallet_elements_regression_1172.py +++ b/test/functional/wallet_elements_regression_1172.py @@ -91,6 +91,36 @@ def run_test(self): amt = satoshi_round(Decimal(amt - Decimal(0.0005))) self.test_send(amt, 1, 2, True) + addresses = [ self.nodes[1].getnewaddress() for i in range(15) ] \ + + [ self.nodes[2].getnewaddress() for i in range(15) ] + txid = self.nodes[2].sendmany(amounts={address: satoshi_round(Decimal(0.00025)) for address in addresses}) + self.log.info(f"Sent many small UTXOs to nodes 1 and 2 in {txid}") + self.nodes[2].generate(2) + self.sync_all() + + self.log.info(f"Issuing some assets from node 1") + # Try issuing assets + amt = satoshi_round(Decimal(1)) + res1 = self.nodes[1].issueasset(amt, amt, True); + res2 = self.nodes[1].issueasset(amt, amt, False); + + assets = [ res1["asset"], res1["token"], res2["asset"], res2["token"] ] + addresses = [ self.nodes[2].getnewaddress() for i in range(len(assets)) ] + txid = self.nodes[1].sendmany( + amounts={address: amt for address in addresses}, + output_assets={addresses[i]: assets[i] for i in range(len(assets))}, + ) + self.log.info(f"Sent them to node 2 in {txid}") + self.nodes[1].generate(2) + self.sync_all() + # Send them back + addresses = [ self.nodes[1].getnewaddress() for i in range(len(assets)) ] + txid = self.nodes[2].sendmany( + amounts={address: amt for address in addresses}, + output_assets={addresses[i]: assets[i] for i in range(len(assets))}, + ) + self.log.info(f"Sent them back to node 1 in {txid}") + if __name__ == '__main__': WalletCtTest().main() From 90c4bf99f6728866f57ae17120189a056c32e99c Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Mon, 26 Sep 2022 12:02:50 -0300 Subject: [PATCH 08/14] Fix lint --- test/functional/wallet_elements_regression_1172.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_elements_regression_1172.py b/test/functional/wallet_elements_regression_1172.py index 0c9afa6db9..25e27c582a 100755 --- a/test/functional/wallet_elements_regression_1172.py +++ b/test/functional/wallet_elements_regression_1172.py @@ -101,8 +101,8 @@ def run_test(self): self.log.info(f"Issuing some assets from node 1") # Try issuing assets amt = satoshi_round(Decimal(1)) - res1 = self.nodes[1].issueasset(amt, amt, True); - res2 = self.nodes[1].issueasset(amt, amt, False); + res1 = self.nodes[1].issueasset(amt, amt, True) + res2 = self.nodes[1].issueasset(amt, amt, False) assets = [ res1["asset"], res1["token"], res2["asset"], res2["token"] ] addresses = [ self.nodes[2].getnewaddress() for i in range(len(assets)) ] From 4553496c442d40319a3789e8c166b272236537b8 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 Sep 2022 17:40:47 +0000 Subject: [PATCH 09/14] wallet: don't clear out all the blinding data when dropping change The Elements 22 blinding logic has an edge case where when we drop change, leaving only a single blinded output, we recompute a bunch of blinding data to handle the potential for us to have 0 inputs and 1 output to blind. (BlindTransaction will fail in this case because it cannot make the transaction balance with only one output to mess with.) In this recomputation, we dropped more data than we meant to, causing us to incorrectly blind an output. (cherry picked from commit fac694be4c6539207cc771d549e8da4e5478b492) --- src/wallet/spend.cpp | 35 +++++----- test/functional/elements_regression_1172.py | 73 +++++++++++++++++++++ test/functional/test_runner.py | 1 + 3 files changed, 94 insertions(+), 15 deletions(-) create mode 100755 test/functional/elements_regression_1172.py diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 316ac45b35..00c9dfc90a 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -718,25 +718,29 @@ static uint32_t GetLocktimeForNewTransaction(interfaces::Chain& chain, const uin } // Reset all non-global blinding details. -void resetBlindDetails(BlindDetails* det) { +static void resetBlindDetails(BlindDetails* det, bool preserve_output_data = false) { det->i_amount_blinds.clear(); det->i_asset_blinds.clear(); det->i_assets.clear(); det->i_amounts.clear(); det->o_amounts.clear(); - det->o_pubkeys.clear(); + if (!preserve_output_data) { + det->o_pubkeys.clear(); + } det->o_amount_blinds.clear(); det->o_assets.clear(); det->o_asset_blinds.clear(); - det->num_to_blind = 0; - det->change_to_blind = 0; - det->only_recipient_blind_index = -1; - det->only_change_pos = -1; + if (!preserve_output_data) { + det->num_to_blind = 0; + det->change_to_blind = 0; + det->only_recipient_blind_index = -1; + det->only_change_pos = -1; + } } -bool fillBlindDetails(BlindDetails* det, CWallet* wallet, CMutableTransaction& txNew, std::vector& selected_coins, bilingual_str& error) { +static bool fillBlindDetails(BlindDetails* det, CWallet* wallet, CMutableTransaction& txNew, std::vector& selected_coins, bilingual_str& error) { int num_inputs_blinded = 0; // Fill in input blinding details @@ -1393,15 +1397,15 @@ bool CWallet::CreateTransactionInternal( blind_details->num_to_blind--; blind_details->change_to_blind--; - // FIXME: I promise this makes sense and fixes an actual problem - // with the wallet that users could encounter. But no human could - // follow the logic as to what this does or why it is safe. After - // the 22.0 rebase we need to double-back and replace the blinding - // logic to eliminate a bunch of edge cases and make this logic - // incomprehensible. But in the interest of minimizing diff during - // the rebase I am going to do this for now. + // FIXME: If we drop the change *and* this means we have only one + // blinded output *and* we have no blinded inputs, then this puts + // us in a situation where BlindTransaction will fail. This is + // prevented in fillBlindDetails, which adds an OP_RETURN output + // to handle this case. So do this ludicrous hack to accomplish + // this. This whole lump of un-followable-logic needs to be replaced + // by a complete rewriting of the wallet blinding logic. if (blind_details->num_to_blind == 1) { - resetBlindDetails(blind_details); + resetBlindDetails(blind_details, true /* don't wipe output data */); if (!fillBlindDetails(blind_details, this, txNew, selected_coins, error)) { return false; } @@ -1535,6 +1539,7 @@ bool CWallet::CreateTransactionInternal( int ret = BlindTransaction(blind_details->i_amount_blinds, blind_details->i_asset_blinds, blind_details->i_assets, blind_details->i_amounts, blind_details->o_amount_blinds, blind_details->o_asset_blinds, blind_details->o_pubkeys, issuance_asset_keys, issuance_token_keys, txNew); assert(ret != -1); if (ret != blind_details->num_to_blind) { + WalletLogPrintf("ERROR: tried to blind %d outputs but only blinded %d\n", (int) blind_details->num_to_blind, (int) ret); error = _("Unable to blind the transaction properly. This should not happen."); return false; } diff --git a/test/functional/elements_regression_1172.py b/test/functional/elements_regression_1172.py new file mode 100755 index 0000000000..79d48dde86 --- /dev/null +++ b/test/functional/elements_regression_1172.py @@ -0,0 +1,73 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test blinding logic when change is dropped and we have only one other blinded input + +Constructs a transaction with a sufficiently small change output that it +gets dropped, in which there is only one other blinded input. In the case +that we have no blinded inputs, we would need to add an OP_RETURN output +to the transaction, neccessitating special logic. + +Check that this special logic still results in a correct transaction that +sends the money to the desired recipient (and that the recipient is able +to receive/spend the money). +""" + +from decimal import Decimal + +from test_framework.blocktools import COINBASE_MATURITY +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + satoshi_round, +) + +class WalletCtTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 3 + self.extra_args = [[ + "-blindedaddresses=1", + "-initialfreecoins=2100000000000000", + "-con_blocksubsidy=0", + "-con_connect_genesis_outputs=1", + "-txindex=1", + ]] * self.num_nodes + self.extra_args[0].append("-anyonecanspendaremine=1") # first node gets the coins + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + # Mine 101 blocks to get the initial coins out of IBD + self.nodes[0].generate(COINBASE_MATURITY + 1) + self.nodes[0].syncwithvalidationinterfacequeue() + self.sync_all() + + for i in range(self.num_nodes): + self.log.info(f"Starting with node {i} balance: {self.nodes[i].getbalance()}") + + # Send 1 coin to a new wallet + txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1) + self.log.info(f"Sent one coin to node 1 in {txid}") + self.nodes[0].generate(2) + self.sync_all() + + # Try to send those coins to yet another wallet, sending a large enough amount + # that the change output is dropped. + amt = satoshi_round(Decimal(0.9995)) + txid = self.nodes[1].sendtoaddress(self.nodes[2].getnewaddress(), amt) + self.log.info(f"Sent {amt} LBTC to node 2 in {txid}") + self.nodes[1].generate(2) + self.sync_all() + + for i in range(self.num_nodes): + self.log.info(f"Finished with node {i} balance: {self.nodes[i].getbalance()}") + + assert_equal(self.nodes[1].getbalance(), { "bitcoin": Decimal(0) }) + assert_equal(self.nodes[2].getbalance(), { "bitcoin": amt }) + +if __name__ == '__main__': + WalletCtTest().main() + diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ab0268f922..ef2c7865cf 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -109,6 +109,7 @@ 'feature_initial_reissuance_token.py', 'feature_progress.py', 'rpc_getnewblockhex.py', + 'elements_regression_1172.py', # Longest test should go first, to favor running tests in parallel 'wallet_hd.py --legacy-wallet', 'wallet_hd.py --descriptors', From ce496535c4dc3013017e9b7d3b1a5a69d7ea25e6 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 20 Sep 2022 17:41:24 +0000 Subject: [PATCH 10/14] wallet: extend fix to "dropped change is the last blinded output" case (cherry picked from commit 79fd90f064fc7c09ab41bced0225c7618a527277) --- src/wallet/spend.cpp | 2 +- test/functional/elements_regression_1172.py | 39 ++++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 00c9dfc90a..930e8ff204 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1404,7 +1404,7 @@ bool CWallet::CreateTransactionInternal( // to handle this case. So do this ludicrous hack to accomplish // this. This whole lump of un-followable-logic needs to be replaced // by a complete rewriting of the wallet blinding logic. - if (blind_details->num_to_blind == 1) { + if (blind_details->num_to_blind < 2) { resetBlindDetails(blind_details, true /* don't wipe output data */); if (!fillBlindDetails(blind_details, this, txNew, selected_coins, error)) { return false; diff --git a/test/functional/elements_regression_1172.py b/test/functional/elements_regression_1172.py index 79d48dde86..e070a4b1e8 100755 --- a/test/functional/elements_regression_1172.py +++ b/test/functional/elements_regression_1172.py @@ -39,6 +39,22 @@ def set_test_params(self): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_send(self, amt, from_idx, to_idx, confidential): + # Try to send those coins to yet another wallet, sending a large enough amount + # that the change output is dropped. + address = self.nodes[to_idx].getnewaddress() + if not confidential: + address = self.nodes[to_idx].getaddressinfo(address)['unconfidential'] + txid = self.nodes[from_idx].sendtoaddress(address, amt) + self.log.info(f"Sent {amt} LBTC to node {to_idx} in {txid}") + self.nodes[from_idx].generate(2) + self.sync_all() + + for i in range(self.num_nodes): + self.log.info(f"Finished with node {i} balance: {self.nodes[i].getbalance()}") + assert_equal(self.nodes[from_idx].getbalance(), { "bitcoin": Decimal(0) }) + assert_equal(self.nodes[to_idx].getbalance(), { "bitcoin": amt }) + def run_test(self): # Mine 101 blocks to get the initial coins out of IBD self.nodes[0].generate(COINBASE_MATURITY + 1) @@ -57,16 +73,23 @@ def run_test(self): # Try to send those coins to yet another wallet, sending a large enough amount # that the change output is dropped. amt = satoshi_round(Decimal(0.9995)) - txid = self.nodes[1].sendtoaddress(self.nodes[2].getnewaddress(), amt) - self.log.info(f"Sent {amt} LBTC to node 2 in {txid}") - self.nodes[1].generate(2) - self.sync_all() + self.test_send(amt, 1, 2, True) - for i in range(self.num_nodes): - self.log.info(f"Finished with node {i} balance: {self.nodes[i].getbalance()}") + # Repeat, sending to a non-confidential output + amt = satoshi_round(Decimal(amt - Decimal(0.00035))) + self.test_send(amt, 2, 1, False) + + # Again, sending from non-confidential to non-confidential + amt = satoshi_round(Decimal(amt - Decimal(0.00033))) + self.test_send(amt, 1, 2, False) + + # Finally sending from non-confidential to confidential + amt = satoshi_round(Decimal(amt - Decimal(0.0005))) + self.test_send(amt, 2, 1, True) - assert_equal(self.nodes[1].getbalance(), { "bitcoin": Decimal(0) }) - assert_equal(self.nodes[2].getbalance(), { "bitcoin": amt }) + # Then send the coins again to make sure they're spendable + amt = satoshi_round(Decimal(amt - Decimal(0.0005))) + self.test_send(amt, 1, 2, True) if __name__ == '__main__': WalletCtTest().main() From a1e1afcda177cce76e3aae838562e611c5f32149 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 19 Sep 2022 21:40:30 +0000 Subject: [PATCH 11/14] test: rename `elements_regression_1172` to follow naming convention (cherry picked from commit 3c896b11e061edf39c3e07ee18bcf3b463d1e6c9) --- test/functional/test_runner.py | 2 +- ...ts_regression_1172.py => wallet_elements_regression_1172.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/functional/{elements_regression_1172.py => wallet_elements_regression_1172.py} (100%) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ef2c7865cf..c8f2703bdd 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -109,7 +109,7 @@ 'feature_initial_reissuance_token.py', 'feature_progress.py', 'rpc_getnewblockhex.py', - 'elements_regression_1172.py', + 'wallet_elements_regression_1172.py', # Longest test should go first, to favor running tests in parallel 'wallet_hd.py --legacy-wallet', 'wallet_hd.py --descriptors', diff --git a/test/functional/elements_regression_1172.py b/test/functional/wallet_elements_regression_1172.py similarity index 100% rename from test/functional/elements_regression_1172.py rename to test/functional/wallet_elements_regression_1172.py From 005a27cac722db1bc6515eed0a495fe275c53600 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Thu, 22 Sep 2022 13:11:07 +0000 Subject: [PATCH 12/14] wallet: account for issuances during coin selection Prior to coin selection we need to indicate that the issuances will take extra space, otherwise we may fail to select enough coins to cover our fees, triggering the new "fee needed exceeds fees available" assertion. (cherry picked from commit e5e3ec2700a1f72351e2d227da68a7cc07c7285a) --- src/wallet/spend.cpp | 22 ++++++++++++++ .../wallet_elements_regression_1172.py | 30 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 930e8ff204..3bd0ff9fc8 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1080,6 +1080,28 @@ bool CWallet::CreateTransactionInternal( // the blinding logic. coin_selection_params.tx_noinputs_size += 70 + 66 +(MAX_RANGEPROOF_SIZE + DEFAULT_SURJECTIONPROOF_SIZE + WITNESS_SCALE_FACTOR - 1)/WITNESS_SCALE_FACTOR; } + // If we are going to issue an asset, add the issuance data to the noinputs_size so that + // we allocate enough coins for them. + if (issuance_details) { + size_t issue_count = 0; + for (unsigned int i = 0; i < txNew.vout.size(); i++) { + if (txNew.vout[i].nAsset.IsExplicit() && txNew.vout[i].nAsset.GetAsset() == CAsset(uint256S("1"))) { + issue_count++; + } else if (txNew.vout[i].nAsset.IsExplicit() && txNew.vout[i].nAsset.GetAsset() == CAsset(uint256S("2"))) { + issue_count++; + } + } + if (issue_count > 0) { + // Allocate space for blinding nonce, entropy, and whichever of nAmount/nInflationKeys is null + coin_selection_params.tx_noinputs_size += 2 * 32 + 2 * (2 - issue_count); + } + // Allocate non-null nAmount/nInflationKeys and rangeproofs + if (issuance_details->blind_issuance) { + coin_selection_params.tx_noinputs_size += issue_count * (33 * WITNESS_SCALE_FACTOR + MAX_RANGEPROOF_SIZE + WITNESS_SCALE_FACTOR - 1) / WITNESS_SCALE_FACTOR; + } else { + coin_selection_params.tx_noinputs_size += issue_count * 9; + } + } // Include the fees for things that aren't inputs, excluding the change output const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); diff --git a/test/functional/wallet_elements_regression_1172.py b/test/functional/wallet_elements_regression_1172.py index e070a4b1e8..0c9afa6db9 100755 --- a/test/functional/wallet_elements_regression_1172.py +++ b/test/functional/wallet_elements_regression_1172.py @@ -91,6 +91,36 @@ def run_test(self): amt = satoshi_round(Decimal(amt - Decimal(0.0005))) self.test_send(amt, 1, 2, True) + addresses = [ self.nodes[1].getnewaddress() for i in range(15) ] \ + + [ self.nodes[2].getnewaddress() for i in range(15) ] + txid = self.nodes[2].sendmany(amounts={address: satoshi_round(Decimal(0.00025)) for address in addresses}) + self.log.info(f"Sent many small UTXOs to nodes 1 and 2 in {txid}") + self.nodes[2].generate(2) + self.sync_all() + + self.log.info(f"Issuing some assets from node 1") + # Try issuing assets + amt = satoshi_round(Decimal(1)) + res1 = self.nodes[1].issueasset(amt, amt, True); + res2 = self.nodes[1].issueasset(amt, amt, False); + + assets = [ res1["asset"], res1["token"], res2["asset"], res2["token"] ] + addresses = [ self.nodes[2].getnewaddress() for i in range(len(assets)) ] + txid = self.nodes[1].sendmany( + amounts={address: amt for address in addresses}, + output_assets={addresses[i]: assets[i] for i in range(len(assets))}, + ) + self.log.info(f"Sent them to node 2 in {txid}") + self.nodes[1].generate(2) + self.sync_all() + # Send them back + addresses = [ self.nodes[1].getnewaddress() for i in range(len(assets)) ] + txid = self.nodes[2].sendmany( + amounts={address: amt for address in addresses}, + output_assets={addresses[i]: assets[i] for i in range(len(assets))}, + ) + self.log.info(f"Sent them back to node 1 in {txid}") + if __name__ == '__main__': WalletCtTest().main() From fd9a1b67b9c4e22208604ae627a6295435b0d192 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Mon, 26 Sep 2022 12:02:50 -0300 Subject: [PATCH 13/14] Fix lint (cherry picked from commit 90c4bf99f6728866f57ae17120189a056c32e99c) --- test/functional/wallet_elements_regression_1172.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_elements_regression_1172.py b/test/functional/wallet_elements_regression_1172.py index 0c9afa6db9..25e27c582a 100755 --- a/test/functional/wallet_elements_regression_1172.py +++ b/test/functional/wallet_elements_regression_1172.py @@ -101,8 +101,8 @@ def run_test(self): self.log.info(f"Issuing some assets from node 1") # Try issuing assets amt = satoshi_round(Decimal(1)) - res1 = self.nodes[1].issueasset(amt, amt, True); - res2 = self.nodes[1].issueasset(amt, amt, False); + res1 = self.nodes[1].issueasset(amt, amt, True) + res2 = self.nodes[1].issueasset(amt, amt, False) assets = [ res1["asset"], res1["token"], res2["asset"], res2["token"] ] addresses = [ self.nodes[2].getnewaddress() for i in range(len(assets)) ] From 4aa849e0e9c71920e8fee969a538548c75aef436 Mon Sep 17 00:00:00 2001 From: Pablo Greco Date: Mon, 26 Sep 2022 11:51:02 -0300 Subject: [PATCH 14/14] Bump version to 22.0.1 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 7baca7ef31..a38759a884 100644 --- a/configure.ac +++ b/configure.ac @@ -1,7 +1,7 @@ AC_PREREQ([2.69]) define(_CLIENT_VERSION_MAJOR, 22) define(_CLIENT_VERSION_MINOR, 0) -define(_CLIENT_VERSION_BUILD, 0) +define(_CLIENT_VERSION_BUILD, 1) define(_CLIENT_VERSION_RC, 0) define(_CLIENT_VERSION_IS_RELEASE, true) define(_COPYRIGHT_YEAR, 2022)