Closed
Conversation
…art 1) (#31634) This is the first part of #31532 It maintains a series of conversion maker which are to be updated by the conversion code (in a follow-up PR, this is a breakdown of a larger PR to make things easier to review). They can be used in this way: - During the conversion, by storing the conversion markers when the block has been processed. This is meant to be written in a function that isn't currently present, hence [this TODO](https://github.com/ethereum/go-ethereum/pull/31634/files#diff-89272f61e115723833d498a0acbe59fa2286e3dc7276a676a7f7816f21e248b7R384). Part of ethereum/go-ethereum#31583 --------- Signed-off-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Co-authored-by: Gary Rong <garyrong0905@gmail.com>
- 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>
This PR makes 2 changes to how [EIP-7825](ethereum/go-ethereum#31824) behaves. When `eth_estimateGas` or `eth_createAccessList` is called without any gas limit in the payload, geth will choose the block's gas limit or the `RPCGasCap`, which can be larger than the `maxTxGas`. When this happens for `estimateGas`, the gas estimation just errors out and ends, when it should continue doing binary search to find the lowest possible gas limit. This PR will: - Add a check to see if `hi` is larger than `maxTxGas` and cap it to `maxTxGas` if it's larger. And add a special case handling for gas estimation execute when it errs with `ErrGasLimitTooHigh` --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Co-authored-by: zsfelfoldi <zsfelfoldi@gmail.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
The separation serves no purpose atm, and the circular dependency that EVM and EVMInterpreter had was begging for them to be merged.
Add missing it.Error() check after iteration in Database.DeleteRange to avoid silently ignoring iterator errors before writing the batch. Aligns behavior with batch.DeleteRange, which already validates iterator errors. No other functional changes; existing tests pass (TestLevelDB).
The previous comment stated that every 3rd block has a tx and every 5th has an uncle. The implementation actually adds one transaction to every second block and does not add uncles. Updated the comment to reflect the real behavior to avoid confusion when reading tests.
Co-authored-by: Felix Lange <fjl@twurst.com>
The GetHeader function was incorrectly returning an error when encountering nil peers in the peers list, which contradicted the comment "keep retrying if none are yet available". Changed the logic to skip nil peers with 'continue' instead of returning an error, allowing the function to properly iterate through all available peers and attempt to retrieve the target header from each valid peer. This ensures the function behaves as intended - trying all available peers before giving up, rather than failing on the first nil peer encountered.
These changes made in the PR should be highlighted here The trie tracer is split into two distinct structs: opTracer and prevalueTracer. The former is specific to MPT, while the latter is generic and applicable to all trie implementations. The original values of dirty nodes are tracked in a NodeSet. This serves as the foundation for both full archive node implementations and the state live tracer.
## Description Correct symmetric tolerance in gas limit validation: Replace ambiguous "+-=" with standard "+/-" in the error message. Logic rejects when |header − parent| ≥ limit, so allowed range is |Δ| ≤ limit − 1. No logic or functionality has been modified.
fix inconsistent function name in comment Signed-off-by: youzichuan <youzichuan6@outlook.com>
### Summary Fixes long-standing ETA calculation errors in progress indicators that have been present since February 2021. The current implementation produces increasingly inaccurate estimates due to integer division precision loss. ### Problem https://github.com/ethereum/go-ethereum/blob/3aeccadd04aee2d18bdb77826f86b1ca000d3b67/triedb/pathdb/history_indexer.go#L541-L553 The ETA calculation has two critical issues: 1. **Integer division precision loss**: `speed` is calculated as `uint64` 2. **Off-by-one**: `speed` uses `+ 1`(2 times) to avoid division by zero, however it makes mistake in the final calculation This results in wildly inaccurate time estimates that don't improve as progress continues. ### Example Current output during state history indexing: ``` lvl=info msg="Indexing state history" processed=16858580 left=41802252 elapsed=18h22m59.848s eta=11h36m42.252s ``` **Expected calculation:** - Speed: 16858580 ÷ 66179848ms = 0.255 blocks/ms - ETA: 41802252 ÷ 0.255 = ~45.6 hours **Current buggy calculation:** - Speed: rounds to 1 block/ms - ETA: 41802252 ÷ 1 = ~11.6 hours ❌ ### Solution - Created centralized `CalculateETA()` function in common package - Replaced all 8 duplicate code copies across the codebase ### Testing Verified accurate ETA calculations during archive node reindexing with significantly improved time estimates.
Filtering for leaf nodes was missing from #32388, which means that even the root done was reported, which made little sense for the bloatnet data processing we want to do.
Implement the binary tree as specified in [eip-7864](https://eips.ethereum.org/EIPS/eip-7864). This will gradually replace verkle trees in the codebase. This is only running the tests and will not be executed in production, but will help me rebase some of my work, so that it doesn't bitrot as much. --------- Signed-off-by: Guillaume Ballet Co-authored-by: Parithosh Jayanthi <parithosh.jayanthi@ethereum.org> Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
~Will probably be mostly supplanted by #32224, but this should do for now for devnet 3.~ Seems like #32224 is going to take some more time, so I have completed the implementation of eth_config here. It is quite a bit simpler to implement now that the config hashing was removed. --------- Co-authored-by: MariusVanDerWijden <m.vanderwijden@live.de> Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
geoknee
added
the
H-l1-fusaka-defense
geoknee
force-pushed
the
gk/upstream-merge/v1.16.3
branch
from
657c5f3 to
947f7a0
Compare
This was referenced Sep 19, 2025
… for non OPStack chains (in traceContext.toBlockContext)
joshklop
reviewed
Sep 19, 2025
protolambda
previously approved these changes
Sep 22, 2025
Contributor
There was a problem hiding this comment.
Reviewed the merge-conflicts, as well as wider check of the full upstream 1.16.3 diff, and it all looks good to me.
There is one chain-config issue (not a merge-conflict, really sneaky issue) that seems to have made it into the upstream geth release, and only fixed in upstream later. We need to be aware of that for BPO compatibility later, but it's ok for now.
Also, before continuing, please add a commit that updates the upstream reference hash in fork.yaml to the commit of 1.16.3
Edit: one more thing; commit cleanup + messages. Withdrawing approval for now. But PR looks good otherwise! Thanks!
Comment on lines
+1221
to
+1233
| // BlobConfig returns the blob config associated with the provided fork. | ||
| func (c *ChainConfig) BlobConfig(fork forks.Fork) *BlobConfig { | ||
| switch fork { | ||
| case forks.Osaka: | ||
| return DefaultOsakaBlobConfig | ||
| case forks.Prague: | ||
| return DefaultPragueBlobConfig | ||
| case forks.Cancun: | ||
| return DefaultCancunBlobConfig | ||
| default: | ||
| return nil | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
While reviewing this I noticed it's not using the receiver c at all, and it looks like an upstream config bug.
It was fixed here, to use the actual config contents: ethereum/go-ethereum#32579
But that fix wasn't included in the 1.16.3 release, so it will use the default configs instead here (!!!).
We should be aware of this difference; usage of the blob config won't be accurate on custom BPO settings.
Contributor
Author
There was a problem hiding this comment.
Great catch. I'll file an issue in this repo for that and reference in a code comment here. We expect to do another upstream merge very shortly which will rectify this problem.
protolambda
dismissed
their stale review
Since we cannot do a squash-merge, please take a moment to combine the final fix commits and clean up the commit messages
Contributor
Author
|
Rewritten here #686 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
Do not squash merge.
Towards #682
ethereum-optimism/optimism#17547