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

Rename the () placeholder to FieldNotPresent #1987

Merged
merged 3 commits into from
Apr 7, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Apr 7, 2021

Motivation

In the transaction version 5 RFC, we use () (an empty tuple) as a place holder when an anchor isn't present in that transaction version.

But it would be clearer to use a unit struct FieldNotPresent;.

Solution

  • Replace () with FieldNotPresent

Review

@oxarbitrage can review, this change conflicts with #1829.

Related Issues

Closes #1971.
Conflicts with #1829.

@teor2345 teor2345 added A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup NU-5 Network Upgrade: NU5 specific tasks P-Medium labels Apr 7, 2021
@teor2345 teor2345 requested a review from oxarbitrage April 7, 2021 03:54
@teor2345 teor2345 self-assigned this Apr 7, 2021
@mpguerra mpguerra modified the milestones: 2021 Sprint 7, 2021 Sprint 10 Apr 7, 2021
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks great.

@oxarbitrage oxarbitrage merged commit 4185754 into ZcashFoundation:main Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-cleanup Category: This is a cleanup NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change () to a unit struct in Sapling ShieldedData
4 participants