Update EIP-8142: Various blob encoding-decoding fixes#11763
Conversation
File
|
frisitano
left a comment
There was a problem hiding this comment.
Looks good - left a comment inline
| bal_length = len(bal_bytes).to_bytes(4, 'big') | ||
| txs_length = len(txs_bytes).to_bytes(4, 'big') |
There was a problem hiding this comment.
I understand that field elements are represented as big-endian - does that mean that we should have all encodings use the same standard? What's the rationale/decision space here and implications?
There was a problem hiding this comment.
does that mean that we should have all encodings use the same standard?
No, that's not necessary, this change is optional.
I see value in consistently using big-endian encoding. It is already used for the blob field elements, and for RLP encoding, we might as well use it for these two integers.
I did a quick search in https://github.com/ethereum/execution-specs. It seems that the EL tends to use big-endian in most cases, though there are a few exception.
There was a problem hiding this comment.
Thanks for the explanation - sounds good to me.
This PR addresses various issues and inconsistencies in the blob encoding/decoding logic in EIP-8142:
chunk_to_blobshould set the first byte of each field element to 0, instead of setting the last byte.bal_lengthandtxs_lengthin the 8-byte blob payload header.blobs_to_execution_payload_data(array overindexing and malleable trailing data). Thoughengine_newPayloadre-encodes using the canonical zero-padding, so this was not a correctness issue.