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

Replace ConsumedNoteInfo with Nullifier #360

Merged
merged 1 commit into from
Dec 15, 2023
Merged

Conversation

bobbinth
Copy link
Contributor

@bobbinth bobbinth commented Dec 14, 2023

This PR addresses a part of #350. Specifically, I removed ConsumedNoteInfo struct and replaced it with Nullifier. Using only nullifiers to compute consumed note commitment is sufficient since #283 as the verifier no longer needs to know script roots for the consumed notes.

The commitment to consumed notes is still computed as a sequential hash of (nullifier, ZERO) tuples instead of just as a sequential hash of nullifiers. I kept it this way primarily to keep the scope of changes relatively small and also because we may need to update the logic again to support #353.

@bobbinth
Copy link
Contributor Author

cc @igamigo: this might affect some things in the client, but the changes should be very minor.

Copy link
Collaborator

@Dominik1999 Dominik1999 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 to me. Left some comments inline.

And why do you not need to change a test? Isn't there a test for consuming notes?

miden-lib/asm/miden/sat/internal/prologue.masm Outdated Show resolved Hide resolved
miden-tx/src/verifier/mod.rs Show resolved Hide resolved
miden-tx/src/verifier/mod.rs Show resolved Hide resolved
miden-tx/src/verifier/mod.rs Show resolved Hide resolved
objects/src/notes/nullifier.rs Outdated Show resolved Hide resolved
objects/src/transaction/proven_tx.rs Show resolved Hide resolved
@bobbinth
Copy link
Contributor Author

And why do you not need to change a test? Isn't there a test for consuming notes?

Because I've changed both the Rust code and the MASM code to compute the input note has in the same way, the tests don't need to be changed (if I updated only MASM or only Rust, the tests would be failing).

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.

2 participants