feat(cketh): deposit_erc20 endpoint (derive + store deposit addresses) by gregorydemay · Pull Request #10729 · dfinity/ic

gregorydemay · 2026-07-10T17:50:54Z

Second slice of DEFI-2921 ([F3] registration state and deposit_erc20), stacked on #10705 (the TimedSizedMap structure).

Adds the deposit_erc20(account, DepositMode) -> Result<text, DepositErc20Error> update endpoint. In DeductFromDeposit mode it derives the account's per-account ckERC20 deposit address (threshold-ECDSA, via the F2 derivation) and registers the account → address mapping in a bounded, time-expiring in-heap registry (TimedSizedMap), returning the EIP-55 address. Behaviour:

re-registering a still-armed account returns the same address without re-arming it (idempotent);
a full registry returns TooManyActiveAddresses;
Sponsored mode returns TemporarilyUnavailable — caller-paid fee handling is a later ticket.

The registry survives upgrades via a snapshot event emitted at pre_upgrade and replayed on post_upgrade (the existing log_scrapings pattern), and is covered by is_equivalent_to. A new chain-code accessor lets the minter derive the per-account addresses from its master key.

Store-only: nothing is done with a registered address yet — no scanning, detection, sweeping, or fee charging. Using the stored addresses (detection/crediting, sweeping) comes in separate follow-up PRs.

Notes for review:

Re-calling for an already-armed address returns the address without extending its window (idempotent get; the no-refresh rule is enforced by the structure).
The active-set capacity is a placeholder (100k) to be tuned when the scanning task lands.

…esses Add the deposit_erc20(account, DepositMode) update endpoint. In DeductFromDeposit mode it derives the account's ckERC20 deposit address and registers the account -> address mapping in a bounded, time-expiring in-heap registry (TimedSizedMap), returning the EIP-55 address. Re-registering a still-armed account returns the same address without re-arming it; a full registry yields TooManyActiveAddresses. Sponsored mode returns TemporarilyUnavailable (fee handling is a later ticket). The registry survives upgrades via a pre_upgrade snapshot event replayed on post_upgrade (the log_scrapings pattern), and is covered by is_equivalent_to. Adds a chain-code accessor (lazy_call_ecdsa_public_key_with_chain_code) so the minter can derive the per-account addresses. No scanning, detection, sweeping, or fee charging yet — the stored addresses are not otherwise used. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a new deposit_erc20(account, DepositMode) update endpoint to the ckETH minter to derive per-account ckERC20 deposit addresses (threshold ECDSA) and register them in a bounded, TTL-based in-heap registry that survives upgrades via snapshot/replay events.

Changes:

Introduces deposit_erc20 endpoint (+ candid types DepositMode / DepositErc20Error) and a new deposit_erc20 module implementing address derivation + registration.
Extends TimedSizedMap with iter_with_time() and from_entries() to support snapshot-based restore of the deposit-address registry.
Adds a new audit-log snapshot event (RegisteredDepositAddresses) emitted at pre-upgrade and replayed post-upgrade to restore the registry.

Reviewed changes

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

Show a summary per file

File	Description
rs/ethereum/cketh/minter/src/timed_sized_map/tests.rs	Adds coverage for timestamped iteration and snapshot rebuild via `from_entries()`.
rs/ethereum/cketh/minter/src/timed_sized_map/mod.rs	Adds `iter_with_time()` and `from_entries()` for snapshot/restore support.
rs/ethereum/cketh/minter/src/state/tests.rs	Adds a round-trip snapshot test for deposit address registry via audit events.
rs/ethereum/cketh/minter/src/state/event.rs	Adds `RegisteredDepositAddresses` event + `DepositAddressRegistration` payload type.
rs/ethereum/cketh/minter/src/state/audit.rs	Rebuilds `deposit_addresses` from snapshot event during replay.
rs/ethereum/cketh/minter/src/state.rs	Adds deposit-address registry to `State` + constants; adds chain-code accessor for ECDSA pk.
rs/ethereum/cketh/minter/src/main.rs	Emits snapshot at pre-upgrade; adds `deposit_erc20` endpoint; filters snapshot events from `get_events`.
rs/ethereum/cketh/minter/src/lifecycle/init.rs	Initializes `deposit_addresses` in `State` from init args.
rs/ethereum/cketh/minter/src/lib.rs	Exposes new `deposit_erc20` module.
rs/ethereum/cketh/minter/src/endpoints.rs	Adds candid types `DepositMode` / `DepositErc20Error`.
rs/ethereum/cketh/minter/src/deposit_erc20/tests.rs	Adds unit tests for derive/store, idempotent re-registration, and Sponsored rejection.
rs/ethereum/cketh/minter/src/deposit_erc20/mod.rs	Implements `register_deposit_address()` (derive + bounded insert).
rs/ethereum/cketh/minter/cketh_minter.did	Updates public interface with `deposit_erc20` and related types.

Comments suppressed due to low confidence (1)

rs/ethereum/cketh/minter/src/main.rs:912

get_events now filters out RegisteredDepositAddresses via filter_map, but still uses storage::total_event_count() (which includes the filtered-out events) and applies start/length to the unfiltered stream. This breaks pagination semantics for callers that use start = events.len() and can lead to loops that never reach total_event_count (see state/audit/tests.rs retrieval logic). Consider treating start/total_event_count as referring to the public (filtered) event stream and compute both accordingly in one pass.

    let events = storage::with_event_iter(|it| {
        it.skip(arg.start as usize)
            .take(arg.length.min(MAX_EVENTS_PER_RESPONSE) as usize)
            .filter_map(map_event)
            .collect()
    });

    GetEventsResult {
        events,
        total_event_count: storage::total_event_count(),
    }
}

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

gregorydemay · 2026-07-10T18:19:55Z

+    let address = deposit_address(
+        master_public_key,
+        chain_code,
+        DepositAddressSchema::CkErc20,
+        &account,
+    );
+    let address_string = address.to_string();
+
+    match state.deposit_addresses.insert(now, account, address) {
+        Ok(_) => Ok(address_string),
+        Err(InsertError::AlreadyPresent { .. }) => Ok(address_string),
+        Err(InsertError::AtCapacity { .. }) => Err(DepositErc20Error::TooManyActiveAddresses),
+    }


🤖 Done. On AlreadyPresent, the endpoint now returns the address stored in the registry (deposit_addresses.get(now, &account)) rather than the freshly derived one, so the "same address" guarantee holds by construction and we never return an address that is not actually registered.

On InsertError::AlreadyPresent, return the address actually stored in the registry rather than the freshly derived one, so the idempotency guarantee holds by construction instead of assuming the derived address equals the stored one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

rs/ethereum/cketh/minter/src/main.rs:912

get_events now filters out RegisteredDepositAddresses after applying start/length and still returns storage::total_event_count() (which counts filtered-out events). This breaks pagination: clients that use total_event_count and start = events.len() can loop forever or fetch overlapping pages once a snapshot event exists (because the server-side index includes hidden events but the client-visible index does not).

    let events = storage::with_event_iter(|it| {
        it.skip(arg.start as usize)
            .take(arg.length.min(MAX_EVENTS_PER_RESPONSE) as usize)
            .filter_map(map_event)
            .collect()
    });

    GetEventsResult {
        events,
        total_event_count: storage::total_event_count(),
    }
}

gregorydemay · 2026-07-10T18:42:54Z

+    pub fn from_entries(
+        ttl: Duration,
+        capacity: NonZeroUsize,
+        entries: impl IntoIterator<Item = (Timestamp, K, V)>,
+    ) -> Self {
+        let mut map = Self::new(ttl, capacity);
+        for (inserted_at, key, value) in entries {
+            map.entries
+                .insert(key.clone(), Entry { value, inserted_at });
+            map.by_time.entry(inserted_at).or_default().push_back(key);
+        }
+        map
+    }


🤖 Done — from_entries now asserts each key is distinct (BUG: from_entries received a duplicate key), so a corrupt snapshot fails loudly at restore instead of silently diverging entries/by_time and panicking later during eviction. Added a #[should_panic] test. In practice the snapshot comes from a BTreeMap iterator so keys are always unique, but this guards the invariant on the pub method.

gregorydemay · 2026-07-10T18:42:56Z

+    let deposit_addresses = read_state(|s| {
+        s.deposit_addresses
+            .iter_with_time()
+            .map(
+                |(registered_at, account, address)| DepositAddressRegistration {
+                    owner: account.owner,
+                    subaccount: account.subaccount,
+                    address: *address,
+                    registered_at_nanos: registered_at.as_nanos(),
+                },
+            )
+            .collect::<Vec<_>>()
+    });


🤖 Done — replaced iter_with_time with iter_live(now), which filters out expired entries, and the pre_upgrade snapshot now uses it, so dormant (expired) registrations are no longer encoded/persisted.

github-actions Bot added the feat label Jul 10, 2026

gregorydemay requested a review from Copilot July 10, 2026 17:51

Copilot started reviewing on behalf of gregorydemay July 10, 2026 17:51 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

gregorydemay requested a review from Copilot July 10, 2026 18:20

Copilot started reviewing on behalf of gregorydemay July 10, 2026 18:20 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(cketh): deposit_erc20 endpoint (derive + store deposit addresses)#10729

feat(cketh): deposit_erc20 endpoint (derive + store deposit addresses)#10729
gregorydemay wants to merge 2 commits into
ic_DEFI-2921_bounded-mapfrom
ic_DEFI-2921_deposit-erc20-endpoint

gregorydemay commented Jul 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gregorydemay Jul 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gregorydemay Jul 10, 2026

Uh oh!

gregorydemay Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gregorydemay commented Jul 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

gregorydemay Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

gregorydemay Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

gregorydemay Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants