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

Update has_inputs_and_outputs() for new consensus rules #2398

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Jun 26, 2021

Motivation

We need to update Zebra for new consensus rules added to the protocol in regards to orchard flags. Closes #2365

Specifications

  • At least one of tx_in_count, nSpendsSapling, and nJoinSplit MUST be non-zero.
  • At least one of tx_out_count, nOutputsSapling, and nJoinSplit MUST be non-zero.
  • This condition must hold: tx_in_count > 0 or nSpendsSapling > 0 or (nActionsOrchard > 0 and enableSpendsOrchard = 1)
  • This condition must hold: tx_out_count > 0 or nOutputsSapling > 0 or (nActionsOrchard > 0 and enableOutputsOrchard = 1)

https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus

Solution

Add the new consensus rules into the has_inputs_and_outputs function, update the tests to cover them.

Review

Anyone can review the code initially, before merging @teor2345 should take a look.

Reviewer Checklist

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

teor2345
teor2345 previously approved these changes Jun 28, 2021
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

This PR is good enough to merge now. I'll leave it to @oxarbitrage to make some improvements if he wants.

@oxarbitrage
Copy link
Contributor Author

I moved the remaining items to new lower priority tickets and enabled auto merge here. Need a new approval.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

This has tests and a follow up ticket, so it's good to go

@oxarbitrage oxarbitrage merged commit c06cd19 into ZcashFoundation:main Jun 28, 2021
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.

Check enableSpends or enableOutputs in has_inputs_and_outputs
3 participants