This PR fixes the out-of-range block number logic of `getBlockLvPointer`
which sometimes caused searches to fail if the head was updated in the
wrong moment. This logic ensures that querying the pointer of a future
block returns the pointer after the last fully indexed block (instead of
failing) and therefore an async range update will not cause the search
to fail. Earier this behaviour only worked when `headIndexed` was true
and `headDelimiter` pointed to the end of the indexed range. Now it also
works for an unfinished index.
This logic is also moved from `FilterMaps.getBlockLvPointer` to
`FilterMapsMatcherBackend.GetBlockLvPointer` because it is only required
by the search anyways. `FilterMaps.getBlockLvPointer` now only returns a
pointer for existing blocks, consistently with how it is used in the
indexer/renderer.
Note that this unhandled case has been present in the code for a long
time but went unnoticed because either one of two previously fixed bugs
did prevent it from being triggered; the incorrectly positive
`tempRange.headIndexed` (fixed in
https://github.com/ethereum/go-ethereum/pull/31680), though caused other
problems, prevented this one from being triggered as with a positive
`headIndexed` no database read was triggered in `getBlockLvPointer`.
Also, the unnecessary `indexLock` in `synced()` (fixed in
https://github.com/ethereum/go-ethereum/pull/31708) usually did prevent
the search seeing the temp range and therefore avoided noticeable
issues.
The functions `rpcRequest` and `batchRpcRequest` call `baseRpcRequest`.
And `resp.Body` will be closed in the function `baseRpcRequest` later by
`t.Cleanup`:
```go
func baseRpcRequest(t *testing.T, url, bodyStr string, extraHeaders ...string) *http.Response {
// ......
t.Cleanup(func() { resp.Body.Close() })
return resp
}
```
Add tests for GetBlockHeaders that verify client does not disconnect when unlikely block numbers are requested, e.g. max uint64.
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
Since the block hash is not returned for pending blocks, ethclient cannot unmarshal into RPC block. This makes hash optional on rpc block and compute the hash locally for pending blocks to correctly key the tx sender cache.
a82303f4e3/internal/ethapi/api.go (L500-L504)
---------
Co-authored-by: lightclient <lightclient@protonmail.com>
This changes the filtermaps to only pull up the raw receipts, not the
derived receipts which saves a lot of allocations.
During normal execution this will reduce the allocations of the whole
geth node by ~15%.
For PeerDAS, we need to compute cell proofs. Both ckzg and gokzg support
computing these cell proofs.
This PR does the following:
- Update the go-kzg library from "github.com/crate-crypto/go-kzg-4844"
to "github.com/crate-crypto/go-eth-kzg" which will be the new upstream
for go-kzg moving forward
- Update ckzg from v1.0.0 to v2.0.1 and switch to /v2
- Updates the trusted setup to contain the g1 points both in lagrange
and monomial form
- Expose `ComputeCells` to compute the cell proofs
This PR applies the config overrides to the new config as well,
otherwise they will not be applied to defined configs, making
shadowforks impossible.
To test:
```
> ./build/bin/geth --override.prague 123 --dev --datadir /tmp/geth
INFO [04-28|21:20:47.009] - Prague: @123
> ./build/bin/geth --override.prague 321 --dev --datadir /tmp/geth
INFO [04-28|21:23:59.760] - Prague: @321
``
This PR adds checking for an edgecase which theoretically can happen in
the range-prover. Right now, we check that a key does not overwrite a
previous one by checking that the key is increasing. However, if keys
are of different lengths, it is possible to create a key which is
increasing _and_ overwrites the previous key. Example: `0xaabbcc`
followed by `0xaabbccdd`.
This can not happen in go-ethereum, which always uses fixed-size paths
for accounts and storage slot paths in the trie, but it might happen if
the range prover is used without guaranteed fixed-size keys.
This PR also adds some testcases for the errors that are expected.
TruncatePending shows up bright red on our nodes, because it computes
the length of a map multiple times.
I don't know why this is so expensive, but around 20% of our time is
spent on this, which is super weird.
```
//PR: BenchmarkTruncatePending-24 17498 69397 ns/op 32872 B/op 3 allocs/op
//Master: BenchmarkTruncatePending-24 9960 123954 ns/op 32872 B/op 3 allocs/op
```
```
benchmark old ns/op new ns/op delta
BenchmarkTruncatePending-24 123954 69397 -44.01%
benchmark old allocs new allocs delta
BenchmarkTruncatePending-24 3 3 +0.00%
benchmark old bytes new bytes delta
BenchmarkTruncatePending-24 32872 32872 +0.00%
```
This simple PR is a 44% improvement over the old state
```
OUTINE ======================== github.com/ethereum/go-ethereum/core/txpool/legacypool.(*LegacyPool).truncatePending in github.com/ethereum/go-ethereum/core/txpool/legacypool/legacypool.go
1.96s 18.02s (flat, cum) 19.57% of Total
. . 1495:func (pool *LegacyPool) truncatePending() {
. . 1496: pending := uint64(0)
60ms 2.99s 1497: for _, list := range pool.pending {
250ms 5.48s 1498: pending += uint64(list.Len())
. . 1499: }
. . 1500: if pending <= pool.config.GlobalSlots {
. . 1501: return
. . 1502: }
. . 1503:
. . 1504: pendingBeforeCap := pending
. . 1505: // Assemble a spam order to penalize large transactors first
. 510ms 1506: spammers := prque.New[int64, common.Address](nil)
140ms 2.50s 1507: for addr, list := range pool.pending {
. . 1508: // Only evict transactions from high rollers
50ms 5.08s 1509: if uint64(list.Len()) > pool.config.AccountSlots {
. . 1510: spammers.Push(addr, int64(list.Len()))
. . 1511: }
. . 1512: }
. . 1513: // Gradually drop transactions from offenders
. . 1514: offenders := []common.Address{}
```
```go
// Benchmarks the speed of batch transaction insertion in case of multiple accounts.
func BenchmarkTruncatePending(b *testing.B) {
// Generate a batch of transactions to enqueue into the pool
pool, _ := setupPool()
defer pool.Close()
b.ReportAllocs()
batches := make(types.Transactions, 4096+1024+1)
for i := range len(batches) {
key, _ := crypto.GenerateKey()
account := crypto.PubkeyToAddress(key.PublicKey)
pool.currentState.AddBalance(account, uint256.NewInt(1000000), tracing.BalanceChangeUnspecified)
tx := transaction(uint64(0), 100000, key)
batches[i] = tx
}
for _, tx := range batches {
pool.addRemotesSync([]*types.Transaction{tx})
}
b.ResetTimer()
// benchmark truncating the pending
for range b.N {
pool.truncatePending()
}
}
```
This PR fixes a deadlock situation is deleteTailEpoch that might arise
when
range delete is running in iterator based fallback mode (either using
leveldb
database or the hashdb state storage scheme).
In this case a stopCb callback is called periodically that does check
events,
including matcher sync requests, in which case it tries to acquire
indexLock
for read access, while deleteTailEpoch already held it for write access.
This pull request removes the indexLock acquiring in
`FilterMapsMatcherBackend.synced`
as this function is only called in the indexLoop.
Fixes https://github.com/ethereum/go-ethereum/issues/31700
This PR adds the `AuthorizationList` field to the `CallMsg` interface to support `eth_call`
and `eth_estimateGas` of set-code transactions.
---------
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
This PR ensures that caching a slice or a slice of slices will never
affect the original version by always cloning a slice fetched from cache
if it is not used in a guaranteed read only way.
This PR changes the chain view update mechanism of the log filter.
Previously the head updates were all wired through the indexer, even in
unindexed mode. This was both a bit weird and also unsafe as the
indexer's chain view was updates asynchronously with some delay, making
some log related tests flaky. Also, the reorg safety of the indexed
search was integrated with unindexed search in a weird way, relying on
`syncRange.ValidBlocks` in the unindexed case too, with a special
condition added to only consider the head of the valid range but not the
tail in the unindexed case.
In this PR the current chain view is directly accessible through the
filter backend and unindexed search is also chain view based, making it
inherently safe. The matcher sync mechanism is now only used for indexed
search as originally intended, removing a few ugly special conditions.
The PR is currently based on top of
https://github.com/ethereum/go-ethereum/pull/31642
Together they fix https://github.com/ethereum/go-ethereum/issues/31518
and replace https://github.com/ethereum/go-ethereum/pull/31542
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
The API `eth_feeHistory` returns
`{"jsonrpc":"2.0","id":1,"error":{"code":-32603,"message":"json:
unsupported value: NaN"}}`, when we query `eth_feeHistory` with a old
block that without a blob, or when the field
`config.blobSchedule.cancun.max` in genesis.config is 0 (that happens
for some projects fork geth but they don't have blob).
So here we specially handle the case when maxBlobGas == 0 to prevent
this issue from happening.
This PR makes `filtermaps.ChainView` thread safe because it is used
concurrently both by the indexer and multiple matcher threads. Even
though it represents an immutable view of the chain, adding a mutex lock
to the `blockHash` function is necessary because it does so by extending
its list of non-canonical hashes if the underlying blockchain is
changed.
The unsafe concurrency did cause a panic once after running the unit
tests for several hours and it could also happen during live operation.
This PR makes the conditions for using a map rendering snapshot stricter
so that whenever a reorg happens, only a snapshot of a common ancestor
block can be used. The issue fixed in
https://github.com/ethereum/go-ethereum/pull/31642 originated from using
a snapshot that wasn't a common ancestor. For example in the following
reorg scenario: `A->B`, then `A->B2`, then `A->B2->C2`, then `A->B->C`
the last reorg triggered a render from snapshot `B` saved earlier. Now
this is possible under certain conditions but extra care is needed, for
example if block `B` crosses a map boundary then it should not be
allowed. With the latest fix the checks are sufficient but I realized I
would just feel safer if we disallowed this rare and risky scenario
altogether and just render from snapshot `A` after the last reorg in the
example above. The performance difference if a few milliseconds and it
occurs rarely (about once a day on Holesky, probably much more rare on
Mainnet).
Note that this PR only makes the snapshot conditions stricter and
`TestIndexerRandomRange` does check that snapshots are still used
whenever it's obviously possible (adding blocks after the current head
without a reorg) so this change can be considered safe. Also I am
running the unit tests and the fuzzer and everything seems to be fine.
This pull request improves error handling for local transaction submissions.
Specifically, if a transaction fails with a temporary error but might be
accepted later, the error will not be returned to the user; instead, the
transaction will be tracked locally for resubmission.
However, if the transaction fails with a permanent error (e.g., invalid
transaction or insufficient balance), the error will be propagated to the user.
These errors returned in the legacyPool are regarded as temporary failure:
- `ErrOutOfOrderTxFromDelegated`
- `txpool.ErrInflightTxLimitReached`
- `ErrAuthorityReserved`
- `txpool.ErrUnderpriced`
- `ErrTxPoolOverflow`
- `ErrFutureReplacePending`
Notably, InsufficientBalance is also treated as a permanent error, as
it’s highly unlikely that users will transfer funds into the sender account
after submitting the transaction. Otherwise, users may be confused—seeing
their transaction submitted but unaware that the sender lacks sufficient funds—and
continue waiting for it to be included.
---------
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>
closes#31310
This has been requested a few times in the past and I think it is a nice
quality-of-life improvement for users. At a predetermined interval,
there will now be a "Fork ready" log when a future fork is scheduled,
but not yet active.
It can only possibly print after block import, which kinda avoids the
scenario where the client isn't progressing or is syncing and the user
thinks it's "ready" because it sees a ready log.
New output:
```console
INFO [03-08|21:32:57.472] Imported new potential chain segment number=7 hash=aa24ee..f09e62 blocks=1 txs=0 mgas=0.000 elapsed="874.916µs" mgasps=0.000 snapdiffs=973.00B triediffs=7.05KiB triedirty=0.00B
INFO [03-08|21:32:57.473] Ready for fork activation fork=Prague date="18 Mar 25 19:29 CET" remaining=237h57m0s timestamp=1,742,322,597
INFO [03-08|21:32:57.475] Chain head was updated number=7 hash=aa24ee..f09e62 root=19b0de..8d32f2 elapsed="129.125µs"
```
Easiest way to verify this behavior is to apply this patch and run `geth
--dev --dev.period=12`
```patch
diff --git a/params/config.go b/params/config.go
index 9c7719d901..030c4f80e7 100644
--- a/params/config.go
+++ b/params/config.go
@@ -174,7 +174,7 @@ var (
ShanghaiTime: newUint64(0),
CancunTime: newUint64(0),
TerminalTotalDifficulty: big.NewInt(0),
- PragueTime: newUint64(0),
+ PragueTime: newUint64(uint64(time.Now().Add(time.Hour * 300).Unix())),
BlobScheduleConfig: &BlobScheduleConfig{
Cancun: DefaultCancunBlobConfig,
Prague: DefaultPragueBlobConfig,
```
BroadcastTransactions needs the Sender address to route message flows
from the same Sender address consistently to the same random subset of
peers. It however spent considerable time calculating the Sender
addresses, even if the Sender address was already calculated and cached
in other parts of the code.
Since we only need the mapping, we can use any signer, and the one that
had already been used is a better choice because of cache reuse.
This is an attempt at fixing #31601. I think what happens is the startup
logic will try to get the full block body (it's `bc.loadLastState`) and
fail because genesis block has been pruned from the freezer. This will
cause it to keep repeating the reset logic, causing a deadlock.
This can happen when due to an unsuccessful sync we don't have the state
for the head (or any other state) fully, and try to redo the snap sync.
---------
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This fixes an issue where running geth with `--history.chain postmerge`
would not work on an empty database.
```
ERROR[04-16|23:11:12.913] Chain history database is pruned to unknown block tail=0
Fatal: Failed to register the Ethereum service: unexpected database tail
```
This PR fixes a bug in the map renderer that sometimes used an obsolete
block log value pointer to initialize the iterator for rendering from a
snapshot. This bug was triggered by chain reorgs and sometimes caused
indexing errors and invalid search results. A few other conditions are
also made safer that were not reported to cause issues yet but could
potentially be unsafe in some corner cases. A new unit test is also
added that reproduced the bug but passes with the new fixes.
Fixes https://github.com/ethereum/go-ethereum/issues/31593
Might also fix https://github.com/ethereum/go-ethereum/issues/31589
though this issue has not been reproduced yet, but it appears to be
related to a log index database corruption around a specific block,
similarly to the other issue.
Note that running this branch resets and regenerates the log index
database. For this purpose a `Version` field has been added to
`rawdb.FilterMapsRange` which will also make this easier in the future
if a breaking database change is needed or the existing one is
considered potentially broken due to a bug, like in this case.
Our metrics related to dial errors were off. The original error was not
wrapped, so the caller function had no chance of picking it up.
Therefore the most common error, which is "TooManyPeers", was not
correctly counted.
The metrics were originally introduced in
https://github.com/ethereum/go-ethereum/pull/27621
I was thinking of various possible solutions.
- the one proposed here wraps both the new error and the origial error.
It is not a pattern we use in other parts of the code, but works. This
is maybe the smallest possible change.
- as an alternate, I could write a proper `errProtoHandshakeError` with
it's own wrapped error
- finally, I'm not even sure we need `errProtoHandshakeError`, maybe we
could just pass up the original error.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Thank you, @holiman, for being an integral part of the Go-Ethereum
and for your invaluable contributions over the years.
This will always be your home and you're welcome back anytime!
I added the history mode configuration in eth/ethconfig initially, since
it seemed like the logical place. But it turns out we need access to the
intended pruning setting at a deeper level, and it actually needs to be
integrated with the blockchain startup procedure.
With this change applied, if a node previously had its history pruned,
and is subsequently restarted **without** the `--history.chain
postmerge` flag, the `BlockChain` initialization code will now verify
the freezer tail against the known pruning point of the predefined
network and will restore pruning status. Note that this logic is quite
restrictive, we allow non-zero tail only for known networks, and only
for the specific pruning point that is defined.
As of now, Geth disconnects peers only on protocol error or timeout,
meaning once connection slots are filled, the peerset is largely fixed.
As mentioned in https://github.com/ethereum/go-ethereum/issues/31321,
Geth should occasionally disconnect peers to ensure some churn.
What/when to disconnect could depend on:
- the state of geth (e.g. sync or not)
- current number of peers
- peer level metrics
This PR adds a very slow churn using a random drop.
---------
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Our previous success metrics gave success even if a peer disconnected
right after connection. These metrics only count peers that stayed
connected for at least 1 min. The 1 min limit is an arbitrary choice. We do
not use this for decision logic, only statistics.
