Introduce constructors for legacy input pairs#753
Conversation
Pull Request Test Coverage Report for Build 15526167253Details
💛 - Coveralls |
|
From this comment from the payjoin-cli wallet code:
The existing code is populating the
The constructors I created here had the spec in mind and populate the |
spacebear21
left a comment
There was a problem hiding this comment.
The constructors I created here had the spec in mind and populate the non_witness_utxo of a PSBT, but should these instead be populating the witness_utxo field?
This sounds like the correct approach. IIRC the reason we just provided witness_utxo for non-witness inputs is just that it was there for us to use in core RPC responses, which removed the need to make an extra RPC call to get the previous Transaction, but that was a shortcut.
spacebear21
left a comment
There was a problem hiding this comment.
ACK aa4e9d3
I left two minor comments that can be addressed in follow-ups.
| let redeem_script = if let Some(ref script_sig) = self.psbtin.final_script_sig { | ||
| script_sig.redeem_script() | ||
| // try the PSBT redeem_script field for unsigned inputs. | ||
| // try the PSBT redeem_script field for unsigned inputs. |
There was a problem hiding this comment.
It looks like this comment belongs in the else block?
| Self::InvalidP2wpkhScriptPubkey => write!(f, "provided script pubkey is invalid for P2WPKH"), | ||
| Self::InvalidP2wshScriptPubkey => write!(f, "provided script pubkey is invalid for P2WSH"), | ||
| Self::InvalidP2trScriptPubkey => write!(f, "provided script pubkey is invalid for P2TR"), | ||
| Self::InvalidScriptPubKey(e) => write!(f, "provided script was not a valid type of {e}") |
There was a problem hiding this comment.
nit:
| Self::InvalidScriptPubKey(e) => write!(f, "provided script was not a valid type of {e}") | |
| Self::InvalidScriptPubKey(type) => write!(f, "provided script was not a valid for {type}") |
Checking off some remaining tasks in #583 - this PR introduces a constructor for each legacy type