foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-18 11:57:37 -05:00
Code Issues Activity

wallet: change FillPSBT to take sighash as optional

Browse source 
Instead of having the caller have to figure out the correct sane default
to provide to FillPSBT, have FillPSBT do that by having it take the
sighash type as an optional. This further allows it to distinguish
between an explicit sighash type being provided and expecting the
default value to be used.
This commit is contained in:
Ava Chow 2025-01-08 17:48:41 -05:00
parent 91d0c3173b
commit aabe989448
15 changed files with 34 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/interfaces/wallet.h
View file

@ -207,7 +207,7 @@ public:
int& num_blocks) = 0;
//! Fill PSBT.
virtual std::optional<common::PSBTError> fillPSBT(int sighash_type,
virtual std::optional<common::PSBTError> fillPSBT(std::optional<int> sighash_type,
bool sign,
bool bip32derivs,
size_t* n_signed,

6
src/qt/psbtoperationsdialog.cpp
View file

@ -59,7 +59,7 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx)
bool complete = FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness.
if (m_wallet_model) {
size_t n_could_sign;
const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)};
const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)};
if (err) {
showStatus(tr("Failed to load transaction: %1")
.arg(QString::fromStdString(PSBTErrorString(*err).translated)),
@ -83,7 +83,7 @@ void PSBTOperationsDialog::signTransaction()
WalletModel::UnlockContext ctx(m_wallet_model->requestUnlock());
const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)};
const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)};
if (err) {
showStatus(tr("Failed to sign transaction: %1")
@ -251,7 +251,7 @@ size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &p
size_t n_signed;
bool complete;
const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)};
const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)};
if (err) {
return 0;

6
src/qt/sendcoinsdialog.cpp
View file

@ -446,7 +446,7 @@ void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx)
bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) {
std::optional<PSBTError> err;
try {
err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
err = model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
} catch (const std::runtime_error& e) {
QMessageBox::critical(nullptr, tr("Sign failed"), e.what());
return false;
@ -503,7 +503,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
// Fill without signing
const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
assert(!complete);
assert(!err);
@ -519,7 +519,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
bool complete = false;
// Always fill without signing first. This prevents an external signer
// from being called prematurely and is not expensive.
const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
assert(!complete);
assert(!err);
send_failure = !signWithExternalSigner(psbtx, mtx, complete);

2
src/qt/walletmodel.cpp
View file

@ -535,7 +535,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
// "Create Unsigned" clicked
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
const auto err{wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)};
const auto err{wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)};
if (err || complete) {
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Can't draft transaction."));
return false;

2
src/wallet/external_signer_scriptpubkeyman.cpp
View file

@ -79,7 +79,7 @@ util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestin
}
// If sign is true, transaction must previously have been filled
std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
{
if (!sign) {
return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed, finalize);

2
src/wallet/external_signer_scriptpubkeyman.h
View file

@ -35,7 +35,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
*/
util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const;
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
};
} // namespace wallet
#endif // BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H

4
src/wallet/feebumper.cpp
View file

@ -343,8 +343,8 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
// First fill transaction with our data without signing,
// so external signers are not asked to sign more than once.
bool complete;
wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
auto err{wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */)};
wallet.FillPSBT(psbtx, complete, std::nullopt, false /* sign */, true /* bip32derivs */);
auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, true /* sign */, false /* bip32derivs */)};
if (err) return false;
complete = FinalizeAndExtractPSBT(psbtx, mtx);
return complete;

2
src/wallet/interfaces.cpp
View file

@ -391,7 +391,7 @@ public:
}
return {};
}
std::optional<PSBTError> fillPSBT(int sighash_type,
std::optional<PSBTError> fillPSBT(std::optional<int> sighash_type,
bool sign,
bool bip32derivs,
size_t* n_signed,

8
src/wallet/rpc/spend.cpp
View file

@ -103,8 +103,8 @@ static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const
// First fill transaction with our data without signing,
// so external signers are not asked to sign more than once.
bool complete;
pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true);
const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)};
pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true);
const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
if (err) {
throw JSONRPCPSBTError(*err);
}
@ -1169,7 +1169,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
} else {
PartiallySignedTransaction psbtx(mtx);
bool complete = false;
const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true)};
const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true)};
CHECK_NONFATAL(!err);
CHECK_NONFATAL(!complete);
DataStream ssTx{};
@ -1767,7 +1767,7 @@ RPCHelpMan walletcreatefundedpsbt()
// Fill transaction with out data but don't sign
bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
bool complete = true;
const auto err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
const auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
if (err) {
throw JSONRPCPSBTError(*err);
}

