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

Check nSpendsSapling, nOutputsSapling, and nActionsOrchard 2^16 limit #3069

Merged
merged 4 commits into from
Nov 18, 2021

Conversation

conradoplg
Copy link
Collaborator

Motivation

The spec says

[NU5 onward] nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16.

[NU5 onward] The rule that nSpendsSapling, nOutputsSapling, and nActionsOrchard MUST all be less than 2^16, is technically redundant because a transaction that could violate this rule would not fit within the 2 MB block size limit. It is included in order to simplify the security argument for balance preservation.

While the check is redudant, it seems useful to add a sanity check to make sure it's being enforced.

Specifications

Designs

Solution

Add static assertions to the current limits (which are derived from the block size) to make sure they are not greater than 2^16.

Closes #2379

Review

Anyone can review. Not urgent.

Closes #2379

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

@teor2345
Copy link
Contributor

The macOS failure is unrelated, PR #3072 should fix it.

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.

I'm not sure if less than or less than equal to should be used in the assertion 🤔

zebra-chain/src/orchard/shielded_data.rs Outdated Show resolved Hide resolved
zebra-chain/src/sapling/output.rs Outdated Show resolved Hide resolved
zebra-chain/src/sapling/spend.rs Outdated Show resolved Hide resolved
zebra-chain/src/lib.rs Outdated Show resolved Hide resolved
conradoplg and others added 3 commits November 18, 2021 13:43
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
@jvff jvff merged commit 88b09c8 into main Nov 18, 2021
@jvff jvff deleted the check-spends-outputs-actions-limits branch November 18, 2021 18:06
mpguerra added a commit that referenced this pull request May 19, 2023
mergify bot pushed a commit that referenced this pull request May 23, 2023
* ZIPs were updated to remove ambiguity, this was tracked in #1267.

* #2105 was fixed by #3039 and #2379 was closed by #3069

* #2230 was a duplicate of #2231 which was closed by #2511

* #3235 was obsoleted by #2156 which was fixed by #3505

* #1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixed by #2969

* We migrated to Rust 2021 edition in Jan 2022 with #3332

* #1631 was closed as not needed

* #338 was fixed by #3040 and #1162 was fixed by #3067

* #2079 was fixed by #2445

* #4794 was fixed by #6122

* #1678 stopped being an issue

* #3151 was fixed by #3934

* #3204 was closed as not needed

* #1213 was fixed by #4586

* #1774 was closed as not needed

* #4633 was closed as not needed

* Clarify behaviour of difficulty spacing

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour when retrying block downloads

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify when we might want to fix

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify what we might want to change in future

Co-authored-by: teor <teor@riseup.net>

* Clarify benefits of how we do block verification

Co-authored-by: teor <teor@riseup.net>

* Fix rustfmt errors

---------

Co-authored-by: teor <teor@riseup.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do we need to validate nSpendsSapling, nOutputsSapling, and nActionsOrchard limits in V5 transactions?
3 participants