diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -649,6 +649,43 @@ !wallet->GetNewDestination(OutputType::LEGACY, "", dest, error)); } +// Explicit calculation which is used to test the wallet constant +static size_t CalculateP2PKHInputSize(bool use_max_sig) { + // Generate ephemeral valid pubkey + CKey key; + key.MakeNewKey(true); + CPubKey pubkey = key.GetPubKey(); + + // Generate pubkey hash + PKHash key_hash(pubkey); + + // Create script to enter into keystore. Key hash can't be 0... + CScript script = GetScriptForDestination(key_hash); + + // Add script to key store and key to watchonly + FillableSigningProvider keystore; + keystore.AddKeyPubKey(key, pubkey); + + // Fill in dummy signatures for fee calculation. + SignatureData sig_data; + if (!ProduceSignature(keystore, + use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR + : DUMMY_SIGNATURE_CREATOR, + script, sig_data)) { + // We're hand-feeding it correct arguments; shouldn't happen + assert(false); + } + + CTxIn tx_in; + UpdateInput(tx_in, sig_data); + return (size_t)GetVirtualTransactionInputSize(tx_in); +} + +BOOST_FIXTURE_TEST_CASE(dummy_input_size_test, TestChain100Setup) { + BOOST_CHECK(CalculateP2PKHInputSize(false) <= DUMMY_P2PKH_INPUT_SIZE); + BOOST_CHECK_EQUAL(CalculateP2PKHInputSize(true), DUMMY_P2PKH_INPUT_SIZE); +} + bool malformed_descriptor(std::ios_base::failure e) { std::string s(e.what()); return s.find("Missing checksum") != std::string::npos; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -107,6 +107,8 @@ //! -maxtxfee will warn if called with a higher fee than this amount (in //! satoshis) constexpr Amount HIGH_MAX_TX_FEE{100 * HIGH_TX_FEE_PER_KB}; +//! Pre-calculated constants for input size estimation +static constexpr size_t DUMMY_P2PKH_INPUT_SIZE = 148; class CChainParams; class CCoinControl; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1647,8 +1647,6 @@ CMutableTransaction txn; txn.vin.push_back(CTxIn(COutPoint())); if (!wallet->DummySignInput(txn.vin[0], txout, use_max_sig)) { - // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) - // implies that we can sign for every input. return -1; } return GetSerializeSize(txn.vin[0], PROTOCOL_VERSION); @@ -3160,9 +3158,17 @@ if (pick_new_inputs) { nValueIn = Amount::zero(); setCoins.clear(); - coin_selection_params.change_spend_size = - CalculateMaximumSignedInputSize(change_prototype_txout, - this); + int change_spend_size = CalculateMaximumSignedInputSize( + change_prototype_txout, this); + // If the wallet doesn't know how to sign change output, assume + // p2pkh as lower-bound to allow BnB to do it's thing + if (change_spend_size == -1) { + coin_selection_params.change_spend_size = + DUMMY_P2PKH_INPUT_SIZE; + } else { + coin_selection_params.change_spend_size = + size_t(change_spend_size); + } coin_selection_params.effective_fee = nFeeRateNeeded; if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params, diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -189,8 +189,18 @@ self.nodes[0].generate(6) self.sync_all() + block_height = self.nodes[0].getblockcount() unspent = self.nodes[0].listunspent()[0] + # Make sure change address wallet does not have P2SH innerscript access to results in success + # when attempting BnB coin selection + self.nodes[0].walletcreatefundedpsbt( + [], + [{self.nodes[2].getnewaddress():unspent["amount"] + 1}], + block_height + 2, + {"changeAddress": self.nodes[1].getnewaddress()}, + False) + # Regression test for 14473 (mishandling of already-signed # transaction): psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid": unspent["txid"], "vout":unspent["vout"]}], [