10
src/wallet/scriptpubkeyman.cpp
View file

@ -638,11 +638,12 @@ SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, con
return SigningResult::SIGNING_FAILED;
}
std::optional<PSBTError> LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
std::optional<PSBTError> LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
{
if (n_signed) {
*n_signed = 0;
}
if (!sighash_type) sighash_type = SIGHASH_DEFAULT;
for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
const CTxIn& txin = psbtx.tx->vin[i];
PSBTInput& input = psbtx.inputs.at(i);
@ -665,7 +666,7 @@ std::optional<PSBTError> LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransact
// There's no UTXO so we can just skip this now
continue;
}
PSBTError res = SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize);
PSBTError res = SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, &txdata, *sighash_type, nullptr, finalize);
if (res != PSBTError::OK && res != PSBTError::INCOMPLETE) {
return res;
}
@ -2548,11 +2549,12 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,
return SigningResult::OK;
}
std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
{
if (n_signed) {
*n_signed = 0;
}
if (!sighash_type) sighash_type = SIGHASH_DEFAULT;
for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
const CTxIn& txin = psbtx.tx->vin[i];
PSBTInput& input = psbtx.inputs.at(i);
@ -2623,7 +2625,7 @@ std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran
}
}
PSBTError res = SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize);
PSBTError res = SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, *sighash_type, nullptr, finalize);
if (res != PSBTError::OK && res != PSBTError::INCOMPLETE) {
return res;
}

6
src/wallet/scriptpubkeyman.h
View file

@ -246,7 +246,7 @@ public:
/** Sign a message with the given script */
virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; };
/** Adds script and derivation path information to a PSBT, and optionally signs it. */
virtual std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return common::PSBTError::UNSUPPORTED; }
virtual std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const { return common::PSBTError::UNSUPPORTED; }
virtual uint256 GetID() const { return uint256(); }
@ -492,7 +492,7 @@ public:
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
uint256 GetID() const override;
@ -681,7 +681,7 @@ public:
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const override;
SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override;
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = SIGHASH_DEFAULT, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, std::optional<int> sighash_type = std::nullopt, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override;
uint256 GetID() const override;

3
src/wallet/test/fuzz/scriptpubkeyman.cpp
View file

@ -188,7 +188,8 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
}
auto psbt{*opt_psbt};
const PrecomputedTransactionData txdata{PrecomputePSBTData(psbt)};
const int sighash_type{fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 150)};
std::optional<int> sighash_type{fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 151)};
if (sighash_type == 151) sighash_type = std::nullopt;
auto sign = fuzzed_data_provider.ConsumeBool();
auto bip32derivs = fuzzed_data_provider.ConsumeBool();
auto finalize = fuzzed_data_provider.ConsumeBool();

4
src/wallet/test/psbt_wallet_tests.cpp
View file

@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
// Fill transaction with our data
bool complete = true;
BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false, true));
BOOST_REQUIRE(!m_wallet.FillPSBT(psbtx, complete, std::nullopt, false, true));
// Get the final tx
DataStream ssTx{};
@ -77,7 +77,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
// Try to sign the mutated input
SignatureData sigdata;
BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true, true));
BOOST_CHECK(m_wallet.FillPSBT(psbtx, complete, std::nullopt, true, true));
}
BOOST_AUTO_TEST_CASE(parse_hd_keypath)

5
src/wallet/wallet.cpp
View file

@ -2197,11 +2197,12 @@ bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint,
return false;
}
std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, int sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const
std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& complete, std::optional<int> sighash_type, bool sign, bool bip32derivs, size_t * n_signed, bool finalize) const
{
if (n_signed) {
*n_signed = 0;
}
if (!sighash_type) sighash_type = SIGHASH_DEFAULT;
LOCK(cs_wallet);
// Get all of the previous transactions
for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
@ -2240,7 +2241,7 @@ std::optional<PSBTError> CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo
}
}
RemoveUnnecessaryTransactions(psbtx, sighash_type);
RemoveUnnecessaryTransactions(psbtx, *sighash_type);
// Complete if every input is now signed
complete = true;

2
src/wallet/wallet.h
View file

@ -663,7 +663,7 @@ public:
*/
std::optional<common::PSBTError> FillPSBT(PartiallySignedTransaction& psbtx,
bool& complete,
int sighash_type = SIGHASH_DEFAULT,
std::optional<int> sighash_type = std::nullopt,
bool sign = true,
bool bip32derivs = true,
size_t* n_signed = nullptr,