-
Notifications
You must be signed in to change notification settings - Fork 997
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.
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
Release v1.5.0-alpha.9 #3982
Merged
Release v1.5.0-alpha.9 #3982
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tions As part of the discussions surrounding EIP-7594 (peerdas), it was highlighted that during sampling and/or data requests, the sampler does not have timing information for when a samplee will have data available. It is desireable to not introduce a deadline, since this artificially introduces latency for the typical scenario where data becomes available earlier than an agreed-upon deadline. Similarly, when a client issues a request for blocks, it does often not know what rate limiting policy of the serving end and must either pessimistically rate limit itself or run the risk of getting disconnected for spamming the server - outcomes which lead to unnecessarily slow syncing as well as testnet mess with peer scoring and disconnection issues. This PR solves both problems by: * removing the time-to-first-byte and response timeouts allowing requesters to optimistically queue requests - the timeouts have historically not been implemented fully in clients to this date * introducing a hard limit in the number of concurrent requests that a client may issue, per protocol * introducing a recommendation for rate limiting that allows optimal bandwidth usage without protocol changes or additional messaging roundtrips On the server side, an "open" request does not consume significant resources while it's resting, meaning that allowing the server to manage resource allocation by slowing down data serving is safe, as long as concurrency is adequately limited. On the client side, clients must be prepared to handle slow servers already and they can simply apply their existing strategy both to uncertainty and rate-limiting scenarios (how long before timeout, what to do in "slow peer" scenarios). Token / leaky buckets are a classic option for rate limiting with desireable properties both for the case when we're sending requests to many clients concurrently (getting good burst performance) and when the requestees are busy (by keeping long-term resource usage in check and fairly serving clients)
Co-authored-by: Pop Chunhapanya <haxx.pop@gmail.com>
Transactions cannot be empty, they always have at least 1 byte. Random tests should produce valid CL data by default. There are still individual tests for invalid transactions.
As part of `newPayload` block hash verification, the `transactionsRoot` is computed by the EL. Because Merkle-Patricia Tries cannot contain `[]` entries, MPT implementations typically treat setting a key to `[]` as deleting the entry for the key. This means that if a CL receives a block with `transactions` containing one or more zero-length transactions, that such transactions will effectively be skipped when computing the `transactionsRoot`. Note that `transactions` are opaque to the CL and zero-length transactions are not filtered out before `newPayload`. ```python # https://eips.ethereum.org/EIPS/eip-2718 def compute_trie_root_from_indexed_data(data): """ Computes the root hash of `patriciaTrie(rlp(Index) => Data)` for a data array. """ t = HexaryTrie(db={}) for i, obj in enumerate(data): k = encode(i, big_endian_int) t.set(k, obj) # Implicitly skipped if `obj == b''` (invalid RLP) return t.root_hash ``` In any case, the `blockHash` validation may still succeed, resulting in a potential `SYNCING/ACCEPTED` result to `newPayload` by spec. Note, however, that there is an effective hash collision if a payload is modified by appending one or more zero-length transactions to the end of `transactions` list: In the trivial case, a block with zero transactions has the same `transactionsRoot` (and `blockHash`) as one of a block with one `[]` transaction (as that one is skipped). This means that the same `blockHash` can refer to a valid block (without extra `[]` transactions added), but also can refer to an invalid block. Because `forkchoiceUpdated` refers to blocks by `blockHash`, outcome may be nondeterministic and implementation dependent. If `forkchoiceUpdated` deems the `blockHash` to refer to a `VALID` object (obtained from a src that does not have the extra `[]` transactions, e.g., devp2p), then this could result in honest attestations to a CL beacon block with invalid `[]` transactions in its `ExecutionPayload`, risking finalizing it. The problem can be avoided by returning `INVALID` in `newPayload` if there are any zero-length `transactions` entries, preventing optimistic import of such blocks by the CL.
As a complement to #3787, this PR introduces a `SingleAttestation` type used for network propagation only. In Electra, the on-chain attestation format introduced in [EIP-7549](#3559) presents several difficulties - not only are the new fields to be interpreted differently during network processing and onchain which adds complexity in clients, they also introduce inefficiency both in hash computation and bandwidth. The new type puts the validator and committee indices directly in the attestation type, this simplifying processing and increasing security. * placing the validator index directly in the attestation allows verifying the signature without computing a shuffling - this closes a loophole where clients either must drop attestations or risk being overwhelmed by shuffling computations during attestation verification * the simpler "structure" of the attestation saves several hash calls during processing (a single-item List has significant hashing overhead compared to a field) * we save a few bytes here and there - we can also put stricter bounds on message size on the attestation topic because `SingleAttestation` is now fixed-size * the ambiguity of interpreting the `attestation_bits` list indices which became contextual under EIP-7549 is removed Because this change only affects the network encoding (and not block contents), the implementation impact on clients should be minimal.
EIP-4844 uses RLP not SSZ for blob transactions.
electra: Epoch processing tests
electra: Misc beacon chain operations tests
electra: Add more transition tests
…`, and `MAX_REQUEST_BLOB_SIDECARS_EIP7594`
eip7549: Ensure non-zero bits for each committee bitfield comprising an aggregate
Add `MAX_BLOBS_PER_BLOCK_EIP7594` and corresponding configs
Synchronously check all `transactions` to have non-zero length
Request hash is not considered in `compute_el_block_hash`, have to use one of the other overloads for this to work.
Fix block hash computation for deposit transition tests
Request hash is not considered in `compute_el_block_hash`, have to use one of the other overloads for this to work.
Followup from #3987 to remove references to the deleted document.
Clean up dead link and typo in LC docs for Electra
Exclude empty requests in requests list
I think we missed #4001 in the release note |
Oops yes I did, good catch. Thank you! Updated the release notes. |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EIP-7742: Uncouple blob limits per block across CL and EL #3800transactions
to have non-zero length #3885Make initial Fulu spec #3994get_attesting_indices()
#3996MAX_BLOBS_PER_BLOCK_EIP7594
and corresponding configs #4008Restrict best LC update collection to canonical blocks #3553work-in-progress release notes
v1.5.0-alpha.9
-- Ampharos -- is the alpha release for the coming Electra upgrade.PR showing full diff can be found here:
#3982
Electra
#3800
#3900
#3976
get_validator_from_deposit
#3978
#3979
get_attesting_indices
#3996
#3998
#4000
#4002
EIP-7594 (PeerDAS)
#3893
#3963
MAX_BLOBS_PER_BLOCK_EIP7594
and corresponding configs#4008
Networking
TTFB
,RESP_TIMEOUT
, introduce rate limiting recommendations#3767
Testing, repo, etc
#3754
#3884
#3885
#3904
Makefile
#3940
check_mods
function for generators#3970
README
, move outdated specs to new section#3973
#3975
#3980
#3984
#3986
#3990
pytest
warnings#3989
pylint
& split config files#3991
#3995