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.

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

upstream: merge branch 'geth-master-02-11' into bsc-develop #2888

Merged
merged 10 commits into from
Feb 12, 2025

Conversation

buddh0
Copy link
Collaborator

@buddh0 buddh0 commented Feb 12, 2025

Description

upstream: merge branch 'geth-master-02-11' into bsc-develop

Rationale

need PR core/txpool/legacypool: add support for SetCode transactions (#31073) in prague upgrade

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

fjl and others added 9 commits February 6, 2025 15:06
After recent changes in Geth (removing TD):

ethereum/go-ethereum@39638c8#diff-d70a44d4b7a0e84fe9dcca25d368f626ae6c9bc0b8fe9690074ba92d298bcc0d

Non-Geth clients are failing many devp2p tests with an error:
`peering failed: status exchange failed: wrong TD in status: have 1 want 0`

Right now only Geth is passing it - all other clients are affected by
this change. I think there should be no validation of TD when checking `Status`
message in hive tests. Now Geth has 0 (and hive tests requires 0) and
all other clients have actual TD. And on real networks there is no validation
of TD when peering
Agreed to the following fork dates for Holesky and Sepolia on ACDC 150

Holesky slot: 3710976	(Mon, Feb 24 at 21:55:12 UTC)
Sepolia slot: 7118848	(Wed, Mar 5 at 07:29:36 UTC)
This removes the method `TestingTTDBlock` introduced by #30744. It was
added to make the beacon consensus engine aware of the merge block in
tests without relying on the total difficulty. However, tracking the
merge block this way is very annoying. We usually configure forks in the
`ChainConfig`, but the method is on the consensus engine, which isn't
always created in the same place. By sidestepping the `ChainConfig` we
don't get the usual fork-order checking, so it's possible to enable the
merge before the London fork, for example. This in turn can lead to very
hard-to-debug outputs and validation errors.

So here I'm changing the consensus engine to check the
`MergeNetsplitBlock` instead. Alternatively, we assume a network is
merged if it has a `TerminalTotalDifficulty` of zero, which is a very
common configuration in tests.
The new SetCode transaction type introduces some additional complexity
when handling the transaction pool.

This complexity stems from two new account behaviors:

1. The balance and nonce of an account can change during regular
   transaction execution *when they have a deployed delegation*.
2. The nonce and code of an account can change without any EVM execution
   at all. This is the "set code" mechanism introduced by EIP-7702.

The first issue has already been considered extensively during the design
of ERC-4337, and we're relatively confident in the solution of simply
limiting the number of in-flight pending transactions an account can have
to one. This puts a reasonable bound on transaction cancellation. Normally
to cancel, you would need to spend 21,000 gas. Now it's possible to cancel
for around the cost of warming the account and sending value
(`2,600+9,000=11,600`). So 50% cheaper.

The second issue is more novel and needs further consideration.
Since authorizations are not bound to a specific transaction, we
cannot drop transactions with conflicting authorizations. Otherwise,
it might be possible to cherry-pick authorizations from txs and front
run them with different txs at much lower fee amounts, effectively DoSing
the authority. Fortunately, conflicting authorizations do not affect the
underlying validity of the transaction so we can just accept both.

---------

Co-authored-by: Marius van der Wijden <[email protected]>
Co-authored-by: Felix Lange <[email protected]>
Fixes an error when the block is not found in debug methods.
@buddh0 buddh0 marked this pull request as ready for review February 12, 2025 06:02
@zzzckck zzzckck merged commit d1ead90 into bnb-chain:develop Feb 12, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants