Skip to content

cmd: sign exits without a beacon node - #4820

Open
OisinKyne wants to merge 8 commits into
mainfrom
oisin/exit-sign-offline
Open

OisinKyne wants to merge 8 commits into
mainfrom
oisin/exit-sign-offline

Conversation

@OisinKyne

Copy link
Copy Markdown
Contributor

Since EIP-7044 the voluntary exit domain is fixed to the Capella fork version. exit sign now computes it locally for known networks, using a new GenesisValidatorsRootHex network field taken from the public /eth/v1/beacon/genesis. With both --validator-public-key and --validator-index, --beacon-node-endpoints becomes optional. If a beacon node is configured, the index is cross-checked against the pubkey: a mismatch is an error, and an unknown index or an unreachable BN is a warning. Custom testnets and --all still need a beacon node. exit broadcast is unchanged.

Tests:

  • the local domain is compared against an independent go-eth2-client ForkData computation for every network, and the mainnet value (0x04000000bba4da96…676640) is pinned;
  • an end-to-end exit sign on Hoodi with no beacon node is verified by the mock Obol API using a beacon node's domain.

category: feature
ticket: #3104

OisinKyne and others added 8 commits October 23, 2026 10:35
create cluster without a definition file skipped withdrawal address
validation and the insecure-keys-on-mainnet check, and add-validators
and create dkg fee recipients were never validated, so a zero or
mistyped address on mainnet produced valid deposits.

All create/edit paths now share one rule: 0x + 40 hex, all lower or
all upper case accepted, mixed case must match EIP-55, zero address
forbidden on mainnet/gnosis, stored checksummed.

feerecipient sign refuses gas limit 0 and adopting an in-progress
registration timestamped more than 5 minutes ahead unless --timestamp
confirms it. exit sign with both --validator-public-key and
--validator-index checks the index belongs to the pubkey when the
beacon node knows it.
Zero withdrawal and fee recipient addresses are legitimate when the
generated deposit data is never used, e.g. splitting existing keys of
active validators, or clusters that deposit through their own contracts
(EtherFi's mainnet clusters use the zero withdrawal address).

Keep the existing zero withdrawal address errors in create dkg and
create cluster --definition-file, and only warn on the paths this
branch newly validates: create cluster from flags, add-validators and
fee recipients.
…sses

Points feerecipient sign and deposit.WithdrawalCredsFromAddrOrCreds at the shared
eth2util.ValidateChecksumAddress, so the EIP-55 rule lives in one place. This also
fixes feerecipient sign rejecting all-uppercase addresses.

category: refactor
ticket: none
Since EIP-7044 the voluntary exit domain is fixed to the Capella fork version, so
exit sign now computes it locally for known networks from a new genesis validators
root network field. With both --validator-public-key and --validator-index,
--beacon-node-endpoints is optional; when a beacon node is configured, the index is
still cross-checked against the public key. Custom testnets and --all still need a
beacon node.

category: feature
ticket: #3104
@github-actions github-actions Bot added the branch-invalid PR raised against invalid branch. Not a main or release branch. label Nov 4, 2026
@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2026

Copy link
Copy Markdown

KaloyanTanev
KaloyanTanev previously approved these changes Nov 4, 2026

@KaloyanTanev KaloyanTanev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, apart from unit tests failing

Base automatically changed from oisin/address-validation to main November 4, 2026 17:17
@obol-bulldozer
obol-bulldozer Bot dismissed KaloyanTanev’s stale review November 4, 2026 17:17

The base branch was changed.

@codecov

codecov Bot commented Nov 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.05298% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.76%. Comparing base (e7d8587) to head (e9cc6ea).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/createcluster.go 89.18% 4 Missing ⚠️
cmd/exit_sign.go 87.50% 4 Missing ⚠️
cmd/exit.go 78.57% 3 Missing ⚠️
eth2util/helper_capella.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4820      +/-   ##
==========================================
+ Coverage   67.72%   67.76%   +0.03%     
==========================================
  Files         255      255              
  Lines       33711    33773      +62     
==========================================
+ Hits        22832    22886      +54     
- Misses      10878    10886       +8     
  Partials        1        1              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

Needs a rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch-invalid PR raised against invalid branch. Not a main or release branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants