all: Merge upstream pre-v1.15.0 master branch for 7702 SetCode transaction type
#474
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
params: begin v1.14.12 release cycle
This pull request removes the `fsync` of index files in freezer.ModifyAncients function for performance gain. Originally, fsync is added after each freezer write operation to ensure the written data is truly transferred into disk. Unfortunately, it turns out `fsync` can be relatively slow, especially on macOS (see ethereum/go-ethereum#28754 for more information). In this pull request, fsync for index file is removed as it turns out index file can be recovered even after a unclean shutdown. But fsync for data file is still kept, as we have no meaningful way to validate the data correctness after unclean shutdown. --- **But why do we need the `fsync` in the first place?** As it's necessary for freezer to survive/recover after the machine crash (e.g. power failure). In linux, whenever the file write is performed, the file metadata update and data update are not necessarily performed at the same time. Typically, the metadata will be flushed/journalled ahead of the file data. Therefore, we make the pessimistic assumption that the file is first extended with invalid "garbage" data (normally zero bytes) and that afterwards the correct data replaces the garbage. We have observed that the index file of the freezer often contain garbage entry with zero value (filenumber = 0, offset = 0) after a machine power failure. It proves that the index file is extended without the data being flushed. And this corruption can destroy the whole freezer data eventually. Performing fsync after each write operation can reduce the time window for data to be transferred to the disk and ensure the correctness of the data in the disk to the greatest extent. --- **How can we maintain this guarantee without relying on fsync?** Because the items in the index file are strictly in order, we can leverage this characteristic to detect the corruption and truncate them when freezer is opened. Specifically these validation rules are performed for each index file: For two consecutive index items: - If their file numbers are the same, then the offset of the latter one MUST not be less than that of the former. - If the file number of the latter one is equal to that of the former plus one, then the offset of the latter one MUST not be 0. - If their file numbers are not equal, and the latter's file number is not equal to the former plus 1, the latter one is valid And also, for the first non-head item, it must refer to the earliest data file, or the next file if the earliest file is not sufficient to place the first item(very special case, only theoretical possible in tests) With these validation rules, we can detect the invalid item in index file with greatest possibility. --- But unfortunately, these scenarios are not covered and could still lead to a freezer corruption if it occurs: **All items in index file are in zero value** It's impossible to distinguish if they are truly zero (e.g. all the data entries maintained in freezer are zero size) or just the garbage left by OS. In this case, these index items will be kept by truncating the entire data file, namely the freezer is corrupted. However, we can consider that the probability of this situation occurring is quite low, and even if it occurs, the freezer can be considered to be close to an empty state. Rerun the state sync should be acceptable. **Index file is integral while relative data file is corrupted** It might be possible the data file is corrupted whose file size is extended correctly with garbage filled (e.g. zero bytes). In this case, it's impossible to detect the corruption by index validation. We can either choose to `fsync` the data file, or blindly believe that if index file is integral then the data file could be integral with very high chance. In this pull request, the first option is taken.
Remove console extensions for already deleted API namespaces (les, vflux and ethash).
The bulk of this PR is authored by @lightclient , in the original EOF-work. More recently, the code has been picked up and reworked for the new EOF specification, by @MariusVanDerWijden , in ethereum/go-ethereum#29518, and also @shemnon has contributed with fixes. This PR is an attempt to start eating the elephant one small bite at a time, by selecting only the eof-validation as a standalone piece which can be merged without interfering too much in the core stuff. In this PR: - [x] Validation of eof containers, lifted from #29518, along with test-vectors from consensus-tests and fuzzing, to ensure that the move did not lose any functionality. - [x] Definition of eof opcodes, which is a prerequisite for validation - [x] Addition of `undefined` to a jumptable entry item. I'm not super-happy with this, but for the moment it seems the least invasive way to do it. A better way might be to go back and allowing nil-items or nil execute-functions to denote "undefined". - [x] benchmarks of eof validation speed --------- Co-authored-by: lightclient <lightclient@protonmail.com> Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de> Co-authored-by: Danno Ferrin <danno.ferrin@shemnon.com>
minimizes the time when the lock is held
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>
…(#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 <lightclient@protonmail.com>
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 <wangpeculiar@gmail.com> Co-authored-by: Gary Rong <garyrong0905@gmail.com>
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 <m.vanderwijden@live.de>
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 ](ethereum/go-ethereum#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 <peterke@gmail.com>
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`.
Co-authored-by: Felix Lange <fjl@twurst.com>
sebastianst
commented
Jan 23, 2025
Comment on lines
+72
to
75
| case config.PragueTime != nil: | ||
| signer = NewPragueSigner(config.ChainID) | ||
| case config.CancunTime != nil && !config.IsOptimism(): | ||
| signer = NewCancunSigner(config.ChainID) |
There was a problem hiding this comment.
Same here, no Optimism exception for Prague like for Cancun.
sebastianst
commented
Jan 23, 2025
Comment on lines
98
to
+101
| func LatestSignerForChainID(chainID *big.Int) Signer { | ||
| var signer Signer | ||
| if chainID != nil { | ||
| signer = NewCancunSigner(chainID) | ||
| signer = NewPragueSigner(chainID) |
There was a problem hiding this comment.
Note that we never had an exception here for Optimism chains.
|
Tagged |
…ger Flex (#31004) The latest firmware for Ledger Nano S Plus now returns `0x5000` for it's product ID, which doesn't match any of the product IDs enumerated in `hub.go`. This PR removes the assumption about the interfaces exposed, and simply checks the upper byte for a match. Also adds support for the `0x0007` / `0x7000` product ID (Ledger Flex).
Won't this make future git merges only more difficult? Since the commits will differ in commit-hash again and again, every time we try to consolidate with the |
Reference: - Remove MUL precompiles: ethereum/EIPs#8945 - Pricing change for pairing operation: ethereum/EIPs#9098 - Pricing change for add, mapping and mul operations: ethereum/EIPs#9097 - Pricing change for MSM operations: ethereum/EIPs#9116 --------- Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
|
The interop proof depends on EIP 2935 meaning that a rebase with that EIP in it would be very helpful for unblocking |
This implements a basic mechanism to query the node's external IP using a STUN server. There is a built-in list of public STUN servers for convenience. The new detection mechanism must be selected explicitly using `--nat=stun` and is not enabled by default in Geth. Fixes #30881 --------- Co-authored-by: Felix Lange <fjl@twurst.com>
Hi I fixed 2 minor spelling issues. --------- Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
@tynes sure, I'll also get the latest changes from the upstream |
sebastianst
force-pushed
the
seb/upstream-pre-v1.14.13-master
branch
from
97d4f3e to
9bfaa89
Compare
sebastianst
force-pushed
the
seb/upstream-pre-v1.14.13-master
branch
from
9bfaa89 to
68e6357
Compare
|
Tagged |
sebastianst
changed the title
SetCode transaction typeSetCode transaction type
It's fixed now, git history now is a merge commit of latest upstream master branch. |
|
One unfortunate consequence of merging in |
This was referenced Jan 28, 2025
|
Before merging anything: we still need to update the upstream-geth commit reference in the |
|
Closing, we're going forward with the single-merge-commit PR #480 |
sebastianst
added a commit
that referenced
this pull request
Jan 31, 2025
all: Merge upstream pre-v1.15.0, based on #474, plus review fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We need the new
SetCodetransaction type to prepare for L1 Pectra compatibility.The upcoming upstream release v1.14.13 is imminent, but not out yet. So this PR cherry-picks all recent commits onmaster.Update: Release v1.14.13 got cancelled, next up is v1.15.0, which is even further out...
I first tried agit merge upstream/masterbut this presented me with a flurry of merge conflicts that resulted not only from the OP-Stack diff, but apparently git recognizing a wrong base between theoptimismbranch, which was based on the latestv1.14.12upstream release tag, andupstream/master. Git didn't recognize changes inupstream/masteras being newer and apply them correctly on top ofoptimism. So instead, I did agit cherry-pick c64cf28..upstream/masterwhich cherry-picked each commit individually from a common "base" commit that sealed thev1.14.12release. This also caused a flurry of merge conflicts to be resolved, but at least they were all coming from conflicts with the actual OP-Stack diff.Update: resolved this by going back to the branch base at
optimism, then initiating the merge, then resolving all conflicts at once by checking out the work already done:There are a few todos for myself and things for reviewers to be aware of:
ApplyTransactionExtended— the API changed ofApplyTransactionCachingDB.ContractCodeagain to support snap sync diff ineth/protocols/snap/handler.go, which used to call legacyContractCode.core/state: introduce code reader interface (#30816)MakeSignerandLatestSignerlogic incore/types/transaction_signing.go.golanci.yml'SA1019:' # temporary, until fully updated to Go 1.21since we're on go 1.22.SetupGenesisBlockWithOverride, it was refactored a lot. There's an open todo to handle a transitioned Bedrock network.toolchainline ingo.mod(go.mod: remove toolchain line ethereum/go-ethereum#31057), we should check if we want this too. I remember that there were some issues with working in a go workspace if the toolchain was not installed.go mod tidywould re-add it. But maybe that was only related to thego.workin the superchain-registry when it pulled in op-geth as a dependency.Genesis.Commitbecause it messed up theTestEvmRunwhich compares exact tooling output, and the warning message was added by us and contains a timestamp, so exact comparison is not possible. In fact, upstream just commented out two other stderr comparisons in that test because of log lines and they seemed to be lazy and just not fix the test for these cases...Metadata
Towards ethereum-optimism/optimism#13627