Fixes an issue I accidentally introduced in #32579. Essentially, because
we gate the engine methods based on particular forks and I did not add
the BPOs as allowed forks to the method.
Another getBlobs PR on top of
https://github.com/ethereum/go-ethereum/pull/32190 to avoid some minor
regressions.
- bring back more log messages from before
- continue processing also on some internal errors
- ensure v2 complies with spec even if there are internal errors
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
This pull request fixes a regression, introduced in #32190
Specifically, in GetBlobsV1 engine API, if any blob is missing, the null
should be placed in
response, unfortunately a behavioral change was introduced in #32190,
returning an error
instead.
What's more, a more comprehensive test suite is added to cover both
`GetBlobsV1` and
`GetBlobsV2` endpoints.
- If all the `vhashes` are in the same `sidecar`, then it will load the
same blob tx many times. This PR aims to upgrade this.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Correct the error message in the ExecuteStatelessPayloadV4 function to
reference newPayloadV4 and the Prague fork, instead of incorrectly
referencing newPayloadV3 and Cancun.
This improves clarity during debugging and aligns the error message with
the actual function and fork being validated. No logic is changed.
---------
Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
The main purpose of this change is to enforce the version setting when
constructing the blobSidecar, avoiding creating sidecar with wrong/default
version tag.
alternate approach to https://github.com/ethereum/go-ethereum/pull/31328
suggested by @MariusVanDerWijden . This prevents Geth from outputting a
lot of logs when trying to commit on-demand dev mode blocks while the
client is shutting down.
The issue is hard to reproduce, but I've seen it myself and it is
annoying when it happens. I think this is a reasonable simple solution,
and we can revisit if we find that the output is still too large (i.e.
there is a large delay between initiating shut down and the simulated
beacon receiving the signal, while in this loop).
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
geth cmd: `geth --dev --dev.period 5`
call: `debug.setHead` to rollback several blocks.
If the `debug.setHead` call is delayed, it will trigger a panic with a
small probability, due to using the null point of
`fcResponse.PayloadID`.
---------
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
This situation was failing quietly for me recently when I had a partial
data corruption issue. Changing the log level to Error would increase
visibility for me.
This PR contains three refactors:
- refactor the latest fork check that we use quite extensively
- refactor the nil checks in NewPayloads
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
closes#31401
---------
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
before this changes, this will result in numerous test failures:
```
> go test -run=Eth2AssembleBlock -c
> stress ./catalyst.test
```
The reason is that after creating/inserting the test chain, there is a
race between the txpool head reset and the promotion of txs added from
tests.
Ensuring that the txpool state is up to date with the head of the chain
before proceeding fixes these flaky tests.
this adds 2 features to improve `geth --dev` experience.
1. we don't need to use `dev_SetFeeRecipient` to set initial coinbase
address. it was a pain.
2. we don't need to unlock keystore if we don't use it. we had it
because of clique.
This pull request enhances the unit test, avoiding unnecessary failure
in CI.
```
--- FAIL: TestSimulatedBeaconSendWithdrawals (12.08s)
simulated_beacon_test.go:139: timed out without including all withdrawals/txs
FAIL
```
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.
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 <garyrong0905@gmail.com>
The total difficulty is the sum of all block difficulties from genesis
to a certain block. This value was used in PoW for deciding which chain
is heavier, and thus which chain to select. Since PoS has a different
fork selection algorithm, all blocks since the merge have a difficulty
of 0, and all total difficulties are the same for the past 2 years.
Whilst the TDs are mostly useless nowadays, there was never really a
reason to mess around removing them since they are so tiny. This
reasoning changes when we go down the path of pruned chain history. In
order to reconstruct any TD, we **must** retrieve all the headers from
chain head to genesis and then iterate all the difficulties to compute
the TD.
In a world where we completely prune past chain segments (bodies,
receipts, headers), it is not possible to reconstruct the TD at all. In
a world where we still keep chain headers and prune only the rest,
reconstructing it possible as long as we process (or download) the chain
forward from genesis, but trying to snap sync the head first and
backfill later hits the same issue, the TD becomes impossible to
calculate until genesis is backfilled.
All in all, the TD is a messy out-of-state, out-of-consensus computed
field that is overall useless nowadays, but code relying on it forces
the client into certain modes of operation and prevents other modes or
other optimizations. This PR completely nukes out the TD from the node.
It doesn't compute it, it doesn't operate on it, it's as if it didn't
even exist.
Caveats:
- Whenever we have APIs that return TD (devp2p handshake, tracer, etc.)
we return a TD of 0.
- For era files, we recompute the TD during export time (fairly quick)
to retain the format content.
- It is not possible to "verify" the merge point (i.e. with TD gone, TTD
is useless). Since we're not verifying PoW any more, just blindly trust
it, not verifying but blindly trusting the many year old merge point
seems just the same trust model.
- Our tests still need to be able to generate pre and post merge blocks,
so they need a new way to split the merge without TTD. The PR introduces
a settable ttdBlock field on the consensus object which is used by tests
as the block where originally the TTD happened. This is not needed for
live nodes, we never want to generate old blocks.
- One merge transition consensus test was disabled. With a
non-operational TD, testing how the client reacts to TTD is useless, it
cannot react.
Questions:
- Should we also drop total terminal difficulty from the genesis json?
It's a number we cannot react on any more, so maybe it would be cleaner
to get rid of even more concepts.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Lots of packages depend on eth/downloader just for the SyncMode type.
Since we have a dedicated package for eth protocol configuration, it
makes more sense to define SyncMode there, turning eth/downloader into
more of a leaf package.
* unify `staterunner` and `blockrunner` CLI flags, especially around
tracing
* added support for struct logger or json logging (although having issue
#30658)
* new --cross-check flag to validate the stateless witness collection
/ execution matches stateful
* adds support for tracing the stateless execution when a tracer is set
(to more easily debug differences)
* --human for more readable test summary
* directory or file input, so if you pass tests/spec-tests/fixtures/blockchain_tests it will execute all
blockchain tests
This adds an API method `DropTransactions` to legacy pool, blob pool and
txpool interface. This method removes all txs currently tracked in the
pools.
It modifies the simulated beacon to use the new method in `Rollback`
which removes previous hacky implementation that also erroneously reset
the gas tip to 1 gwei.
---------
Co-authored-by: Felix Lange <fjl@twurst.com>
This test depends on a 100ms timer, which fails quite often, messing up
our pipelines. Hook directly into the internal version of getPayload
which has the capacity to wait for the full payload before returning.
This might not be absolutely correct from a test perspective, but it
beats failing ci. The alternative would be to expose the full build hook
into the outside, but it might be a bit overkill for this scenario.
rebased https://github.com/ethereum/go-ethereum/pull/29766 . The
downstream branch appears to have been deleted and I don't have perms to
push to that fork.
`TerminalTotalDifficultyPassed` is removed. `TerminalTotalDifficulty`
must now be non-nil, and it is expected that networks are already
merged: we can only import PoW/Clique chains, not produce blocks on
them.
---------
Co-authored-by: stevemilk <wangpeculiar@gmail.com>
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.
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 <lightclient@protonmail.com>
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.
This implements recent changes to EIP-7685, EIP-6110, and
execution-apis.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
Co-authored-by: Shude Li <islishude@gmail.com>
Reverts ethereum/go-ethereum#30495
You are free to create a proper Clear method if that's the best way. But
one that does a proper cleanup, not some hacky call to set gas which
screws up logs, metrics and everything along the way. Also doesn't work
for legacy pool local transactions.
The current code had a hack in the simulated code, now we have a hack in
live txpooling code. No, that's not acceptable. I want the live code to
be proper, meaningful API, meaningful comments, meaningful
implementation.
Here we move the method that drops all transactions by temporarily increasing the fee
into the TxPool itself. It's better to have it there because we can set it back to the
configured value afterwards. This resolves a TODO in the simulated backend.
