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

Implement Smt struct (replacement to TieredSmt) #254

Merged
merged 122 commits into from
Jan 19, 2024
Merged

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Jan 17, 2024

Builds on #245

@plafer plafer changed the base branch from main to next January 17, 2024 15:00
@plafer plafer mentioned this pull request Jan 17, 2024
7 tasks
Copy link
Contributor

@bobbinth bobbinth 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! Thank you! I left some minor comments inline. After these are addressed, we are good to merge.

src/merkle/smt/full/mod.rs Outdated Show resolved Hide resolved
src/merkle/smt/full/mod.rs Outdated Show resolved Hide resolved
src/merkle/smt/full/mod.rs Outdated Show resolved Hide resolved
src/merkle/smt/full/mod.rs Outdated Show resolved Hide resolved
src/merkle/smt/full/mod.rs Outdated Show resolved Hide resolved
src/merkle/smt/full/mod.rs Show resolved Hide resolved
src/merkle/smt/full/mod.rs Outdated Show resolved Hide resolved
#[test]
fn test_smt_insert_at_same_key_2() {
let key_already_present: RpoDigest = {
let raw = 0b_01101001_01101100_00011111_11111111_10010110_10010011_11100000_00000000_u64;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think this is the same value as the one on line 75 below (it took me some time to understand this). I'm not sure there is a big benefit to using binary representation here and it actually makes this a bit more difficult to follow. I'd probably use just decimal values for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right - cleaned it up

@plafer
Copy link
Contributor Author

plafer commented Jan 19, 2024

@bobbinth addressed PR comments, and fixed SmtLeaf as discussed in a880c88

Copy link

sonarcloud bot commented Jan 19, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

All looks good! Thank you!

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