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

[pull] master from ethereum:master #1137

Open
wants to merge 1,005 commits into
base: master
Choose a base branch
from
Open

Conversation

pull[bot]
Copy link

@pull pull bot commented Dec 5, 2023

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

zhiqiangxu and others added 28 commits October 2, 2024 16:00
minimizes the time when the lock is held
This implements recent changes to EIP-7685, EIP-6110, and
execution-apis.

---------

Co-authored-by: lightclient <[email protected]>
Co-authored-by: Shude Li <[email protected]>
…30520)

This fixes `debug_traceBlock` methods for JS tracers in that it correctly
applies the beacon block root processing to the state.
A couple of tests set the debug level to `TRACE` on stdout,
and all subsequent tests in the same package are also affected
by that, resulting in outputs of tens of megabytes. 

This PR removes such calls from two packages where it was prevalent.
This makes getting a summary of failing tests simpler, and possibly
reduces some strain from the CI pipeline.
Block no longer has Requests. This PR just removes some code that wasn't removed in #30425.
Allows live custom tracers to access contract transient storage through the StateDB interface.
This is a redo of #29052 based on newer specs. Here we implement EIPs
scheduled for the Prague fork:

- EIP-7002: Execution layer triggerable withdrawals
- EIP-7251: Increase the MAX_EFFECTIVE_BALANCE

Co-authored-by: lightclient <[email protected]>
This fixes a few issues missed in #29052:

* `requests` must be hex encoded, so added a helper to marshal.
* The statedb was committed too early and so the result of the system
calls was lost.
* For devnet-4 we need to pull off the type byte prefix from the request
data.
This change makes the trie commit operation concurrent, if the number of changes exceed 100. 

Co-authored-by: stevemilk <[email protected]>
Co-authored-by: Gary Rong <[email protected]>
Changelog: https://golangci-lint.run/product/changelog/#1610 

Removes `exportloopref` (no longer needed), replaces it with
`copyloopvar` which is basically the opposite.

Also adds: 
- `durationcheck`
- `gocheckcompilerdirectives`
- `reassign`
- `mirror`
- `tenv`

---------

Co-authored-by: Marius van der Wijden <[email protected]>
This change brings geth into compliance with the current engine API
specification for the Prague fork. I have moved the assignment of
ExecutionPayloadEnvelope.Requests into BlockToExecutableData to ensure
there is a single place where the type is removed.

While doing so, I noticed that handling of requests in the miner was not
quite correct for the empty payload. It would return `nil` requests for
the empty payload even for blocks after the Prague fork. To fix this, I
have added the emptyRequests field in miner.Payload.
calculating a reasonable tx blob fee cap (`max_blob_fee_per_gas *
total_blob_gas`) only depends on the excess blob gas of the parent
header. The parent header is assumed to be correct, so the method should
not be able to fail and return an error.
Use `github.com/decred/dcrd/dcrec/secp256k1/v4` directly rather than
`github.com/btcsuite/btcd/btcec/v2` which is just a wrapper around the
underlying decred library. Inspired by
cosmos/cosmos-sdk#15018

`github.com/btcsuite/btcd/btcec/v2` has a very annoying breaking change
when upgrading from `v2.3.3` to `v2.3.4`. The easiest way to workaround
this is to just remove the wrapper.

Would be very nice if you could backport this to the release branches.

References:
- btcsuite/btcd#2221
- cometbft/cometbft#4294
- cometbft/cometbft#3728
- zeta-chain/node#2934
## Description

Omit null `witness` field from payload envelope.

## Motivation

