Skip to content

#57, #55 Update inclusion proof verification, add cbor tags#59

Open
martti007 wants to merge 2 commits intoissue-52from
issue-57
Open

#57, #55 Update inclusion proof verification, add cbor tags#59
martti007 wants to merge 2 commits intoissue-52from
issue-57

Conversation

@martti007
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

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

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 performs a major refactoring of the SDK, introducing InclusionCertificate and ShardId while standardizing CBOR tagging and versioning across API components. The Merkle tree implementation was moved to a new smt package and updated to a radix-based approach. Review feedback identifies a high-severity bug in the popcount logic of InclusionCertificate caused by signed byte extension. Furthermore, improvements are needed for the equals and hashCode implementations in CertificationData and InclusionProof to ensure consistency and reliability in hash-based collections.

Comment thread src/main/java/org/unicitylabs/sdk/api/InclusionCertificate.java Outdated
Comment thread src/main/java/org/unicitylabs/sdk/api/CertificationData.java Outdated
Comment thread src/main/java/org/unicitylabs/sdk/api/InclusionProof.java Outdated
Copy link
Copy Markdown
Member

@MastaP MastaP left a comment

Choose a reason for hiding this comment

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

Three blocking equals/hashCode issues (inline) plus one cleanup item: src/main/java/org/unicitylabs/sdk/api/bft/verification/rule/InputRecordCurrentHashVerificationRule.java is left as a 0-byte file rather than removed. Please git rm it — its check is now folded into InclusionProofVerificationRule.verify (line 63), so the empty source file just adds confusion.

Comment thread src/main/java/org/unicitylabs/sdk/api/CertificationData.java
Comment thread src/main/java/org/unicitylabs/sdk/api/InclusionProof.java
Comment thread src/main/java/org/unicitylabs/sdk/api/InclusionProof.java
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.

[TGE] Update inclusion proof format [TGE] Update SDK implementation to use the new type tags

2 participants