Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add verify_with_flags to Script and Transaction #598

Merged
merged 3 commits into from May 5, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 13 additions & 6 deletions src/blockdata/script.rs
Expand Up @@ -449,13 +449,20 @@ impl Script {
}

#[cfg(feature="bitcoinconsensus")]
/// verify spend of an input script
/// # Parameters
/// * index - the input index in spending which is spending this transaction
/// * amount - the amount this script guards
/// * spending - the transaction that attempts to spend the output holding this script
/// Shorthand for [Self::verify_with_flags] with flag [bitcoinconsensus::VERIFY_ALL]
pub fn verify (&self, index: usize, amount: u64, spending: &[u8]) -> Result<(), Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be Amount? (especially once we adopt wider Amount use like in #599)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep this non API breaking and not change yet the verify method signature, but I changed the amount type in verify_with_flags in 69117a1

Ok(bitcoinconsensus::verify (&self.0[..], amount, spending, index)?)
self.verify_with_flags(index, amount, spending, ::bitcoinconsensus::VERIFY_ALL)
}

#[cfg(feature="bitcoinconsensus")]
/// Verify spend of an input script
/// # Parameters
/// * `index` - the input index in spending which is spending this transaction
/// * `amount` - the amount this script guards
/// * `spending` - the transaction that attempts to spend the output holding this script
/// * `flags` - verification flags, see [bitcoinconsensus::VERIFY_ALL] and similar
pub fn verify_with_flags<F: Into<u32>>(&self, index: usize, amount: u64, spending: &[u8], flags: F) -> Result<(), Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ibid

Suggested change
pub fn verify_with_flags<F: Into<u32>>(&self, index: usize, amount: u64, spending: &[u8], flags: F) -> Result<(), Error> {
pub fn verify_with_flags<F: Into<u32>>(&self, index: usize, amount: Amount, spending: &[u8], flags: F) -> Result<(), Error> {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed in 69117a1

Ok(bitcoinconsensus::verify_with_flags (&self.0[..], amount, spending, index, flags.into())?)
}

/// Write the assembly decoding of the script bytes to the formatter.
Expand Down
13 changes: 10 additions & 3 deletions src/blockdata/transaction.rs
Expand Up @@ -462,15 +462,22 @@ impl Transaction {
}
}

#[cfg(feature="bitcoinconsensus")]
/// Shorthand for [Self::verify_with_flags] with flag [bitcoinconsensus::VERIFY_ALL]
pub fn verify<S>(&self, spent: S) -> Result<(), script::Error>
where S: FnMut(&OutPoint) -> Option<TxOut> {
self.verify_with_flags(spent, ::bitcoinconsensus::VERIFY_ALL)
}

#[cfg(feature="bitcoinconsensus")]
/// Verify that this transaction is able to spend its inputs
/// The lambda spent should not return the same TxOut twice!
pub fn verify<S>(&self, mut spent: S) -> Result<(), script::Error>
where S: FnMut(&OutPoint) -> Option<TxOut> {
pub fn verify_with_flags<S, F>(&self, mut spent: S, flags: F) -> Result<(), script::Error>
where S: FnMut(&OutPoint) -> Option<TxOut>, F : Into<u32> + Copy {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we need to require flags to be Copy here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason was that flags is used in a for, but yeah I solved by creating the u32 before in d1f4c0a

let tx = encode::serialize(&*self);
for (idx, input) in self.input.iter().enumerate() {
if let Some(output) = spent(&input.previous_output) {
output.script_pubkey.verify(idx, output.value, tx.as_slice())?;
output.script_pubkey.verify_with_flags(idx, output.value, tx.as_slice(), flags)?;
} else {
return Err(script::Error::UnknownSpentOutput(input.previous_output.clone()));
}
Expand Down