From 3e320dacff74d9fe876f351d4488eae59f59831d Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 20 Oct 2021 12:06:58 -0600 Subject: [PATCH 1/4] fix gossip and tx size limits for the merge --- presets/mainnet/merge.yaml | 4 +-- presets/minimal/merge.yaml | 4 +-- specs/merge/beacon-chain.md | 2 +- specs/merge/p2p-interface.md | 49 +++++++++++++++++++++++++++++++++--- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 66f7abe282..10834dcc37 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -2,8 +2,8 @@ # Execution # --------------------------------------------------------------- -# 2**20 (= 1,048,576) -MAX_BYTES_PER_TRANSACTION: 1048576 +# 2**24 (= 16,777,216) +MAX_BYTES_PER_TRANSACTION: 16777216 # 2**14 (= 16,384) MAX_TRANSACTIONS_PER_PAYLOAD: 16384 # 2**8 (= 256) diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index d9b5640ddd..ec2dd384d8 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -2,8 +2,8 @@ # Execution # --------------------------------------------------------------- -# 2**20 (= 1,048,576) -MAX_BYTES_PER_TRANSACTION: 1048576 +# 2**24 (= 16,777,216) +MAX_BYTES_PER_TRANSACTION: 16777216 # 2**14 (= 16,384) MAX_TRANSACTIONS_PER_PAYLOAD: 16384 # 2**8 (= 256) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 8fafb07206..9bb7db6c91 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -59,7 +59,7 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | Value | | - | - | -| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**20)` (= 1,048,576) | +| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**24)` (= 16,777,216) | | `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**14)` (= 16,384) | | `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) | | `GAS_LIMIT_DENOMINATOR` | `uint64(2**10)` (= 1,024) | diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index 3cbba2e234..e3eb3b152c 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -14,6 +14,7 @@ Readers should understand the Phase 0 and Altair documents and use them as a bas - [Warning](#warning) - [Modifications in the Merge](#modifications-in-the-merge) + - [Configuration](#configuration) - [The gossip domain: gossipsub](#the-gossip-domain-gossipsub) - [Topics and messages](#topics-and-messages) - [Global topics](#global-topics) @@ -23,6 +24,9 @@ Readers should understand the Phase 0 and Altair documents and use them as a bas - [Messages](#messages) - [BeaconBlocksByRange v2](#beaconblocksbyrange-v2) - [BeaconBlocksByRoot v2](#beaconblocksbyroot-v2) +- [Design decision rationale](#design-decision-rationale) + - [Gossipsub](#gossipsub) + - [Why was the max gossip message size increased at the Merge?](#why-was-the-max-gossip-message-size-increased-at-the-merge) @@ -34,6 +38,14 @@ Refer to the note in the [validator guide](./validator.md) for further details. # Modifications in the Merge +## Configuration + +This section outlines modifications constants that are used in this spec. + +| Name | Value | Description | +|---|---|---| +| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10485760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | + ## The gossip domain: gossipsub Some gossip meshes are upgraded in the Merge to support upgraded types. @@ -43,7 +55,12 @@ Some gossip meshes are upgraded in the Merge to support upgraded types. Topics follow the same specification as in prior upgrades. All topics remain stable except the beacon block topic which is updated with the modified type. -The specification around the creation, validation, and dissemination of messages has not changed from the Phase 0 and Altair documents. +The specification around the creation, validation, and dissemination of messages has not changed from the Phase 0 and Altair documents unless explicitly noted here. + +Starting at the Merge upgrade, each gossipsub [message](https://github.com/libp2p/go-libp2p-pubsub/blob/master/pb/rpc.proto#L17-L24) +has a maximum size of `GOSSIP_MAX_SIZE_MERGE`. +Clients MUST reject (fail validation) messages that are over this size limit. +Likewise, clients MUST NOT emit or propagate messages larger than this limit. The derivation of the `message-id` remains stable. @@ -75,8 +92,7 @@ Alias `block = signed_beacon_block.message`, `execution_payload = block.body.exe - _[REJECT]_ Gas used is less than the gas limit -- i.e. `execution_payload.gas_used <= execution_payload.gas_limit`. - _[REJECT]_ The execution payload transaction list data is within expected size limits, - the data MUST NOT be larger than the SSZ list-limit, - and a client MAY be more strict. + the data MUST NOT be larger than the SSZ list-limit. *Note*: Additional [gossip validations](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#block-encoding-and-validity) (see block "data validity" conditions) that rely more heavily on execution-layer state and logic are currently under consideration. @@ -123,3 +139,30 @@ Per `context = compute_fork_digest(fork_version, genesis_validators_root)`: | `GENESIS_FORK_VERSION` | `phase0.SignedBeaconBlock` | | `ALTAIR_FORK_VERSION` | `altair.SignedBeaconBlock` | | `MERGE_FORK_VERSION` | `merge.SignedBeaconBlock` | + +# Design decision rationale + +## Gossipsub + +### Why was the max gossip message size increased at the Merge? + +With the addition of `ExecutionPayload` to `BeaconBlock`s, there is a dynamic +field -- `transactions` -- which can validly exceed the `GOSSIP_MAX_SIZE` limit (1 MiB) put in place in +place at Phase 0. At the `GAS_LIMIT` (~30M) currently seen on mainnet in 2021, a single transaction +filled entirely with data at a cost of 16 gas per byte can create a valid +`ExecutionPayload` of ~2 MiB. Thus we need a size limit to at least account for +current mainnet conditions. + +Geth currently has a [max gossip message size](https://github.com/ethereum/go-ethereum/blob/3ce9f6d96f38712f5d6756e97b59ccc20cc403b3/eth/protocols/eth/protocol.go#L49) of 10 MiB. +To support backward compatibility with this previously defined network limit, +we adopt `GOSSIP_MAX_SIZE_MERGE` of 10 MiB for maximum gossip sizes at the +point of the Merge and beyond. Note, that clients SHOULD still reject objects +that exceed their maximum theoretical bounds which in most cases is less than `GOSSIP_MAX_SIZE_MERGE`. + +Note, that due to additional size induced by the `BeaconBlock` contents (e.g. +proposer signature, operations lists, etc) this does reduce the +theoretical max valid `ExecutionPayload` (and `transactions` list) size as +slightly lower than 10 MiB. Considering that `BeaconBlock` max size is on the +order of 128 KiB in the worst case and the current gas limit (~30M) bounds max blocksize to less +than 2 MiB today, this marginal difference in theoretical bounds will have zero +impact on network functionality and security. From dae5b87c2ba61cab8608fc906c641c6ef92f33d6 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 21 Oct 2021 15:56:58 -0600 Subject: [PATCH 2/4] increase TX sizes to account for very high gas limits --- presets/mainnet/merge.yaml | 8 ++++---- presets/minimal/merge.yaml | 8 ++++---- specs/merge/beacon-chain.md | 4 ++-- specs/merge/p2p-interface.md | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/presets/mainnet/merge.yaml b/presets/mainnet/merge.yaml index 10834dcc37..3deac4574c 100644 --- a/presets/mainnet/merge.yaml +++ b/presets/mainnet/merge.yaml @@ -2,10 +2,10 @@ # Execution # --------------------------------------------------------------- -# 2**24 (= 16,777,216) -MAX_BYTES_PER_TRANSACTION: 16777216 -# 2**14 (= 16,384) -MAX_TRANSACTIONS_PER_PAYLOAD: 16384 +# 2**30 (= 1,073,741,824) +MAX_BYTES_PER_TRANSACTION: 1073741824 +# 2**20 (= 1,048,576) +MAX_TRANSACTIONS_PER_PAYLOAD: 1048576 # 2**8 (= 256) BYTES_PER_LOGS_BLOOM: 256 # 2**10 (= 1,024) diff --git a/presets/minimal/merge.yaml b/presets/minimal/merge.yaml index ec2dd384d8..5df32ff579 100644 --- a/presets/minimal/merge.yaml +++ b/presets/minimal/merge.yaml @@ -2,10 +2,10 @@ # Execution # --------------------------------------------------------------- -# 2**24 (= 16,777,216) -MAX_BYTES_PER_TRANSACTION: 16777216 -# 2**14 (= 16,384) -MAX_TRANSACTIONS_PER_PAYLOAD: 16384 +# 2**30 (= 1,073,741,824) +MAX_BYTES_PER_TRANSACTION: 1073741824 +# 2**20 (= 1,048,576) +MAX_TRANSACTIONS_PER_PAYLOAD: 1048576 # 2**8 (= 256) BYTES_PER_LOGS_BLOOM: 256 # 2**10 (= 1,024) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 9bb7db6c91..aa2a771c5f 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -59,8 +59,8 @@ This patch adds transaction execution to the beacon chain as part of the Merge f | Name | Value | | - | - | -| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**24)` (= 16,777,216) | -| `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**14)` (= 16,384) | +| `MAX_BYTES_PER_TRANSACTION` | `uint64(2**30)` (= 1,073,741,824) | +| `MAX_TRANSACTIONS_PER_PAYLOAD` | `uint64(2**20)` (= 1,048,576) | | `BYTES_PER_LOGS_BLOOM` | `uint64(2**8)` (= 256) | | `GAS_LIMIT_DENOMINATOR` | `uint64(2**10)` (= 1,024) | | `MIN_GAS_LIMIT` | `uint64(5000)` (= 5,000) | diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index e3eb3b152c..4283a05176 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -44,7 +44,7 @@ This section outlines modifications constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10485760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | +| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | ## The gossip domain: gossipsub From bda07e15ad4518acaae7745aa3baca23ebaaf6e9 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 26 Oct 2021 13:42:31 -0600 Subject: [PATCH 3/4] remove extraneous gossip condition --- specs/merge/p2p-interface.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index 4283a05176..ff37bd6e3e 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -91,8 +91,6 @@ Alias `block = signed_beacon_block.message`, `execution_payload = block.body.exe -- i.e. `execution_payload.timestamp == compute_timestamp_at_slot(state, block.slot)`. - _[REJECT]_ Gas used is less than the gas limit -- i.e. `execution_payload.gas_used <= execution_payload.gas_limit`. - - _[REJECT]_ The execution payload transaction list data is within expected size limits, - the data MUST NOT be larger than the SSZ list-limit. *Note*: Additional [gossip validations](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#block-encoding-and-validity) (see block "data validity" conditions) that rely more heavily on execution-layer state and logic are currently under consideration. From d58b1c57efd862009411cd2686e902ce745a443f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 27 Oct 2021 10:44:55 -0600 Subject: [PATCH 4/4] bump up MAX_CHUNK_SIZE at the merge as well --- specs/merge/p2p-interface.md | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/specs/merge/p2p-interface.md b/specs/merge/p2p-interface.md index ff37bd6e3e..ea945a1b34 100644 --- a/specs/merge/p2p-interface.md +++ b/specs/merge/p2p-interface.md @@ -27,6 +27,8 @@ Readers should understand the Phase 0 and Altair documents and use them as a bas - [Design decision rationale](#design-decision-rationale) - [Gossipsub](#gossipsub) - [Why was the max gossip message size increased at the Merge?](#why-was-the-max-gossip-message-size-increased-at-the-merge) + - [Req/Resp](#reqresp) + - [Why was the max chunk response size increased at the Merge?](#why-was-the-max-chunk-response-size-increased-at-the-merge) @@ -44,7 +46,8 @@ This section outlines modifications constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade | +| `GOSSIP_MAX_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed gossip messages starting at the Merge upgrade. | +| `MAX_CHUNK_SIZE_MERGE` | `10 * 2**20` (= 10,485,760, 10 MiB) | The maximum allowed size of uncompressed req/resp chunked responses starting at the Merge upgrade. | ## The gossip domain: gossipsub @@ -108,7 +111,12 @@ details on how to handle transitioning gossip topics for the Merge. **Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks_by_range/2/` -Request and Response remain unchanged. +Request and Response remain unchanged unless explicitly noted here. + +Starting at the Merge upgrade, +a global maximum uncompressed byte size of `MAX_CHUNK_SIZE_MERGE` MUST be applied to all method response chunks +regardless of type specific bounds that *MUST* also be respected. + The Merge fork-digest is introduced to the `context` enum to specify the Merge block type. Per `context = compute_fork_digest(fork_version, genesis_validators_root)`: @@ -164,3 +172,17 @@ slightly lower than 10 MiB. Considering that `BeaconBlock` max size is on the order of 128 KiB in the worst case and the current gas limit (~30M) bounds max blocksize to less than 2 MiB today, this marginal difference in theoretical bounds will have zero impact on network functionality and security. + +## Req/Resp + +### Why was the max chunk response size increased at the Merge? + +Similar to the discussion about the maximum gossip size increase, the +`ExecutionPayload` type can cause `BeaconBlock`s to exceed the 1 MiB bounds put +in place during Phase 0. + +As with the gossip limit, 10 MiB is selected because this is firmly below any +valid block sizes in the range of gas limits expected in the medium term. + +As with both gossip and req/rsp maximum values, type-specific limits should +always by simultaneously respected.