Repository navigation
fix(ethereum): decode canonical Glamsterdam block hashes - #4381
Draft
mswilkison wants to merge 4 commits into
Draft
mswilkison wants to merge 4 commits into
mswilkison wants to merge 4 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
This branch has not been deployed
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
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.
After Glamsterdam, geth v1.17.3 decodes the block access list hash under the obsolete JSON name
balHash, while Ethereum RPC responses useblockAccessListHash.GetBlockHashByNumberthen hashes an incomplete header. Wallet coordination consumes that hash, so corrected and old clients can select different leaders and heartbeat choices for the same wallet and window.Update geth to v1.17.10, including its required transitive module versions and checksums. Preserve every existing explicit replace, vendored cryptographic pin, and Go toolchain directive. Add a real public Sepolia response and an in-process RPC regression that exercises the production
baseChain.GetBlockHashByNumberpath; it fails on v1.17.3 and passes on v1.17.10. Seedocs/glamsterdam-client-compatibility.mdfor the release gate and fixture provenance.Coordinated client release required: every operator must run the corrected client before the relevant mainnet fork activation. Sepolia already carries these fields and requires a coordinated whole-fleet transition or a compatible activation gate; this patch must not be deployed there as an ungated rolling upgrade. No application activation gate or cutover block is implemented or chosen here. Release verification must compare canonical hashes and coordination outputs across the actual RPC providers operators use, beyond the saved fixture, and complete an actual heartbeat and deposit-sweep signing and live execution rehearsal on the coordinated corrected fleet. This patch does not claim that rehearsal is complete. No live-node or governance action is included.
Activation-boundary transaction gate: generated
ForceMiningreplacements still preserve the original nonzeroGasLimit; the geth bump does not fix that behavior. The runbook requires a bounded, coordinated drain of pending/pre-fork-estimated calls, post-fork estimates for post-fork inclusion, and receipt/confirmed-and-pending-nonce/current-business-state reconciliation before any fresh estimate and nonce-appropriate retry. Work that may still be pending or may already have succeeded must not be blindly rebroadcast. This is an interim release prerequisite, not an implemented drain mechanism or a chosen fork date.Validation:
go test -mod=readonly ./pkg/chain/ethereum -count=1— passed.go build -mod=readonly ./...— passed; no application API changes required.go vetat the module root — passed.git diff --check— passed.go vet -tags=integration ./...reports the existing lock-value copy atpkg/tecdsa/signing/protocol.go:758; that source is unchanged, and the same finding was documented in the earlier public geth bump, Merge keep-common pkg/ into pkg/keepcommon/, bump go-ethereum to v1.17.3 #4327. No unrelated source change is included.TestTssRoundTwo_SymmetricKeyMissingtimed out while waiting for a round-one outgoing message. The exact test passed in isolation (14.828s); every other package passed. #4327 documented the same existing timeout class. CI remains the full-suite gate.The image-build checksum failure is fixed. Actual WalletRegistry ABI generation with
-mod=readonly, Linux/amd64 generator dependency resolution and the complete CI image build pass at77cb42f.The CI scan reports 14 G115 conversions in unchanged vendored btcec and go-electrum files. Their source and scan configuration match the development baseline. That baseline passed with an older scanner revision; it has not been rerun with the current scanner, so precise causality is unproven. This scan and its dependent gate are the only failing current checks. The findings remain visible as a release blocker. No detector, scan scope, vendored source or CI rule has been weakened.