Skip to content

Comments

Change hash representation from slice to 32-byte array#505

Merged
LarryRuane merged 1 commit intozcash:masterfrom
LarryRuane:2025-01-hash32
Aug 5, 2025
Merged

Change hash representation from slice to 32-byte array#505
LarryRuane merged 1 commit intozcash:masterfrom
LarryRuane:2025-01-hash32

Conversation

@LarryRuane
Copy link
Collaborator

This change is non-functional, just a clean-up, reducing tech debt. I've been meaning to do this almost from when I first started working on lightwalletd in 2019.

This codebase manipulates many 32-byte hashes, such as block hashes, transaction IDs, and merkle roots. These should always have been represented as fixed-size 32-byte arrays rather than variable-length slices. This prevents bugs (a slice that's supposed to be of length 32 bytes could be a different length) and makes assigning, comparing, function argument passing, and function return value passing simpler and less fragile.

The new hash type, hash32.T, which is defined as [32]byte (32-byte array of bytes), can be treated like any simple variable, such as an integer. A slice, in contrast, is like a string; it's really a structure that includes a length, capacity, and pointer to separately-allocated memory to hold the elements of the slice.

The only point of friction is that protobuf doesn't support fixed-sized arrays, only (in effect) slices. So conversion must be done in each direction.

This PR also changes the representations of a few other (non-hash) variables that have known, fixed sizes, such as the block header fields NBitsBytes, Nonce, and Solution.

I tested this (besides running the unit test suite) manually by using both the current master branch and this PR's branch to sync the mainnet blockchain, which means deserializing full blocks, converting them to compact blocks, and writing them to disk. Then, I compared these files to verify that they were identical. If this PR introduced a bug in serializing or deserializing compact blocks, this test would have caught it.

Copy link
Contributor

@y4ssi y4ssi left a comment

Choose a reason for hiding this comment

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

utACK

This change is non-functional, just a clean-up, reducing tech debt.

This codebase manipulates many 32-byte hashes, such as block hashes,
transaction IDs, and merkle roots. These should always have been
represented as fixed-size 32-byte arrays rather than variable-length
slices. This prevents bugs (a slice that's supposed to be of length
32 bytes could be a different length) and makes assigning, comparing,
function argument passing, and function return value passing simpler
and less fragile.

The new hash type, hash32.T, which is defined as [32]byte (32-byte
array of bytes), can be treated like any simple variable, such as an
integer. A slice, in contrast, is like a string; it's really a structure
that includes a length, capacity, and pointer to separately-allocated
memory to hold the elements of the slice.

The only point of friction is that protobuf doesn't support fixed-sized
arrays, only (in effect) slices. So conversion must be done in each
direction.
@LarryRuane LarryRuane merged commit e5a7393 into zcash:master Aug 5, 2025
3 checks passed
@LarryRuane LarryRuane deleted the 2025-01-hash32 branch August 5, 2025 19:19
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.

2 participants