Skip to content

Conversation

@cce
Copy link
Contributor

@cce cce commented May 23, 2025

Summary

This adds SHA-512 hashing to the block header, including the transaction commitment. When #3829 was added, a generic hashing abstraction and a new vector commitment implementation were added that supports both 64-byte (sumhash) and 32-byte (SHA-256, SHA-512/256) hashes, so not many changes were needed.

Matching go-algorand-sdk PR in algorand/go-algorand-sdk#723

Test Plan

Existing tests should pass; in addition, #3829 added some tests for SHA-256 transaction commitments that could be copy/pasted into this PR for SHA-512.

@codecov
Copy link

codecov bot commented May 23, 2025

Codecov Report

Attention: Patch coverage is 59.37500% with 13 lines in your changes missing coverage. Please review.

Project coverage is 51.89%. Comparing base (6f65ab1) to head (2c061fa).
Report is 29 commits behind head on master.

Files with missing lines Patch % Lines
crypto/hashes.go 0.00% 6 Missing ⚠️
data/bookkeeping/block.go 72.72% 3 Missing and 3 partials ⚠️
data/bookkeeping/genesis.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6339      +/-   ##
==========================================
- Coverage   51.90%   51.89%   -0.02%     
==========================================
  Files         653      653              
  Lines       87469    87500      +31     
==========================================
+ Hits        45399    45405       +6     
- Misses      39197    39219      +22     
- Partials     2873     2876       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cce added a commit to algorand/go-algorand-sdk that referenced this pull request May 28, 2025
@cce cce requested a review from Copilot May 28, 2025 19:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for SHA-512 hashing in block headers and transaction commitments.

  • Introduces a new Branch512 field on BlockHeader and wires it through PreCheck, serialization, and genesis/block creation.
  • Adds TxnMerkleTreeSHA512 and paysetCommitSHA512 alongside the existing SHA-256 and SHA-512/256 commitments.
  • Updates consensus parameters, tests, and regenerated MsgPack code to account for the new 64-byte fields.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
protocol/tags.go Increased ProposalPayloadTagMaxSize for extra hash.
data/bookkeeping/genesis.go Modified genesis block initialization of TxnCommitments.
data/bookkeeping/txn_merkle.go Added TxnMerkleTreeSHA512 method.
data/bookkeeping/block.go Added Branch512, Hash512, paysetCommitSHA512; integrated into MakeBlock and PreCheck.
data/bookkeeping/msgp_gen.go Regenerated MsgPack code to include new fields.
crypto/hashes.go Added Sha512 type, Sha512Digest, and hash factory case.
config/consensus.go Introduced EnableSha512BlockHash flag and enabled it for future protocols.
crypto/msgp_gen.go & crypto/msgp_gen_test.go Added MsgPack support and tests for Sha512Digest.
catchup/catchpointService_test.go Updated test to set Branch512 on downloaded blocks.
(plus extensive autogenerated changes in agreement/msgp_gen.go and transmittedPayload code)

gmalouf pushed a commit to algorand/go-algorand-sdk that referenced this pull request Jun 3, 2025
@gmalouf gmalouf merged commit ab67434 into algorand:master Jun 3, 2025
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants