Skip to content

Fix composite types#17

Merged
kasey merged 2 commits intomainfrom
fix-composite-types
Nov 3, 2025
Merged

Fix composite types#17
kasey merged 2 commits intomainfrom
fix-composite-types

Conversation

@terencechain
Copy link
Copy Markdown

@terencechain terencechain commented Aug 18, 2025

This PR ensures a vector of composite type does not panic, example when running https://github.com/OffchainLabs/prysm/blob/develop/hack/update-go-ssz.sh

main.uintVToName(0x14001b7f810?)
        external/com_github_prysmaticlabs_fastssz/sszgen/main.go:1291 +0x138
main.(*Value).hashRoots(0x140010ef360, 0x0, 0x7)
        external/com_github_prysmaticlabs_fastssz/sszgen/hash.go:59 +0x154

- Add special handling for TypeVector with TypeContainer elements
- Generate code that calls HashTreeRootWith() for each composite element
- Fixes panic when using composite types with ssz_size annotations
// Test struct for composite types
type TestComposite struct {
Field1 uint64
Field2 []byte `ssz-size:"32"`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't we need also a test with a vector of variable size composites?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hm, i could. But fixed vector size composites was what failed. (ie builder_pending_payments)

Comment on lines +21 to +27
func (t *TestComposite) HashTreeRootWith(hh *ssz.Hasher) (err error) {
indx := hh.Index()
hh.PutUint64(t.Field1)
hh.PutBytes(t.Field2)
hh.Merkleize(indx)
return
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this correct? a container with an uint64 x and a 32 byte vector y inside is hashed as

hash( [0x00]x24 | x | y)

This seems to be hashing hash(x|y|[0x00]x24)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

resolved offline

@kasey kasey merged commit 2593022 into main Nov 3, 2025
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.

3 participants