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.

Sign up for GitHub

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

Jump to bottom

Check MAX_BLOCK_SIGOPS in the block verifier #3049

Merged
merged 12 commits into from
Nov 15, 2021
Merged

Check MAX_BLOCK_SIGOPS in the block verifier #3049

merged 12 commits into from
Nov 15, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Nov 11, 2021 •
edited
Loading

Motivation

Zebra needs to check the MAX_BLOCK_SIGOPS consensus rule.

Solution

Zebra:

  • Add legacy sigops counts to zebra-script
  • Return legacy sigops in transaction verifier responses
  • Check legacy sigops in the block verifier
  • Test MAX_BLOCK_SIGOPS on large generated blocks and historical blocks

Dependency updates:

Closes #3015

Dependency Stability

I tagged the zcash_script commit that Zebra depends on as:
https://github.com/ZcashFoundation/zcash_script/releases/tag/v0.1.6-alpha.1-zebra-v1.0.0-beta.0

I tagged the zcashd commit in the submodule as:
https://github.com/ZcashFoundation/zcashd/releases/tag/v4.5.1-1-zcash-script-zebra-v1.0.0-beta.0

These tags will make sure we don't accidentally delete the commits we're depending on.

Review

@jvff did the zcash_script and Zebra reviews, so @oxarbitrage can do the zcashd review.

Reviewer Checklist

  • Unsafe code has up-to-date safety comments
  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

@teor2345 teor2345 added A-dependencies Area: Dependency file updates A-consensus Area: Consensus rule updates NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter) P-Medium labels Nov 11, 2021
@teor2345 teor2345 self-assigned this Nov 11, 2021
@zfnd-bot zfnd-bot bot added this to In progress in 🦓 Nov 11, 2021
@teor2345 teor2345 changed the title Script sigops Check MAX_BLOCK_SIGOPS in the block verifier Nov 11, 2021
jvff
jvff previously approved these changes Nov 12, 2021
View reviewed changes
Copy link
Contributor

@jvff jvff left a comment

Choose a reason for hiding this comment

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

Looks good, I just added some optional suggestions.

zebra-test/src/prelude.rs Show resolved Hide resolved
zebra-consensus/Cargo.toml Outdated Show resolved Hide resolved
zebra-script/src/lib.rs Outdated Show resolved Hide resolved
zebra-script/src/lib.rs Outdated Show resolved Hide resolved
zebra-script/src/lib.rs Outdated Show resolved Hide resolved
@teor2345 teor2345 requested review from jvff and removed request for oxarbitrage November 15, 2021 01:31
@teor2345 teor2345 enabled auto-merge (squash) November 15, 2021 20:10
@teor2345 teor2345 merged commit 1df3bdb into main Nov 15, 2021
@teor2345 teor2345 deleted the script-sigops branch November 15, 2021 20:55
🦓 automation moved this from In progress to Done Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jvff jvff jvff approved these changes

Assignees

@teor2345 teor2345

Labels
A-consensus Area: Consensus rule updates A-dependencies Area: Dependency file updates NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter)
Projects
No open projects
🦓
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Validate transparent signature operation limits in blocks
2 participants
@teor2345 @jvff