Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Synchronously check all transactions to have non-zero length #573

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Synchronously check all transactions to have non-zero length #573

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15bc86e
Synchronously check all `transactions` to have non-zero length
etan-status Aug 14, 2024
180eb4c
Allow `INVALID` on invalid transaction.
etan-status Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/engine/paris.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,26 +161,30 @@ The payload build process is specified as follows:

#### Specification

1. Client software **MUST** validate `blockHash` value as being equivalent to `Keccak256(RLP(ExecutionBlockHeader))`, where `ExecutionBlockHeader` is the execution layer block header (the former PoW block header structure). Fields of this object are set to the corresponding payload values and constant values according to the Block structure section of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#block-structure), extended with the corresponding section of [EIP-4399](https://eips.ethereum.org/EIPS/eip-4399#block-structure). Client software **MUST** run this validation in all cases even if this branch or any other branches of the block tree are in an active sync process.
1. Client software **MUST** validate that all `transactions` have non-zero length (at least 1 byte). Client software **MUST** run this validation in all cases even if this branch or any other branches of the block tree are in an active sync process.

2. Client software **MAY** initiate a sync process if requisite data for payload validation is missing. Sync process is specified in the [Sync](#sync) section.
2. Client software **MUST** validate `blockHash` value as being equivalent to `Keccak256(RLP(ExecutionBlockHeader))`, where `ExecutionBlockHeader` is the execution layer block header (the former PoW block header structure). Fields of this object are set to the corresponding payload values and constant values according to the Block structure section of [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#block-structure), extended with the corresponding section of [EIP-4399](https://eips.ethereum.org/EIPS/eip-4399#block-structure). Client software **MUST** run this validation in all cases even if this branch or any other branches of the block tree are in an active sync process.

3. Client software **MUST** validate the payload if it extends the canonical chain and requisite data for the validation is locally available. The validation process is specified in the [Payload validation](#payload-validation) section.
3. Client software **MAY** initiate a sync process if requisite data for payload validation is missing. Sync process is specified in the [Sync](#sync) section.

4. Client software **MAY NOT** validate the payload if the payload doesn't belong to the canonical chain.
4. Client software **MUST** validate the payload if it extends the canonical chain and requisite data for the validation is locally available. The validation process is specified in the [Payload validation](#payload-validation) section.

5. Client software **MUST** respond to this method call in the following way:
5. Client software **MAY NOT** validate the payload if the payload doesn't belong to the canonical chain.

6. Client software **MUST** respond to this method call in the following way:
* `{status: INVALID, latestValidHash: null, validationError: errorMessage | null}` if `transactions` contains zero length entries
* `{status: INVALID_BLOCK_HASH, latestValidHash: null, validationError: errorMessage | null}` if the `blockHash` validation has failed
* `{status: INVALID, latestValidHash: 0x0000000000000000000000000000000000000000000000000000000000000000, validationError: errorMessage | null}` if terminal block conditions are not satisfied
* `{status: SYNCING, latestValidHash: null, validationError: null}` if requisite data for the payload's acceptance or validation is missing
* with the payload status obtained from the [Payload validation](#payload-validation) process if the payload has been fully validated while processing the call
* `{status: ACCEPTED, latestValidHash: null, validationError: null}` if the following conditions are met:
- all `transactions` have non-zero length
Copy link
Contributor

Choose a reason for hiding this comment

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

or we can change it to: all transactions de-serialize to valid transactions

Copy link
Author

Choose a reason for hiding this comment

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

while that would also cover the non-zero case, it's not strictly needed in newPayload and could be reported in forkchoiceUpdated as well. So far, the synchronous errors seem to cover the absolute minimum to ensure security, as in, to link the EL data with the blockHash.

Copy link
Author

Choose a reason for hiding this comment

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

agree though that it should at least be possible to return an INVALID if transactions fail to deserialize (even if not required)

- the `blockHash` of the payload is valid
- the payload doesn't extend the canonical chain
- the payload hasn't been fully validated
- ancestors of a payload are known and comprise a well-formed chain.

6. If any of the above fails due to errors unrelated to the normal processing flow of the method, client software **MUST** respond with an error object.
7. If any of the above fails due to errors unrelated to the normal processing flow of the method, client software **MUST** respond with an error object.

### engine_forkchoiceUpdatedV1

Expand Down