LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Refactor PSBTInput signing to enforce invariant

Browse source 
Refactor the process of PSBTInput signing to enforce the invariant that
a PSBTInput always has _either_ a witness_utxo or a non_witness_utxo,
never both.

This simplifies the logic of SignPSBTInput slightly, since it no longer
has to deal with the "both" case. When calling it, we now give it, in
order of preference: (1) whichever of the utxo fields was already
present in the PSBT we received, or (2) if neither, the
non_witness_utxo field, which is just a copy of the input transaction,
which we get from the wallet.

SignPSBTInput no longer has to remove one of the two fields; instead, it
will check if we have a witness signature, and if so, it will replace
the non_witness_utxo with the witness_utxo (which is smaller, as it is
just a copy of the output being spent.)

Add PSBTInput::IsSane checks in two more places, which checks for
both utxo fields being present; we will now give an RPC error early on
if we are supplied such a malformed PSBT to fill in.

Also add a check to FillPSBT, to avoid touching any input that is
already signed. (This is now redundant, since we should no longer
potentially harm an already-signed input, but it's harmless.)

fixes #14473

Github-Pull: #14588
This commit is contained in:
Glenn Willen 2018-10-26 15:31:41 -07:00 committed by Pieter Wuille
parent ad94165db9
commit db445d4e5a
2 changed files with 37 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/script/sign.cpp
View file

@ -253,15 +253,19 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction&
// Get UTXO
bool require_witness_sig = false;
CTxOut utxo;
// Verify input sanity, which checks that at most one of witness or non-witness utxos is provided.
if (!input.IsSane()) {
return false;
}
if (input.non_witness_utxo) {
// If we're taking our information from a non-witness UTXO, verify that it matches the prevout.
if (input.non_witness_utxo->GetHash() != tx.vin[index].prevout.hash) return false;
// If both witness and non-witness UTXO are provided, verify that they match. This check shouldn't
// matter, as the PSBT deserializer enforces only one of both is provided, and the only way both
// can be present is when they're added simultaneously by FillPSBT (in which case they always match).
// Still, check in order to not rely on callers to enforce this.
if (!input.witness_utxo.IsNull() && input.non_witness_utxo->vout[tx.vin[index].prevout.n] != input.witness_utxo) return false;
utxo = input.non_witness_utxo->vout[tx.vin[index].prevout.n];
COutPoint prevout = tx.vin[index].prevout;
if (input.non_witness_utxo->GetHash() != prevout.hash) {
return false;
}
utxo = input.non_witness_utxo->vout[prevout.n];
} else if (!input.witness_utxo.IsNull()) {
utxo = input.witness_utxo;
// When we're taking our information from a witness UTXO, we can't verify it is actually data from
@ -279,6 +283,13 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction&
// Verify that a witness signature was produced in case one was required.
if (require_witness_sig && !sigdata.witness) return false;
input.FromSignatureData(sigdata);
// If we have a witness signature, use the smaller witness UTXO.
if (sigdata.witness) {
input.witness_utxo = utxo;
input.non_witness_utxo = nullptr;
}
return sig_complete;
}

27
src/wallet/rpcwallet.cpp
View file

@ -4511,15 +4511,25 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sig
const CTxIn& txin = psbtx.tx->vin[i];
PSBTInput& input = psbtx.inputs.at(i);
// If we don't know about this input, skip it and let someone else deal with it
if (PSBTInputSigned(input)) {
continue;
}
// Verify input looks sane. This will check that we have at most one uxto, witness or non-witness.
if (!input.IsSane()) {
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "PSBT input is not sane.");
}
// If we have no utxo, grab it from the wallet.
if (!input.non_witness_utxo && input.witness_utxo.IsNull()) {
const uint256& txhash = txin.prevout.hash;
const auto it = pwallet->mapWallet.find(txhash);
if (it != pwallet->mapWallet.end()) {
const CWalletTx& wtx = it->second;
CTxOut utxo = wtx.tx->vout[txin.prevout.n];
// Update both UTXOs from the wallet.
// We only need the non_witness_utxo, which is a superset of the witness_utxo.
// The signing code will switch to the smaller witness_utxo if this is ok.
input.non_witness_utxo = wtx.tx;
input.witness_utxo = utxo;
}
}
// Get the Sighash type
@ -4541,15 +4551,6 @@ bool FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& psbtx, int sig
}
}
// If both UTXO types are present, drop the unnecessary one.
if (input.non_witness_utxo && !input.witness_utxo.IsNull()) {
if (sigdata.witness) {
input.non_witness_utxo = nullptr;
} else {
input.witness_utxo.SetNull();
}
}
// Get public key paths
if (bip32derivs) {
for (const auto& pubkey_it : sigdata.misc_pubkeys) {