Skip to content

cl/merkle_tree: add EIP-7495 ProgressiveContainer root - #23130

Open
JackCC703 wants to merge 9 commits into
erigontech:mainfrom
JackCC703:eip-7495-progressive-container-root
Open

cl/merkle_tree: add EIP-7495 ProgressiveContainer root#23130
JackCC703 wants to merge 9 commits into
erigontech:mainfrom
JackCC703:eip-7495-progressive-container-root

Conversation

@JackCC703

Copy link
Copy Markdown
Contributor

Summary

Add ProgressiveContainerRoot following EIP-7495.

The helper:

  • places active field roots at their stable indices
  • zero-fills inactive positions
  • combines MerkleizeProgressive with MixInActiveFields
  • rejects invalid active_fields configurations

Tests cover the EIP-7807 18-field layout, sparse fields, the 256-bit boundary, invalid configurations, and input immutability.

References

  • EIP-7495: ethereum/EIPs@c81d843b3f8aa839fe42911c5b6e501c7d2940a3
  • Reference implementation: ethereum/remerkleable@2f0baeef0082d4278acaef7d822deb7009d7db7e
  • EIP-7807: ethereum/EIPs@75d7bc2c20a91ec017d147f400d6bbf767843e2c

Reference vectors were generated from the pinned remerkleable revision.

Testing

  • go test ./cl/merkle_tree -run '^TestProgressiveContainerRoot' -count=1
  • go test ./cl/merkle_tree -count=1
  • go test -race ./cl/merkle_tree -count=1
  • go tool golangci-lint run --config ./.golangci.yml ./cl/merkle_tree/...
  • make erigon integration

Depends on #22528.

Comment on lines +46 to +49
func MixInActiveFields(root [32]byte, activeFields []bool) ([32]byte, error) {
if len(activeFields) > 256 {
return [32]byte{}, errors.New("active fields exceed 256 bits")
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

MixInActiveFields is currently only checking len(activeFields) > 256. But as per EIP-7495 spec, an active_fields configuration cannot be empty (len == 0) and cannot end with a 0 bit (!activeFields[len-1]).

Right now ProgressiveContainerRoot validates these edge cases, but since MixInActiveFields is exported publicly, if someone calls it directly with something like []bool{true, false} or empty slice, it silently hashes without giving error.

Can we add these checks here as well?

if len(activeFields) == 0 {
    return [32]byte{}, errors.New("active fields cannot be empty")
}
if !activeFields[len(activeFields)-1] {
    return [32]byte{}, errors.New("active fields must end with an active field")
}

return [32]byte{}, errors.New("active field count does not match field roots")
}

expandedRoots := make([][32]byte, len(activeFields))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here make([][32]byte, len(activeFields)) will allocate on heap for every call.

In actual execution / consensus paths, almost all progressive containers have ≤ 32 fields (for example ExecutionPayload in EIP-7807 has only 18 fields).

We can do one small optimization here: use a small stack array var stackRoots [32][32]byte for len(activeFields) <= 32 so we completely avoid heap allocations on hot paths (similar to maxStackLeaves = 32 in merkle_root.go).

Something like:

var stackRoots [32][32]byte
var expandedRoots [][32]byte
if len(activeFields) <= 32 {
    expandedRoots = stackRoots[:len(activeFields)]
} else {
    expandedRoots = make([][32]byte, len(activeFields))
}

This will save GC pressure when processing blocks repeatedly.

Comment on lines +77 to +85
activeFieldCount := 0
for _, active := range activeFields {
if active {
activeFieldCount++
}
}
if activeFieldCount != len(fieldRoots) {
return [32]byte{}, errors.New("active field count does not match field roots")
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also here, currently we are iterating over activeFields 3 times in total (first time to count 1s, second time to populate expandedRoots, and third time inside MixInActiveFields).

We can actually merge this count validation directly into the expandedRoots loop in a single pass:

fieldIndex := 0
for i, active := range activeFields {
    if active {
        if fieldIndex >= len(fieldRoots) {
            return [32]byte{}, errors.New("active field count does not match field roots")
        }
        expandedRoots[i] = fieldRoots[fieldIndex]
        fieldIndex++
    }
}
if fieldIndex != len(fieldRoots) {
    return [32]byte{}, errors.New("active field count does not match field roots")
}

That way loop pass is reduced and it also fails fast if fieldRoots count is less than active bits.

}
}

func TestMixInActiveFieldsReferenceVectors(t *testing.T) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here, all current reference vector tests (sparseFields, boundaryFields, etc.), field index 0 is always set to true.

Could we add a small unit test where field index 0 is inactive (activeFields = []bool{false, true, true})? Just to ensure zero-filling at leaf position 0 / gindex 4 works as expected when the first field itself is absent.

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