Skip to content

Conversation

@m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Apr 16, 2025

Work towards Pectra upgrade 🚀

fjl and others added 30 commits March 31, 2025 18:29
…m#31361)

Implements ethereum#31275

---------

Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
It introduces a new variable to store the external port returned by the
addAnyPortMapping function and ensures that the correct external port is
returned even in case of an error.

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
This PR adds metrics related to map rendering and pattern matching to
the `core/filtermaps` package.
…ent` (ethereum#31501)

found the failed testcase here
https://ci.appveyor.com/project/ethereum/go-ethereum/builds/51767091/job/rbjke432c05pufja

add a timeout to wait the tx to be mined.

---------

Signed-off-by: jsvisa <delweng@gmail.com>
Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
This PR refactors the `nodeSet` structure in the path database to use
separate maps for account and storage trie nodes, resulting in
performance improvements. The change maintains the same API while
optimizing the internal data structure.
This is an alternative to ethereum#31309

With eth/68, transaction announcement must have transaction type and
size. So in announceTransactions, we need to query the transaction from
transaction pool with its hash. This creates overhead in case of blob
transaction which needs to load data from billy and RLP decode. This
commit creates a lightweight lookup from transaction hash to transaction
size and a function GetMetadata to query transaction type and
transaction size given the transaction hash.

---------

Co-authored-by: Gary Rong <garyrong0905@gmail.com>
…thereum#31543)

When resending the WHOAREYOU packet, a new nonce and random IV should not
be generated. The sent packet needs to match the previously-sent one exactly
in order to make the handshake retry work.

---------

Co-authored-by: Felix Lange <fjl@twurst.com>
…um#31075)

This is for the implementation of Portal Network in the Shisui client.
Their handler needs access to the node object in order to send further
calls to the requesting node. This is a breaking API change but it
should be fine, since there are basically no known users of TALKREQ
outside of Portal network.

---------

Signed-off-by: thinkAfCod <q315xia@163.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Fixes a few typos in readme.
Co-authored-by: lightclient <lightclient@protonmail.com>
This pull request introduces new sync logic for pruning mode. The downloader will now skip
insertion of block bodies and receipts before the configured history cutoff point.

Originally, in snap sync, the header chain and other components (bodies and receipts) were
inserted separately. However, in Proof-of-Stake, this separation is unnecessary since the
sync target is already verified by the CL.

To simplify the process, this pull request modifies `InsertReceiptChain` to insert headers
along with block bodies and receipts together. Besides, `InsertReceiptChain` doesn't have
the notion of reorg, as the common ancestor is always be found before the sync and extra
side chain is truncated at the beginning if they fall in the ancient store. The stale
canonical chain flags will always be rewritten by the new chain. Explicit reorg logic is
no longer required in `InsertReceiptChain`.
These were caused by crossed merges of recent PRs ethereum#31414 and ethereum#31361
This adds the test description text to the output, instead of keeping it
as a Go comment. Logs are visible in the hive UI where these tests run,
while Go comments are not.
)

This PR adds a new `--beacon.checkpoint.file` config flag to geth and
blsync which specifies a checkpoint import/export file. If a file with
an existing checkpoint is specified, it is used for initialization
instead of the hardcoded one (except when `--beacon.checkpoint` is also
specified simultaneously). Whenever the client encounters a new valid
finality update with a suitable finalized beacon block root at an epoch
boundary, it saves the block root in hex format to the checkpoint file.
Here we are modifying the port mapping logic so that existing port
mappings will only be removed when they were previously created by geth.

The AddAnyPortMapping functionality has been adapted to work consistently
between the IGDv1 and IGDv2 backends.
Co-authored-by: lightclient <lightclient@protonmail.com>

Add GetHeaderByNumber and GetReceiptsByNumber to allow more efficient API request filling from Era files.
This PR improves error handling in the remotedb package by fixing two
issues:

1. In the `Has` method, we now properly propagate errors instead of
silently returning false. This makes the behavior more predictable and
helps clients better understand when there are connection issues.

2. In the `New` constructor, we add a nil check for the client parameter
to prevent potential panics. This follows Go best practices for
constructor functions.

These changes make the code more robust and follow Go's error handling
idioms without requiring any changes to other parts of the codebase.

Changes:
- Modified `Has` method to return errors instead of silently returning
false
- Added nil check in `New` constructor
- Fixed field name in constructor to match struct definition
This pull request introduces two constraints in the blobPool:

(a) If the sender has a pending authorization or delegation, only one
in-flight
executable transaction can be cached.
(b) If the authority address in a SetCode transaction is already
reserved by
the blobPool, the transaction will be rejected.

These constraints mitigate an attack where an attacker spams the pool
with
numerous blob transactions, evicts other transactions, and then cancels
all
pending blob transactions by draining the sender’s funds if they have a
delegation.

Note, because there is no exclusive lock held between different subpools
when processing transactions, it's totally possible the SetCode
transaction
and blob transactions with conflict sender and authorities are accepted
simultaneously. I think it's acceptable as it's very hard to be
exploited.

---------

Co-authored-by: lightclient <lightclient@protonmail.com>
During my benchmarks on Holesky, around 10% of all CPU time was spent in
PUSH2
```
ROUTINE ======================== github.com/ethereum/go-ethereum/core/vm.newFrontierInstructionSet.makePush.func1 in github.com/ethereum/go-ethereum/core/vm/instructions.go
    16.38s     20.35s (flat, cum) 10.31% of Total
     740ms      740ms    976:	return func(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) {
         .          .    977:		var (
      40ms       40ms    978:			codeLen = len(scope.Contract.Code)
     970ms      970ms    979:			start   = min(codeLen, int(*pc+1))
     200ms      200ms    980:			end     = min(codeLen, start+pushByteSize)
         .          .    981:		)
     670ms      2.39s    982:		a := new(uint256.Int).SetBytes(scope.Contract.Code[start:end])
         .          .    983:
         .          .    984:		// Missing bytes: pushByteSize - len(pushData)
     410ms      410ms    985:		if missing := pushByteSize - (end - start); missing > 0 {
         .          .    986:			a.Lsh(a, uint(8*missing))
         .          .    987:		}
    12.69s     14.94s    988:		scope.Stack.push2(*a)
      10ms       10ms    989:		*pc += size
     650ms      650ms    990:		return nil, nil
         .          .    991:	}
         .          .    992:}
