Skip to content

Comments

chore: fix ABI encoding of eip-1271#26

Open
MatusKysel wants to merge 1 commit intomainfrom
fix-eip-1271-abi
Open

chore: fix ABI encoding of eip-1271#26
MatusKysel wants to merge 1 commit intomainfrom
fix-eip-1271-abi

Conversation

@MatusKysel
Copy link
Contributor

#25

Copy link

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

This PR updates the EIP-1271 ABI and Go bindings to use a bytes32 _hash parameter instead of arbitrary bytes _data, and aligns SSZ encoding offsets by removing outdated offset increments and enforcing exact offsets on decode.

  • Remove per-field offset += … in SSZ marshalling and change decode checks to strict equality
  • Update EIP-1271 ABI (_data_hash) and regenerate Go contract bindings
  • Fix the call site in crypto/signature.go to pass the new [32]byte hash signature

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
types_encoding.go Drop incremental offset += … adjustments and require exact offsets on decode
eip1271/eip1271.go Swap ABI method to take bytes32 _hash, update Go binding signatures
eip1271/abi.abi Change input name/type in ABI JSON from _data:bytes to _hash:bytes32
crypto/signature.go Call IsValidSignature with a [32]byte hash instead of a byte slice
Comments suppressed due to low confidence (2)

types_encoding.go:669

  • This loop for computing variable offsets was removed, but the offset variable still drives subsequent WriteOffset calls. Without advancing offset, later fields will encode with stale offsets. Restore or replace this logic so offset reflects the true byte length of Proofs.
	dst = ssz.WriteOffset(dst, offset)

types_encoding.go:31

  • Removing this offset += len(o.PubKey) means the offset variable never advances before the next dynamic field, so subsequent SSZ offsets will be wrong. Please reintroduce an offset increment or adjust the encoding loop to keep offsets in sync.
	dst = append(dst, o.PubKey...)

Copy link
Contributor

@radiken radiken left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alan-ssvlabs alan-ssvlabs left a comment

Choose a reason for hiding this comment

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

LGTM

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