Currently, JSON encoded payload types always include `"witness": null`,
which, I believe, is not intentional.
~~Opening this as a draft to have a discussion.~~ Pressed the wrong
button
I had [a previous PR
](#24616 long time ago
which reduced the peak memory used during reorgs by not accumulating all
transactions and logs.
This PR reduces the peak memory further by not storing the blocks in
memory.
However this means we need to pull the blocks back up from storage
multiple times during the reorg.
I collected the following numbers on peak memory usage: 

// Master: BenchmarkReorg-8 10000 899591 ns/op 820154 B/op 1440
allocs/op 1549443072 bytes of heap used
// WithoutOldChain: BenchmarkReorg-8 10000 1147281 ns/op 943163 B/op
1564 allocs/op 1163870208 bytes of heap used
// WithoutNewChain: BenchmarkReorg-8 10000 1018922 ns/op 943580 B/op
1564 allocs/op 1171890176 bytes of heap used

Each block contains a transaction with ~50k bytes and we're doing a 10k
block reorg, so the chain should be ~500MB in size

---------

Co-authored-by: Péter Szilágyi <[email protected]>
Breaking changes:

- The ChainConfig was exposed to tracers via VMContext passed in
`OnTxStart`. This is unnecessary specially looking through the lens of
live tracers as chain config remains the same throughout the lifetime of
the program. It was there so that native API-invoked tracers could
access it. So instead we moved it to the constructor of API tracers.

Non-breaking:

- Change the default config of the tracers to be `{}` instead of nil.
This way an extra nil check can be avoided.

Refactoring:

- Rename `supply` struct to `supplyTracer`.
- Un-export some hook definitions.
Fixes an issue missed in #30576 where we send empty requests for a full
payload being resolved, causing hash mismatch later on when we get the
payload back via `NewPayload`.
Swarm moved out more than 5 years ago, time to let it go.
Clean out some ancient stuff from git ignore.
0xkazak and others added 30 commits February 4, 2025 07:09
Fixes a typo in the error message within the `fuzzCrossG2Add`
function. The panic message incorrectly references "G1 point addition
mismatch" when it should be "G2 point addition mismatch," as the
function deals with G2 points.

This doesn't affect functionality but could cause confusion during
debugging. I've updated the message to reflect the correct curve.
This is a follow-up PR to #29792 to get rid of the data file sync.

**This is a non-backward compatible change, which increments the
database version from 8 to 9**.

We introduce a flushOffset for each freezer table, which tracks the position
of the most recently fsync’d item in the index file. When this offset moves
forward, it indicates that all index entries below it, along with their corresponding
data items, have been properly persisted to disk. The offset can also be moved
backward when truncating from either the head or tail of the file.

Previously, the data file required an explicit fsync after every mutation, which
was highly inefficient. With the introduction of the flush offset, the synchronization
strategy becomes more flexible, allowing the freezer to sync every 30 seconds
instead.

The data items above the flush offset are regarded volatile and callers must ensure
they are recoverable after the unclean shutdown, or explicitly sync the freezer
before any proceeding operations.

---------

Co-authored-by: Felix Lange <[email protected]>
This PR fixes a data race in SetupGenesisWithOverride.
This PR defines the Osaka fork. An easy first step to start our work on
the next hardfork

(This is needed for EOF testing as well)

---------

Co-authored-by: lightclient <[email protected]>
)

I hit this case while trying something with the simulated backend. The
EVM only enables instruction set forks after the merge when 'Random' is
set. In the simulated backend, the random value will be set via the
engine API for all blocks after genesis. But for the genesis block
itself, the random value will not be assigned in the vm.BlockContext
because the genesis has a non-zero difficulty. For my case, this meant
that estimateGas did not work for the first transaction sent on the
simulated chain, since the contract contained a PUSH0 instruction.

This could also be fixed by explicitly configuring a zero difficulty in
the simulated backend. However, I think that zero difficulty is a better
default these days.

---------

Co-authored-by: lightclient <[email protected]>
Replaces  #29297, descendant from #27535

---------

This PR removes `locals` as a concept from transaction pools. Therefore,
the pool now acts as very a good simulation/approximation of how our
peers' pools behave. What this PR does instead, is implement a
locals-tracker, which basically is a little thing which, from time to
time, asks the pool "did you forget this transaction?". If it did, the
tracker resubmits it.

If the txpool _had_ forgotten it, chances are that the peers had also
forgotten it. It will be propagated again.

Doing this change means that we can simplify the pool internals, quite a
lot.

### The semantics of `local` 

Historically, there has been two features, or usecases, that has been
combined into the concept of `locals`.

