Skip to content

cmd: convert deposit sign withdrawal addresses to creds - #4804

Merged
OisinKyne merged 3 commits into
mainfrom
oisin/deposit-sign-withdrawal-creds
Nov 3, 2026
Merged

OisinKyne merged 3 commits into
mainfrom
oisin/deposit-sign-withdrawal-creds

Conversation

@OisinKyne

Copy link
Copy Markdown
Contributor

charon deposit sign hex-decoded --withdrawal-addresses straight into DepositMessage.WithdrawalCredentials. A 20 byte address, as the alter withdrawal address docs show, was silently zero-padded by SSZ hashing, so operators signed, and the Obol API stored, deposits with malformed credentials (<address><12 zero bytes>).

  • Adds deposit.WithdrawalCredsFromAddrOrCreds: a 20 byte address becomes 0x01 credentials (0x02 for compounding clusters). Mixed case addresses must match their EIP-55 checksum; all lower or all upper case is accepted. Full 32 byte credentials are still accepted if the prefix matches the cluster's compounding setting and bytes 1-11 are zero. Anything else is rejected.
  • deposit sign uses it and rejects the zero address on mainnet/gnosis, as create dkg does.
  • deposit fetch rejects API credentials that are not 32 bytes, covering bad records already stored.
  • obolapimock rejects non 32 byte credentials, mirroring the API fix.
  • Tests now use real addresses (the 32 byte input in the old test hid the bug) and the sign/fetch test checks the credentials written to the deposit data file.

Companion changes: obol-api oisin/validate-withdrawal-creds, obol-sdk oisin/validate-withdrawal-creds-length, obol-docs oisin/deposit-sign-v1.11-credentials and oisin/deposit-sign-withdrawal-addresses.

category: bug
ticket: none

charon deposit sign hex-decoded --withdrawal-addresses straight into
DepositMessage.WithdrawalCredentials. A 20 byte address (as documented)
was zero-padded by SSZ hashing, so operators signed and the API stored
malformed deposits.

Addresses are now converted to 0x01 (or 0x02 for compounding clusters)
withdrawal credentials. Mixed case addresses must match their EIP-55
checksum. Full 32 byte credentials are still accepted if the prefix
matches the cluster's compounding setting. deposit fetch rejects
credentials from the API that are not 32 bytes.
@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.89189% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.69%. Comparing base (4e4d996) to head (f137bd8).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
eth2util/deposit/deposit.go 92.00% 2 Missing ⚠️
cmd/depositsign.go 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4804      +/-   ##
==========================================
+ Coverage   67.61%   67.69%   +0.08%     
==========================================
  Files         254      255       +1     
  Lines       33609    33698      +89     
==========================================
+ Hits        22725    22813      +88     
- Misses      10883    10884       +1     
  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.

aly-obol
aly-obol previously approved these changes Oct 23, 2026
@OisinKyne OisinKyne changed the title cmd: convert deposit sign withdrawal addresses to credentials cmd: convert deposit sign withdrawal addresses to creds Oct 23, 2026
@sonarqubecloud

Copy link
Copy Markdown

@OisinKyne
OisinKyne merged commit e7d8587 into main Nov 3, 2026
11 checks passed
@OisinKyne
OisinKyne deleted the oisin/deposit-sign-withdrawal-creds branch November 3, 2026 16:22
obol-bulldozer Bot pushed a commit that referenced this pull request Nov 4, 2026
Validates fee recipient and withdrawal addresses the same way in `create cluster`, `create dkg` and `edit add-validators`, and stores them in EIP-55 checksummed form. Before this, `create cluster` without a definition file skipped the withdrawal address check entirely. A zero address on mainnet or gnosis is an error in `create dkg` and a warning elsewhere (where it's legitimate, e.g. splitting the keys of validators that are already active).

The "all-lowercase or all-uppercase, otherwise it must match EIP-55" rule now lives in one place, `eth2util.ValidateChecksumAddress`. It's used by these commands, by `feerecipient sign` (which wrongly rejected all-uppercase addresses) and by `deposit.WithdrawalCredsFromAddrOrCreds` (#4804).

category: bug
ticket: none
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.

3 participants