Skip to content

Commit d6001dc

Browse files
achow101achow101
committed
wallet: change FillPSBT to take sighash as optional
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.
1 parent e58b680 commit d6001dc

15 files changed

+30
-27
lines changed

‎src/interfaces/wallet.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ class Wallet
204204
int& num_blocks) = 0;
205205

206206
//! Fill PSBT.
207-
virtual std::optional<common::PSBTError> fillPSBT(int sighash_type,
207+
virtual std::optional<common::PSBTError> fillPSBT(std::optional<int> sighash_type,
208208
bool sign,
209209
bool bip32derivs,
210210
size_t* n_signed,

‎src/qt/psbtoperationsdialog.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx)
5959
bool complete = FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness.
6060
if (m_wallet_model) {
6161
size_t n_could_sign;
62-
const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)};
62+
const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, &n_could_sign, m_transaction_data, complete)};
6363
if (err) {
6464
showStatus(tr("Failed to load transaction: %1")
6565
.arg(QString::fromStdString(PSBTErrorString(*err).translated)),
@@ -83,7 +83,7 @@ void PSBTOperationsDialog::signTransaction()
8383

8484
WalletModel::UnlockContext ctx(m_wallet_model->requestUnlock());
8585

86-
const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)};
86+
const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, &n_signed, m_transaction_data, complete)};
8787

8888
if (err) {
8989
showStatus(tr("Failed to sign transaction: %1")
@@ -251,7 +251,7 @@ size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &p
251251

252252
size_t n_signed;
253253
bool complete;
254-
const auto err{m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)};
254+
const auto err{m_wallet_model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/false, &n_signed, m_transaction_data, complete)};
255255

256256
if (err) {
257257
return 0;

‎src/qt/sendcoinsdialog.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ void SendCoinsDialog::presentPSBT(PartiallySignedTransaction& psbtx)
450450
bool SendCoinsDialog::signWithExternalSigner(PartiallySignedTransaction& psbtx, CMutableTransaction& mtx, bool& complete) {
451451
std::optional<PSBTError> err;
452452
try {
453-
err = model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
453+
err = model->wallet().fillPSBT(std::nullopt, /*sign=*/true, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete);
454454
} catch (const std::runtime_error& e) {
455455
QMessageBox::critical(nullptr, tr("Sign failed"), e.what());
456456
return false;
@@ -507,7 +507,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
507507
PartiallySignedTransaction psbtx(mtx);
508508
bool complete = false;
509509
// Fill without signing
510-
const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
510+
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
511511
assert(!complete);
512512
assert(!err);
513513

@@ -523,7 +523,7 @@ void SendCoinsDialog::sendButtonClicked([[maybe_unused]] bool checked)
523523
bool complete = false;
524524
// Always fill without signing first. This prevents an external signer
525525
// from being called prematurely and is not expensive.
526-
const auto err{model->wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
526+
const auto err{model->wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, /*n_signed=*/nullptr, psbtx, complete)};
527527
assert(!complete);
528528
assert(!err);
529529
send_failure = !signWithExternalSigner(psbtx, mtx, complete);

‎src/qt/walletmodel.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
519519
// "Create Unsigned" clicked
520520
PartiallySignedTransaction psbtx(mtx);
521521
bool complete = false;
522-
const auto err{wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)};
522+
const auto err{wallet().fillPSBT(std::nullopt, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete)};
523523
if (err || complete) {
524524
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Can't draft transaction."));
525525
return false;

‎src/wallet/external_signer_scriptpubkeyman.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ util::Result<void> ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestin
7979
}
8080

8181
// If sign is true, transaction must previously have been filled
82-
std::optional<PSBTError> ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
82+
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
8383
{
8484
if (!sign) {
8585
return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed, finalize);

‎src/wallet/external_signer_scriptpubkeyman.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan
3535
*/
3636
util::Result<void> DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const;
3737

38-
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;
38+
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;
3939
};
4040
} // namespace wallet
4141
#endif // BITCOIN_WALLET_EXTERNAL_SIGNER_SCRIPTPUBKEYMAN_H

‎src/wallet/feebumper.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,8 +338,8 @@ bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
338338
// First fill transaction with our data without signing,
339339
// so external signers are not asked to sign more than once.
340340
bool complete;
341-
wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
342-
auto err{wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */)};
341+
wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true);
342+
auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
343343
if (err) return false;
344344
complete = FinalizeAndExtractPSBT(psbtx, mtx);
345345
return complete;

‎src/wallet/interfaces.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ class WalletImpl : public Wallet
383383
}
384384
return {};
385385
}
386-
std::optional<PSBTError> fillPSBT(int sighash_type,
386+
std::optional<PSBTError> fillPSBT(std::optional<int> sighash_type,
387387
bool sign,
388388
bool bip32derivs,
389389
size_t* n_signed,

‎src/wallet/rpc/spend.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const
100100
// First fill transaction with our data without signing,
101101
// so external signers are not asked to sign more than once.
102102
bool complete;
103-
pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true);
104-
const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)};
103+
pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true);
104+
const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/true, /*bip32derivs=*/false)};
105105
if (err) {
106106
throw JSONRPCPSBTError(*err);
107107
}
@@ -1175,7 +1175,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
11751175
} else {
11761176
PartiallySignedTransaction psbtx(mtx);
11771177
bool complete = false;
1178-
const auto err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true)};
1178+
const auto err{pwallet->FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/true)};
11791179
CHECK_NONFATAL(!err);
11801180
CHECK_NONFATAL(!complete);
11811181
DataStream ssTx{};
@@ -1777,7 +1777,7 @@ RPCHelpMan walletcreatefundedpsbt()
17771777
// Fill transaction with out data but don't sign
17781778
bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
17791779
bool complete = true;
1780-
const auto err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
1780+
const auto err{wallet.FillPSBT(psbtx, complete, std::nullopt, /*sign=*/false, /*bip32derivs=*/bip32derivs)};
17811781
if (err) {
17821782
throw JSONRPCPSBTError(*err);
17831783
}

‎src/wallet/scriptpubkeyman.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,11 +1311,12 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message,
13111311
return SigningResult::OK;
13121312
}
13131313

1314-
std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed, bool finalize) const
1314+
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
13151315
{
13161316
if (n_signed) {
13171317
*n_signed = 0;
13181318
}
1319+
if (!sighash_type) sighash_type = SIGHASH_DEFAULT;
13191320
for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) {
13201321
const CTxIn& txin = psbtx.tx->vin[i];
13211322
PSBTInput& input = psbtx.inputs.at(i);
@@ -1386,7 +1387,7 @@ std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran
13861387
}
13871388
}
13881389

1389-
PSBTError res = SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize);
1390+
PSBTError res = SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, *sighash_type, nullptr, finalize);
13901391
if (res != PSBTError::OK && res != PSBTError::INCOMPLETE) {
13911392
return res;
13921393
}

0 commit comments

Comments
 (0)