```

Which is quite crazy. We have a handwritten encoder for PUSH1 already,
this PR adds one for PUSH2.

PUSH2 is the second most used opcode as shown here:
https://gist.github.com/shemnon/fb9b292a103abb02d98d64df6fbd35c8 since
it is used by solidity quite significantly. Its used ~20 times as much
as PUSH20 and PUSH32.

# Benchmarks

```
BenchmarkPush/makePush-14         	94196547	        12.27 ns/op	       0 B/op	       0 allocs/op
BenchmarkPush/push-14             	429976924	         2.829 ns/op	       0 B/op	       0 allocs/op
``` 

---------

Co-authored-by: jwasinger <j-wasinger@hotmail.com>
Make UPnP more robust

- Once a random port was mapped, we try to stick to it even if a UPnP
refresh fails. Previously we were immediately moving back to try the
default port, leading to frequent ENR changes.

- We were deleting port mappings before refresh as a possible
workaround. This created issues in some UPnP servers. The UPnP (and PMP)
specification is explicit about the refresh requirements, and delete is
clearly not needed (see
ethereum#30265 (comment)).
From now on we only delete when closing.

- We were trying to add port mappings only once, and then moved on to
random ports. Now we insist a bit more, so that a simple failed request
won't lead to ENR changes.

Fixes ethereum#31418

---------

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
…ode (ethereum#31567)

The `Sync(..)` function explicitly says not to rely on in production
code, but it is used in `Clear(..)` so should add a similar mention.
…hereum#31373)

This PR proposes a change to the authorizations' validation introduced
in commit cdb66c8. These changes make the expected behavior independent
of the order of admission of authorizations, improving the
predictability of the resulting state and the usability of the system
with it.

The current implementation behavior is dependent on the transaction
submission order: This issue is related to authorities and the sender of
a transaction, and can be reproduced respecting the normal nonce rules.

The issue can be reproduced by the two following cases:
**First case**
- Given an empty pool.
- Submit transaction `{ from: B, auths [ A ] }`: is accepted.
- Submit transaction `{ from: A }`: Is accepted: it becomes the one
in-flight transaction allowed.

**Second case**
- Given an empty pool.
- Submit transaction `{ from: A }`:  is accepted
- Submit transaction `{ from: B, auths [ A ] }`: is rejected since there
is already a queued/pending transaction from A.

The expected behavior is that both sequences of events would lead to the
same sets of accepted and rejected transactions.

**Proposed changes** 
The queued/pending transactions issued from any authority of the
transaction being validated have to be counted, allowing one transaction
from accounts submitting an authorization.

- Notice that the expected behavior was explicitly forbidden in the case
"reject-delegation-from-pending-account", I believe that this behavior
conflicts to the definition of the limitation, and it is removed in this
PR. The expected behavior is tested in
"accept-authorization-from-sender-of-one-inflight-tx".
- Replacement tests have been separated to improve readability of the
acceptance test.
- The test "allow-more-than-one-tx-from-replaced-authority" has been
extended with one extra transaction, since the system would always have
accepted one transaction (but not two).
- The test "accept-one-inflight-tx-of-delegated-account" is extended to
clean-up state, avoiding leaking the delegation used into the other
tests. Additionally, replacement check is removed to be tested in its
own test case.

**Expected behavior** 
The expected behavior of the authorizations' validation shall be as
follows:

![image](https://github.com/user-attachments/assets/dbde7a1f-9679-4691-94eb-c197a0cbb823)
Notice that replacement shall be allowed, and behavior shall remain
coherent with the table, according to the replaced transaction.

---------

Co-authored-by: lightclient <lightclient@protonmail.com>
…ethereum#31468)

Fixes ethereum#31169

The TestTransactionForgotten test was flaky due to real time
dependencies. This PR:

- Replaces real time with mock clock for deterministic timing control
- Adds precise state checks at timeout boundaries
- Verifies underpriced cache states and cleanup
- Improves test reliability by controlling transaction timestamps
- Adds checks for transaction re-enqueueing behavior

The changes ensure consistent test behavior without timing-related
flakiness.

---------

Co-authored-by: Zsolt Felfoldi <zsfelfoldi@gmail.com>
…7439)

This fix allows Trezor to support full 32bit chainId in geth, with the
next version of firmware.

For `chainId > 2147483630` case, Trezor returns signature bit only.
- Trezor returns only signature parity for `chainId > 2147483630` case.
- for `chainId == 2147483630` case, Trezor returns `MAX_UINT32` or `0`,
but it doesn't matter.
  (`2147483630 * 2 + 35` = `4294967295`(`MAX_UINT32`))

chainId | returned signature_v | compatible issue
---------|------------------------|--------------------
0 < chainId <= 255 | chainId * 2 + 35 + v | no issue (firmware `1.6.2`
for Trezor one)
255 < chainId <= 2147483630 | chainId * 2 + 35 + v | ***fixed.***
*firmware `1.6.3`*
chainId > 2147483630 | v | *firmware `1.6.3`*

Please see also: full 32bit chainId support for Trezor
- Trezor one: trezor/trezor-mcu#399 ***merged***
- Trezor model T: trezor/trezor-core#311
***merged***

---------

Signed-off-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
jwasinger and others added 6 commits April 10, 2025 19:49
…rtion to prevent race between txpool head reset and promotion of txs that will be subsequently added (ethereum#31595)

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.
When we instantiate a sub-logger via
`go-ethereum/internal/testlog/logger.With`, we copy the reference to the
`bufHandler` from the parent logger. However, internally,
`go-ethereum/internal/testlog/logger.With` calls `log/slog/Logger.With`
which creates a new handler instance (via
`internal/bufHandler.WithAttrs`).

This PR modifies sub-logger instantiation to use the newly-instantiated
handler, instead of copying the reference from the parent instance. The
type cast from `slog.Handler` to `*bufHandler` in
`internal/testlog/Logger.With` is safe here because a
`internal/testlog/Logger` can only be instantiated with a `*bufHandler`
as the underlying handler type.

Note, that I've also removed a pre-existing method that broke the above
assumption. However, this method is not used in our codebase.

I'm not sure if the assumption holds for forks of geth (e.g. optimism
has modified the testlogger somewhat allowing test loggers to accept
arbitrary handler types), but it seems okay to break API compatibility
given that this is in the `internal` package.

closes ethereum#31533
The submodule was accidentally updated to another commit by f64aa6e.
@m-Peter m-Peter self-assigned this Apr 16, 2025
@m-Peter m-Peter merged commit 47d9f96 into master Apr 18, 2025
0 of 2 checks passed
@m-Peter m-Peter deleted the mpeter/update-go-ethereum-v1.15.8 branch April 18, 2025 06:52
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.