status-im / nimbus-eth2

Nim implementation of the Ethereum Beacon Chain
https://nimbus.guide
Other
543 stars 233 forks source link

`QUANTITY` fields check of Execution Payload is too relaxed #3844

Open marioevz opened 2 years ago

marioevz commented 2 years ago

Describe the bug The QUANTITY fields of the ExecutionPayloadV1 in the Engine API (https://github.com/ethereum/execution-apis/blob/main/src/engine/specification.md) must comply to the following definition:

Values of a field of QUANTITY type MUST be encoded as a hexadecimal string with a 0x prefix and the leading 0s stripped (except for the case of encoding the value 0) matching the regular expression ^0x(?:0|(?:[a-fA-F1-9][a-fA-F0-9]*))$.

Note: Byte order of encoded value having QUANTITY type is big-endian.

Currently Nimbus is allowing some Quantity types pass unchecked from the execution client in test invalid-quantity-fields of the eth2/engine simulator in hive.

The test verifies fields blockNumber (64 bitsize), gasLimit (64 bitsize), gasUsed (64 bitsize), timestamp (64 bitsize), baseFeePerGas (256 bitsize), with the following invalidations:

To Reproduce Use branch https://github.com/marioevz/hive/tree/all-tests-plus-cl-clients and run

./hive --client go-ethereum,nimbus-bn,nimbus-vc --sim eth2/engine --sim.loglevel 5
tersec commented 2 years ago

https://github.com/status-im/nim-web3/pull/53 addresses this.

tersec commented 2 years ago

https://github.com/status-im/nimbus-eth2/pull/3850 points out some spec points around this.