Skip to content

feat: per-account deposit-address derivation for the ckETH/ckERC20 minter - #10685

Merged
gregorydemay merged 16 commits into
masterfrom
ic_DEFI-2920_deposit-address-derivation
Jul 9, 2026
Merged

feat: per-account deposit-address derivation for the ckETH/ckERC20 minter#10685
gregorydemay merged 16 commits into
masterfrom
ic_DEFI-2920_deposit-address-derivation

Conversation

@gregorydemay

@gregorydemay gregorydemay commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Each IC account gets a unique, deterministic Ethereum deposit address derived from the minter's threshold-ECDSA key, so a plain exchange withdrawal to that address can later be attributed and swept. The addresses are per-account and per-asset (separate ckERC20 and ckETH schemas), never collide with the minter's main address, and are signable by the minter — each address matches the derivation path the minter later signs with, so it is exactly what the minter can sweep from. Pure library code; registration, detection, and sweeping arrive in later tickets (F3 / DEFI-2921 onward).

Also in this PR:

  • a design-doc subsection consolidating the addressing model — the derivation tree (main, per-asset deposit, and sweeper addresses as independent siblings), which key signs which part of a sweep, and why Ethereum nonces stay per-address;
  • a behaviour-preserving consolidation of a duplicated principal-encoding helper, which now also rejects the empty principal (an input the minter can never receive).

Satisfies R1: determinism, uniqueness across principals / subaccounts / asset schemas, non-collision with the minter's main address, and EIP-55 encoding.

gregorydemay and others added 2 commits July 7, 2026 15:12
…minter)

Add F2 library support for deriving per-account Ethereum deposit and
sweeper addresses from the minter's master threshold-ECDSA public key
using non-empty derivation paths, reusing the existing public-key
derivation, Keccak-256/EIP-55 address machinery and deposit-account
principal/subaccount encoding. Pure library code, no runtime wiring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consolidate the size-prefixed principal-into-32-bytes encoding that was
duplicated across `deposit_address::principal_to_bytes32`, the
`principal_to_hex` helper binary and the test helper
`to_32_bytes_with_size_prefix` into a single `principal_to_bytes32`
paired with the existing `parse_principal_from_slice` decoder in
`eth_logs`. No behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces deterministic, per-account (principal + optional subaccount) and per-asset (ckETH vs ckERC20) Ethereum deposit-address derivation for the ckETH/ckERC20 minter, plus a small refactor to centralize principal→bytes32 encoding used across the codebase.

Changes:

  • Adds a new deposit_address module that derives per-account deposit addresses and a dedicated sweeper address via tECDSA derivation paths.
  • Introduces principal_to_bytes32 in eth_logs and refactors existing tests/tools to reuse it.
  • Adds unit tests validating determinism, uniqueness, non-collision with the main address, and EIP-55 checksumming.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rs/ethereum/cketh/minter/src/lib.rs Exposes the new deposit_address module.
