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

[WIP] Transaction verifier #1100

Closed
wants to merge 19 commits into from
Closed

[WIP] Transaction verifier #1100

wants to merge 19 commits into from

Conversation

dconnolly
Copy link
Contributor

This is a high-level sketch of what our TransactionVerifier should do. Right now it's calling signature batch verifiers that we may not want to use anymore, and (Spend|Output|JoinSplit)Verifiers that don't exist yet, and is incomplete in all its checks and outputs, but you get the idea.

@dconnolly dconnolly added A-consensus Area: Consensus rule updates NU-1 Sapling Network Upgrade: Sapling specific tasks NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) A-rust Area: Updates to Rust code S-needs-design Status: Needs a design decision labels Sep 25, 2020
@dconnolly dconnolly mentioned this pull request Sep 25, 2020
27 tasks
Comment on lines +27 to +35
/// The different SigHash types, as defined in https://zips.z.cash/zip-0143
pub struct HashType: u32 {
///
const ALL = 0b0000_0001;
///
const NONE = 0b0000_0010;
///
const SINGLE = Self::ALL.bits | Self::NONE.bits;
///
Copy link
Contributor

Choose a reason for hiding this comment

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

The empty doc comments prevent a linter from ever noticing that they're empty, so I think it would be preferable to avoid adding them.

@dconnolly dconnolly added this to the Validate transactions. milestone Sep 28, 2020
@dconnolly dconnolly force-pushed the transaction-verifier branch 2 times, most recently from 9c05425 to eca674a Compare October 8, 2020 20:55
@yaahc yaahc force-pushed the transaction-verifier branch from 8111e69 to 91c8013 Compare October 14, 2020 21:12
@hdevalence
Copy link
Contributor

We rebased this onto #961.

Comment on lines +77 to +78
/// transaction transfers no money
NoTransfer,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a misleading name, as it suggests that the consensus rules verify that zero-value coins and notes cannot be spent. The actual rules that lead to this error are only checking for the presence of inputs / spends.

@hdevalence
Copy link
Contributor

Hi all, just a heads' up that this PR will be rebased and split into the part that sets up the plumbing for the transaction verification service, and the part that adds the consensus rules, which will be split off into separate PRs. I'm hoping to get this done in the next few hours.

@dconnolly
Copy link
Contributor Author

Closing as this has now been supplanted by #1173 and #1174

@dconnolly dconnolly closed this Oct 21, 2020
@dconnolly dconnolly deleted the transaction-verifier branch October 21, 2020 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) NU-1 Sapling Network Upgrade: Sapling specific tasks S-needs-design Status: Needs a design decision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants