feat: align HCS-27 with published draft#6
Conversation
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the HCS-27 client to conform to the latest draft specification. The primary goal is to standardize data representation and validation for checkpoint metadata, particularly by changing tree size fields to canonical string formats and updating hash field names. Additionally, it introduces new utilities for verifying proof objects, enhancing the client's capability to interact with the updated specification. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully aligns the Go HCS-27 client with the published draft specification. Key changes include switching treeSize fields to canonical decimal strings, updating field names to rootHashB64u, removing the batch_range dependency, and enforcing rfc9162 for Merkle tree calculations. New helper functions for parsing canonical uint64 values from strings have been introduced, and proof object models now match the draft's off-ledger shapes. The changes are reflected across documentation, examples, and unit/integration tests, demonstrating a thorough update to the new specification.
I am having trouble creating individual review comments. Click here to see my feedback.
pkg/hcs27/types.go (27-28)
The TreeSize field in RootCommitment has been correctly updated from uint64 to string to align with the new specification requiring canonical decimal strings.
pkg/hcs27/validation_test.go (337-344)
The TestValidateMetadataTreeSizeCanonicalDecimal test is a valuable addition, specifically verifying that TreeSize values are canonical decimal strings, which is a new requirement in the specification.
pkg/hcs27/validation.go (127-129)
The validation logic for metadata.prev.treeSize now correctly compares the parsed uint64 values, ensuring previousTreeSize is not greater than rootTreeSize.
pkg/hcs27/client.go (412-418)
The introduction of parseCanonicalUint64 for metadata.root.treeSize is a critical change to correctly handle the new string representation of tree sizes. This ensures that the string is parsed into a uint64 for internal logic while validating its canonical format.
pkg/hcs27/client.go (431-437)
Similar to the root tree size, parseCanonicalUint64 is correctly applied to metadata.prev.treeSize to handle its string representation and validate its format. This is crucial for maintaining the integrity of the checkpoint chain validation.
pkg/hcs27/validation.go (120-123)
The metadata.prev.treeSize validation now correctly uses parseCanonicalUint64 to handle the string representation and ensure it's a valid canonical decimal string. This is a critical correctness improvement.
pkg/hcs27/validation.go (112-115)
The metadata.root.treeSize validation now correctly uses parseCanonicalUint64 to handle the string representation and ensure it's a valid canonical decimal string. This is a critical correctness improvement.
pkg/hcs27/integers.go (1-28)
The new integers.go file introduces parseCanonicalUint64 and canonicalUint64 functions. These are well-implemented helper functions that correctly handle the conversion and validation of canonical decimal string representations for uint64 values, which is essential for the updated HCS-27 specification.
pkg/hcs27/validation.go (109-110)
The validation for metadata.Log.Merkle has been correctly updated to explicitly enforce rfc9162, ensuring adherence to the new specification.
pkg/hcs27/types.go (56-73)
The addition of InclusionProof and ConsistencyProof structs is a significant improvement, providing structured types for handling proof objects as defined in the HCS-27 draft specification.
pkg/hcs27/types.go (32-33)
The TreeSize field in PreviousCommitment has been correctly updated from uint64 to string to align with the new specification requiring canonical decimal strings.
pkg/hcs27/merkle.go (135-157)
The new VerifyInclusionProofObject function correctly implements the verification logic for inclusion proofs based on the HCS-27 draft shape. It properly utilizes parseCanonicalUint64 for LeafIndex and TreeSize and validates TreeVersion.
pkg/hcs27/merkle.go (237-259)
The new VerifyConsistencyProofObject function correctly implements the verification logic for consistency proofs based on the HCS-27 draft shape. It properly utilizes parseCanonicalUint64 for OldTreeSize and NewTreeSize and validates TreeVersion.
pkg/hcs27/types.go (48)
The BatchRange field has been removed from CheckpointMetadata, aligning with the updated specification.
pkg/hcs27/validation.go (127-131)
The removal of BatchRange validation is consistent with the removal of the BatchRange struct from the metadata, streamlining the validation process.
pkg/hcs27/merkle_test.go (54-67)
A new test case has been added for VerifyInclusionProofObject, which is crucial for ensuring the correctness of the newly introduced proof verification logic.
pkg/hcs27/integration_test.go (165-170)
The overflow metadata in the integration test is updated to use rfc9162 for Merkle, canonicalUint64 for TreeSize, and RootHashB64u, ensuring consistency with the new specification.
pkg/hcs27/types.go (36-38)
The BatchRange struct has been removed, which is consistent with the updated HCS-27 draft specification.
README.md (114-115)
The example code in the README.md has been updated to reflect the new TreeSize as a string and the RootHashB64u field name, along with the removal of BatchRange. This is a good update to keep the documentation aligned with the latest specification.
pkg/hcs27/integration_test.go (92-101)
The second checkpoint in the integration test also correctly reflects the updated Merkle algorithm, TreeSize as a canonical string, and RootHashB64u for both current and previous commitments.
pkg/hcs27/integration_test.go (63-68)
The integration test for the first checkpoint correctly updates the Merkle algorithm to rfc9162, TreeSize to a canonical string using canonicalUint64, and RootHashB64u to match the new specification.
pkg/hcs27/doc.go (31)
The example in the doc.go file has been updated to reflect the TreeSize as a string and the RootHashB64u field name, which is important for accurate documentation.
pkg/hcs27/validation.go (116-117)
The field name RootHashB64 has been correctly updated to RootHashB64u in the validation logic, aligning with the specification.
pkg/hcs27/client_overflow_test.go (136-141)
The test metadata has been updated to align with the new specification, using rfc9162 for Merkle, canonicalUint64 for TreeSize, and RootHashB64u. This ensures the overflow tests are run against the correct data structures.
pkg/hcs27/validation.go (124-125)
The field name RootHashB64 has been correctly updated to RootHashB64u in the validation logic for previous commitments, aligning with the specification.
pkg/hcs27/client.go (399-400)
The previousRecord struct has been updated to reflect the new RootHashB64u field name, ensuring consistency with the updated RootCommitment and PreviousCommitment types.
pkg/hcs27/merkle_test.go (85-98)
A new test case has been added for VerifyConsistencyProofObject, which is crucial for ensuring the correctness of the newly introduced proof verification logic.
pkg/hcs27/validation_test.go (133-139)
The buildValidMetadata helper function has been updated to reflect the new Leaf and Merkle values, TreeSize as a canonical string, and RootHashB64u field name, ensuring test data aligns with the specification.
pkg/hcs27/validation_test.go (226)
The test case for invalid root hash has been updated to use RootHashB64u, correctly reflecting the field name change.
pkg/hcs27/validation_test.go (237-238)
The test case for previous commitment now correctly uses canonicalUint64 for TreeSize and RootHashB64u for the hash, aligning with the updated types.
pkg/hcs27/validation_test.go (249-250)
The test case for an invalid previous hash has been updated to use canonicalUint64 for TreeSize and RootHashB64u for the hash, ensuring the test targets the correct fields.
pkg/hcs27/validation_test.go (262-263)
The test case for PreviousTreeSizeTooLarge now correctly uses canonicalUint64 for TreeSize and RootHashB64u for the hash, reflecting the updated types.
pkg/hcs27/validation_test.go (275-292)
The removal of TestValidateMetadataBatchRangeInvalid and TestValidateMetadataBatchRangeEndExceedsTreeSize tests is appropriate, as the BatchRange struct has been removed from the specification.
examples/hcs27-publish-checkpoint/main.go (41-46)
The example has been correctly updated to use rfc9162 for the Merkle algorithm, TreeSize as a string, and RootHashB64u as per the new specification. The removal of BatchRange is also consistent.
pkg/hcs27/validation_test.go (509-516)
The ValidateCheckpointChain test cases have been updated to use canonicalUint64 for TreeSize and RootHashB64u for root hashes, ensuring they align with the new data types.
pkg/hcs27/validation_test.go (535-542)
The ValidateCheckpointChainTreeSizeDecreased test case has been updated to use canonicalUint64 for TreeSize and RootHashB64u for root hashes, ensuring it aligns with the new data types.
pkg/hcs27/validation_test.go (561-567)
The ValidateCheckpointChainMissingPrev test case has been updated to use canonicalUint64 for TreeSize and RootHashB64u for root hashes, ensuring it aligns with the new data types.
pkg/hcs27/validation_test.go (586-593)
The ValidateCheckpointChainPrevTreeSizeMismatch test case has been updated to use canonicalUint64 for TreeSize and RootHashB64u for root hashes, ensuring it aligns with the new data types.
pkg/hcs27/validation_test.go (613-620)
The ValidateCheckpointChainPrevRootHashMismatch test case has been updated to use canonicalUint64 for TreeSize and RootHashB64u for root hashes, ensuring it aligns with the new data types.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bdacaa157
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 2 CRITICAL Issues Found | Recommendation: Address before merge Overview
Incremental Review (6c7815a → 437e4fd)RESOLVED P2: The new commit fetches existing checkpoints before publishing, then waits for Issue Details (UNRESOLVED)CRITICAL
Resolved Issues
Files Reviewed (1 new file)
|
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c629f75cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cafd0019aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c7815ab4c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Michael Kantor <6068672+kantorcodes@users.noreply.github.com>
1 participant
Summary
treeSizefields to canonical decimal strings and update field names torootHashB64ubatch_rangedependency, requirerfc9162, and add draft-shaped proof object helpersVerification
go test ./pkg/hcs27/...go test ./...go test -run TestHCS27Integration ./pkg/hcs27 -v(skips in this shell becauseHEDERA_ACCOUNT_ID/HEDERA_PRIVATE_KEYare not exported)RUN_INTEGRATION=1 go test -run TestHCS27Integration ./pkg/hcs27 -v(still skips here for the same missing exported credentials)Draft deltas addressed
metadata.root.treeSize/metadata.prev.treeSizeare canonical decimal stringsmetadata.log.merkleis enforced asrfc9162metadata.root.rootHashB64u/metadata.prev.rootHashB64umatch the published field namesmetadataas an HCS-1 reference plus optionalmetadata_digest