rs/ethereum/cketh/minter/src/eth_logs/tests.rs Refactors tests to use shared principal_to_bytes32 helper.
rs/ethereum/cketh/minter/src/eth_logs/mod.rs Adds principal_to_bytes32 encoding helper.
rs/ethereum/cketh/minter/src/deposit_address.rs Implements derivation-path construction and address derivation for deposit/sweeper addresses.
rs/ethereum/cketh/minter/src/deposit_address/tests.rs Adds tests for determinism/uniqueness/non-collision and EIP-55 encoding.
rs/ethereum/cketh/minter/BUILD.bazel Updates Bazel deps so the principal_to_hex binary can depend on the minter library.
rs/ethereum/cketh/minter/bin/principal_to_hex.rs Uses the shared principal_to_bytes32 helper instead of duplicating encoding logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rs/ethereum/cketh/minter/src/eth_logs/mod.rs Outdated
gregorydemay and others added 3 commits July 7, 2026 15:42
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread rs/ethereum/cketh/minter/src/deposit_address.rs Outdated
gregorydemay and others added 3 commits July 8, 2026 07:11
Add a subsection to the CEX-deposit design consolidating the addressing model
that this PR implements: the master-key derivation tree (main, ckERC20/ckETH
deposit EOAs, and the sweeper address are independent siblings, not nested);
who signs what during a sweep (deposit-EOA path for the authorization tuple,
sweeper path for the type-0x04 transaction, main path for withdrawals); and
that Ethereum nonces are per-address and independent of the derivation tree.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
principal_to_bytes32 now asserts the principal is non-empty: the only
zero-length principal is the management canister, which can never call the
minter and therefore is never a deposit owner. Drops that impossible input
from the edge-case derivation test (keeping the 1-byte and 29-byte length
extremes) and adds a should_panic test locking the invariant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread rs/ethereum/cketh/minter/bin/principal_to_hex.rs Outdated
@gregorydemay
gregorydemay marked this pull request as ready for review July 8, 2026 07:33
@gregorydemay
gregorydemay requested a review from a team as a code owner July 8, 2026 07:33
@github-actions github-actions Bot added the @defi label Jul 8, 2026
@zeropath-ai

zeropath-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 4ee9662.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/ethereum/cketh/minter/src/lib.rs
    Add deposit_address module export
Enhancement ► rs/ethereum/cketh/minter/src/deposit_address/mod.rs
    Implement deposit/address derivation and sweeper address logic
    Derive addresses from master public key and derivation paths
► rs/ethereum/cketh/minter/src/deposit_address/tests.rs
    Add tests for address derivation behavior and stability

@mbjorkqvist mbjorkqvist left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @gregorydemay, just a couple of minor questions/comments!

Comment thread rs/ethereum/cketh/minter/src/eth_logs/mod.rs Outdated
Comment thread rs/ethereum/cketh/minter/src/deposit_address/tests.rs Outdated
Comment thread rs/ethereum/cketh/minter/BUILD.bazel Outdated
Comment thread rs/ethereum/cketh/minter/src/deposit_address.rs Outdated
gregorydemay and others added 5 commits July 9, 2026 16:07
Encode the deposit-address derivation path with variable-length
principal bytes (ckBTC-style) instead of the 32-byte size-prefixed
encoding: derivation-path elements do not need 32-byte alignment. This
reverses the shared principal_to_bytes32 extraction, since its only
productive callers are gone: the derivation path no longer uses it and
the principal_to_hex helper binary keeps its own inline encoder so it
stays independent of the minter. Also drops an unrelated EIP-55
reference-vector test from the deposit-address tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Take a single &Account (like the ckBTC minter) instead of a separate
owner principal and optional subaccount, and build the derivation path
from account.effective_subaccount() so a missing subaccount normalizes
to the default zero subaccount. Drops the LedgerSubaccount newtype from
the derivation path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…incipals

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gregorydemay and others added 3 commits July 9, 2026 16:46
…baccounts

Property-test that distinct subaccounts derive distinct addresses (one
arbitrary principal, an arbitrary set of 32-byte subaccounts), and cover
an arbitrary optional subaccount in the distinct-principals property.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eposit addresses

Property-test that distinct schemas derive distinct addresses and that a
derived deposit address collides with neither the main nor the sweeper
address (arbitrary principal, arbitrary optional subaccount), and make
the derivation-path helpers private now that the tests no longer use them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Assert hard-coded ckERC20/ckETH deposit addresses for two principals and
two subaccounts so any change to the derivation scheme is caught.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gregorydemay
gregorydemay requested a review from mbjorkqvist July 9, 2026 15:04
@gregorydemay
gregorydemay added this pull request to the merge queue Jul 9, 2026
Merged via the queue into master with commit 26d21ef Jul 9, 2026
39 checks passed
@gregorydemay
gregorydemay deleted the ic_DEFI-2920_deposit-address-derivation branch July 9, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants