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: verifies input range in the computeRoot to avoid panic #186

Merged
merged 42 commits into from
May 3, 2023
Merged

feat: verifies input range in the computeRoot to avoid panic #186

merged 42 commits into from
May 3, 2023

Conversation

staheri14
Copy link
Contributor

@staheri14 staheri14 commented Apr 28, 2023 •
edited
Loading

Overview

Closes #137

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

@staheri14
updates criteria of an empty proof
e49d7b1
@staheri14
replaces slice literal with nID1
5e9091c
@staheri14
refactors the code to reject invalid empty proof, adds tests
b6e3cd3
@staheri14
corrects IsOfEmptyProof description
5c32cbc
@staheri14
prefixes min and max with root
84865d7
@staheri14
fixes linter issues
c8a8059
@staheri14
revises some typos
8a27636
@staheri14
renames IsOfEmptyProof to IsEmptyProof
9e3d8de
@staheri14
returns immediately on invalid range
20b2205
@staheri14
adds unit tests
6222d5f
@staheri14
Merge remote-tracking branch 'origin/master' into verifies-proof-range
30cbde4
@staheri14
considers start=end as invalid
9fd31f2
@staheri14
defines a utility function for proof range verification
167629c
@staheri14
returns the error returned by validateRange
fcd3cd0
@staheri14
adds proof range check
d11e9c2
@staheri14
incorporates further tests covering new range check
bb07d07
@staheri14
returns err produced by validateRange
d491b52
@staheri14
Merge branch 'verifies-proof-range' into panics-in-verifyleafhashes
e6cb4dc
@staheri14
implements the necessary logic to handle empty proofs
a449d68
@staheri14
develops tests
29007f4
@staheri14
revises the err message
8fdaab2
@staheri14
extracts the NID from the leaf
409f0f8
@staheri14
removes an excess line
2d504d5
@staheri14
Merge branch 'master' into panics-in-verifyleafhashes
48010b1
@staheri14
revises tests descriptions
712ed08
@staheri14
declares a variable for nonEmptyProof for readability
e01ce18
@staheri14
removes an excess line
fc595aa
@staheri14
replaces manual extraction of min and max NID with proper func calls
021adc0
@staheri14
adds node format validation to the validateSiblingsNamespaceOrder
951ad75
@staheri14
checks the input range
d132fdc
@staheri14 staheri14 self-assigned this Apr 28, 2023
@staheri14 staheri14 added the enhancement New feature or request label Apr 28, 2023
@codecov
Copy link

codecov bot commented Apr 28, 2023 •
edited
Loading

Codecov Report

Merging #186 (eecd425) into master (1262b7f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #186      +/-   ##
==========================================
+ Coverage   95.69%   95.70%   +0.01%     
==========================================
  Files           5        5              
  Lines         557      559       +2     
==========================================
+ Hits          533      535       +2     
  Misses         14       14              
  Partials       10       10
Impacted Files Coverage Δ
nmt.go 98.01% <100.00%> (+0.01%) ⬆️

@staheri14 staheri14 marked this pull request as ready for review May 1, 2023 22:39
rootulp
rootulp previously approved these changes May 2, 2023
View reviewed changes
nmt_test.go Show resolved Hide resolved
evan-forbes
evan-forbes approved these changes May 3, 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.

LGTM

@staheri14 staheri14 merged commit 9c56d14 into master May 3, 2023
@staheri14 staheri14 deleted the compute-root-range-check branch May 3, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evan-forbes evan-forbes evan-forbes approved these changes

@rootulp rootulp rootulp approved these changes

@liamsi liamsi Awaiting requested review from liamsi

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.

Capture index range error in the computeRoot helper
3 participants
@staheri14 @rootulp @evan-forbes