Skip to content

consistency proof: fix edge case with 0 sized tree - #9

Open
phbnf wants to merge 1 commit into
mainfrom
edgecase
Open

consistency proof: fix edge case with 0 sized tree#9
phbnf wants to merge 1 commit into
mainfrom
edgecase

Conversation

@phbnf

@phbnf phbnf commented Jun 17, 2026

Copy link
Copy Markdown
Owner

No description provided.

@phbnf

phbnf commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the consistency proof logic for Merkle trees to treat a starting tree size of zero as invalid, returning an error instead of treating it as a valid empty proof. This change is propagated across proof generation, verification, unit tests, and fuzz tests. The reviewer suggested updating the error message in proof/verify.go to include the article "an" for consistency with the error message in proof/proof.go.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread proof/verify.go
Comment on lines +115 to +116
case size1 == 0:
return nil, errors.New("consistency proof from empty tree is meaningless")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For consistency with the error message in proof/proof.go (which uses "consistency proof from an empty tree is meaningless"), please update this error message to include the article "an".

Suggested change
case size1 == 0:
return nil, errors.New("consistency proof from empty tree is meaningless")
case size1 == 0:
return nil, errors.New("consistency proof from an empty tree is meaningless")

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Merkle tree consistency proof logic to disallow consistency proofs starting from an empty tree (size 0). It updates the validation in Consistency and RootFromConsistencyProof to return an error when size1 == 0, and adjusts the corresponding tests, fuzzers, and test data to reflect this new constraint. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@phbnf
phbnf force-pushed the edgecase branch 2 times, most recently from a25b929 to 6b43ed0 Compare June 17, 2026 11:10
@phbnf

phbnf commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the consistency proof generation and verification logic to disallow consistency proofs starting from an empty tree (size 0). This validation is applied across the codebase, including updates to tests, fuzzers, and test data. The reviewer pointed out that a fuzz test in testonly/tree_test.go still generates tree sizes starting from 0, which will lead to test failures, and provided a code suggestion to ensure generated sizes are at least 1.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread testonly/tree_test.go
}

for size1 := range uint64(8) + 1 {
for size1 := uint64(1); size1 <= 8; size1++ {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In TestTreeConsistencyProofFuzz (around line 184), size2 and size1 are generated using rand.Uint64N(treeSize + 1) and rand.Uint64N(size2 + 1). This allows size1 to be 0. Since consistency proofs from a 0-sized tree are now considered invalid and return an error, this will cause TestTreeConsistencyProofFuzz to fail when size1 is randomly chosen as 0. To fix this, we should ensure size1 and size2 are at least 1 by adjusting the random generation to:\n\ngo\nsize2 := rand.Uint64N(treeSize) + 1\nsize1 := rand.Uint64N(size2) + 1\n

@phbnf

phbnf commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request restricts Merkle tree consistency proofs by disallowing a starting tree size of zero (size1 == 0), treating it as an error. The changes include updates to the proof generation, verification logic, unit tests, fuzz tests, and test data to enforce and validate this new constraint. There are no review comments, so no additional feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@phbnf

phbnf commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the consistency proof generation and verification logic to treat a starting tree size of zero (size1 == 0) as invalid, returning an error since consistency proofs from an empty tree are meaningless. Corresponding updates have been made to the unit tests, fuzz tests, and test data to reflect this change. The reviewer pointed out that a comment in cmd/proofgen/main.go has become misleading because the test cases below it are now expected to fail, and suggested moving those cases to keep the test structure accurate.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread cmd/proofgen/main.go Outdated
Comment on lines 405 to 408
// Sizes that are always consistent.
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", false},
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", true},
{0, 1, root1, root2, proof1, "size1 is zero and does not equal size2", true},
{1, 1, root2, root2, proof1, "sizes are equal (one) and proof is empty", false},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The comment // Sizes that are always consistent. is now misleading because the {0, 0, ...} and {0, 1, ...} cases directly below it are now expected to fail (wantErr: true). Moving these cases above the comment keeps the "always consistent" section accurate and prevents confusion for future maintainers.

Suggested change
// Sizes that are always consistent.
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", false},
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", true},
{0, 1, root1, root2, proof1, "size1 is zero and does not equal size2", true},
{1, 1, root2, root2, proof1, "sizes are equal (one) and proof is empty", false},
{0, 0, root1, root1, proof1, "sizes are equal (zero) and proof is empty", true},
{0, 1, root1, root2, proof1, "size1 is zero and does not equal size2", true},
// Sizes that are always consistent.
{1, 1, root2, root2, proof1, "sizes are equal (one) and proof is empty", false},

@phbnf

phbnf commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Merkle tree consistency proof logic to disallow consistency proofs starting from an empty tree (where size1 is 0). It modifies validation checks in proof.go and verify.go, and updates corresponding unit tests, fuzz tests, and test data to enforce this constraint. There are no review comments to address, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

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.

1 participant