1. "I want my local node to remember this transaction indefinitely, and
resubmit to the network occasionally"
2. "I want this (valid) transaction included to be top-prio for my
miner"


This PR splits these features up, let's call it `1: local` and `2:
prio`. The `prio` is not actually individual transaction, but rather a
set of `address`es to prioritize.
The attribute `local` means it will be tracked, and `prio` means it will
be prioritized by miner.

For `local`: anything transaction received via the RPC is marked as
`local`, and tracked by the tracker.
For `prio`: any transactions from this sender is included first, when
building a block. The existing commandline-flag `--txpool.locals` sets
the set of `prio` addresses.

---------

Co-authored-by: Gary Rong <[email protected]>
Fixes the linter on master which was broken by
#30559
A clarification was made to EIP-7691 stating that at the fork boundary
it is required to use the target blob count associated with the head
block, rather than the parent as implemented here.

See for more: ethereum/EIPs#9249
This PR changes the signature of `CalcExcessBlobGas` to take in just
the header timestamp instead of the whole object. It also adds a sanity
check for the parent->child block order to `VerifyEIP4844Header`.
Here we add some more changes for live tracing API v1.1:

- Hook `OnSystemCallStartV2` was introduced with `VMContext` as parameter.
- Hook `OnBlockHashRead` was introduced.
- `GetCodeHash` was added to the state interface
- The new `WrapWithJournal` construction helps with tracking EVM reverts in the tracer.

---------

Co-authored-by: Felix Lange <[email protected]>
After recent changes in Geth (removing TD):

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.
This fixes an error where executing `evm run --dump ...` omits preimages
from the dump (because the statedb used for execution is a copy of
another instance).
closes #31072

BLST released their newest version which includes a fix for go v.1.24:
https://github.com/supranational/blst/releases/tag/v0.3.14

I went through all commits between 0.3.14 and 0.3.13 for a sanity check
This is to prevent a crash on startup with a custom genesis configuration.
With this change in place, upgrading a chain created by geth v1.14.x and
below will now print an error instead of crashing:

    Fatal: Failed to register the Ethereum service: invalid chain configuration: missing entry for fork "cancun" in blobSchedule

Arguably this is not great, and it should just auto-upgrade the config.
We'll address this in a follow-up PR for geth v1.15.2
This PR addresses a flaw in the freezer table upgrade path.

In v1.15.0, freezer table v2 was introduced, including an additional 
field (`flushOffset`) maintained in the metadata file. To ensure 
backward compatibility, an upgrade path was implemented for legacy
freezer tables by setting `flushOffset` to the size of the index file.

However, if the freezer table is opened in read-only mode, this file 
write operation is rejected, causing Geth to shut down entirely.

Given that invalid items in the freezer index file can be detected and 
truncated, all items in freezer v0 index files are guaranteed to be
complete. Therefore, when operating in read-only mode, it is safe to
use the  freezer data without performing an upgrade.
Currently, when calculating block's bloom, we loop through all the
receipt logs to calculate the hash value. However, normally, after going
through applyTransaction, the receipt's bloom is already calculated
based on the receipt log, so the block's bloom can be calculated by just
ORing these receipt's blooms.
```
goos: darwin
goarch: arm64
pkg: github.com/ethereum/go-ethereum/core/types
cpu: Apple M1 Pro
BenchmarkCreateBloom
BenchmarkCreateBloom/small
BenchmarkCreateBloom/small-10             810922              1481 ns/op             104 B/op          5 allocs/op
BenchmarkCreateBloom/large
BenchmarkCreateBloom/large-10               8173            143764 ns/op            9614 B/op        401 allocs/op
BenchmarkCreateBloom/small-mergebloom
BenchmarkCreateBloom/small-mergebloom-10                 5178918               232.0 ns/op             0 B/op          0 allocs/op
BenchmarkCreateBloom/large-mergebloom
BenchmarkCreateBloom/large-mergebloom-10                   54110             22207 ns/op               0 B/op          0 allocs/op
```

---------

Co-authored-by: Gary Rong <[email protected]>
Co-authored-by: Zsolt Felfoldi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.