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.

Sign up for GitHub

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

Jump to bottom

feat!: checks the exact size of the node in HashNode #175

Merged
merged 13 commits into from
Apr 13, 2023
Merged

feat!: checks the exact size of the node in HashNode #175

merged 13 commits into from
Apr 13, 2023

Conversation

staheri14
Copy link
Collaborator

@staheri14 staheri14 commented Apr 11, 2023 •
edited
Loading

Overview

Closes #153 (part of #173)

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@codecov
Copy link

codecov bot commented Apr 11, 2023 •
edited
Loading

Codecov Report

Merging #175 (dd6a640) into master (2c15503) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #175   +/-   ##
=======================================
  Coverage   95.76%   95.76%           
=======================================
  Files           5        5           
  Lines         519      519           
=======================================
  Hits          497      497           
  Misses         13       13           
  Partials        9        9
Impacted Files Coverage Δ
hasher.go 97.54% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@staheri14 staheri14 self-assigned this Apr 11, 2023
@staheri14 staheri14 added the enhancement New feature or request label Apr 11, 2023
@staheri14 staheri14 marked this pull request as ready for review April 11, 2023 21:12
@staheri14 staheri14 requested review from liamsi and rootulp April 11, 2023 21:13
@staheri14 staheri14 assigned evan-forbes and unassigned evan-forbes Apr 11, 2023
evan-forbes
evan-forbes reviewed Apr 12, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

nice, LGTM, only had a single question

this change seems like it could be consensus breaking if some nodes use it and others don't, so we might want to rename this PR to feat!... to indicate such a change

hasher.go Outdated Show resolved Hide resolved
@staheri14 staheri14 changed the title feat: checks the exact size of the node in HashNode feat!: checks the exact size of the node in HashNode Apr 12, 2023
@staheri14
Copy link
Collaborator Author

this change seems like it could be consensus breaking if some nodes use it and others don't, so we might want to rename this PR to feat!... to indicate such a change

Right, updated the PR description.

@staheri14 staheri14 requested a review from evan-forbes April 12, 2023 17:19
evan-forbes
evan-forbes previously approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

👍 👍

rootulp
rootulp requested changes Apr 13, 2023
View reviewed changes
hasher.go Outdated Show resolved Hide resolved
hasher.go Outdated Show resolved Hide resolved
hasher_test.go Show resolved Hide resolved
@staheri14 staheri14 requested a review from rootulp April 13, 2023 17:01
@staheri14 staheri14 requested a review from evan-forbes April 13, 2023 17:07
@staheri14 staheri14 merged commit be509c3 into master Apr 13, 2023
@staheri14 staheri14 deleted the check-node-exact-length-in-hasher branch April 13, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rootulp rootulp rootulp approved these changes

@adlerjohn adlerjohn adlerjohn approved these changes

@liamsi liamsi Awaiting requested review from liamsi

@cmwaters cmwaters Awaiting requested review from cmwaters

@evan-forbes evan-forbes Awaiting requested review from evan-forbes

Assignees

@staheri14 staheri14

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ValidateNode does not check the exact size of the non-leaf nodes
4 participants
@staheri14 @adlerjohn @rootulp @evan-forbes