Merge #14851: [backport] fix assert crash when specified change output spend size is unknown
2a5cc40dc4
CreateTransaction: Assume minimum p2sh-p2wpkh spend size for unknown change (Gregory Sanders)53dcf2b407
Remove stale comment in CalculateMaximumSignedInputSize (Gregory Sanders) Pull request description: backport of #14380 Tree-SHA512: 42e261bd797d1938f8e041ccd10073ecd1d72695e2e4ce322e5a3ce262647e32108b01dde73361b6d2ac36438522ab3c4cd58ca072194f25011132437430cd27
This commit is contained in:
commit
924cf794e1
4 changed files with 59 additions and 3 deletions
|
@ -16,6 +16,7 @@
|
|||
#include <validation.h>
|
||||
#include <wallet/coincontrol.h>
|
||||
#include <wallet/test/wallet_test_fixture.h>
|
||||
#include <policy/policy.h>
|
||||
|
||||
#include <boost/test/unit_test.hpp>
|
||||
#include <univalue.h>
|
||||
|
@ -374,4 +375,47 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
|
|||
BOOST_CHECK(!wallet->GetKeyFromPool(pubkey, false));
|
||||
}
|
||||
|
||||
// Explicit calculation which is used to test the wallet constant
|
||||
// We get the same virtual size due to rounding(weight/4) for both use_max_sig values
|
||||
static size_t CalculateNestedKeyhashInputSize(bool use_max_sig)
|
||||
{
|
||||
// Generate ephemeral valid pubkey
|
||||
CKey key;
|
||||
key.MakeNewKey(true);
|
||||
CPubKey pubkey = key.GetPubKey();
|
||||
|
||||
// Generate pubkey hash
|
||||
uint160 key_hash(Hash160(pubkey.begin(), pubkey.end()));
|
||||
|
||||
// Create inner-script to enter into keystore. Key hash can't be 0...
|
||||
CScript inner_script = CScript() << OP_0 << std::vector<unsigned char>(key_hash.begin(), key_hash.end());
|
||||
|
||||
// Create outer P2SH script for the output
|
||||
uint160 script_id(Hash160(inner_script.begin(), inner_script.end()));
|
||||
CScript script_pubkey = CScript() << OP_HASH160 << std::vector<unsigned char>(script_id.begin(), script_id.end()) << OP_EQUAL;
|
||||
|
||||
// Add inner-script to key store and key to watchonly
|
||||
CBasicKeyStore keystore;
|
||||
keystore.AddCScript(inner_script);
|
||||
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_pubkey, 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_EQUAL(CalculateNestedKeyhashInputSize(false), DUMMY_NESTED_P2WPKH_INPUT_SIZE);
|
||||
BOOST_CHECK_EQUAL(CalculateNestedKeyhashInputSize(true), DUMMY_NESTED_P2WPKH_INPUT_SIZE);
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
|
|
|
@ -1607,8 +1607,6 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet,
|
|||
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 GetVirtualTransactionInputSize(txn.vin[0]);
|
||||
|
@ -2832,7 +2830,14 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
|
|||
if (pick_new_inputs) {
|
||||
nValueIn = 0;
|
||||
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 p2sh-p2wpkh
|
||||
// as lower-bound to allow BnB to do it's thing
|
||||
if (change_spend_size == -1) {
|
||||
coin_selection_params.change_spend_size = DUMMY_NESTED_P2WPKH_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, bnb_used))
|
||||
{
|
||||
|
|
|
@ -64,6 +64,9 @@ static const bool DEFAULT_WALLET_RBF = false;
|
|||
static const bool DEFAULT_WALLETBROADCAST = true;
|
||||
static const bool DEFAULT_DISABLE_WALLET = false;
|
||||
|
||||
//! Pre-calculated constants for input size estimation in *virtual size*
|
||||
static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91;
|
||||
|
||||
class CBlockIndex;
|
||||
class CCoinControl;
|
||||
class COutput;
|
||||
|
|
|
@ -169,6 +169,10 @@ class PSBTTest(BitcoinTestFramework):
|
|||
assert_equal(decoded_psbt["tx"]["locktime"], 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)
|
||||
|
||||
# BIP 174 Test Vectors
|
||||
|
||||
# Check that unknown values are just passed through
|
||||
|
|
Loading…
Reference in a new issue