feat(cketh): TimedSizedMap bounded, time-expiring map#10705
Open
gregorydemay wants to merge 8 commits into
Open
feat(cketh): TimedSizedMap bounded, time-expiring map#10705gregorydemay wants to merge 8 commits into
gregorydemay wants to merge 8 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new TimedSizedMap<K, V> utility in the ckETH minter crate: an in-heap, deterministic (caller-supplied time) map bounded by both a maximum entry count and a per-entry TTL, with eviction order of “expired first, then oldest”.
Changes:
- Added
TimedSizedMap+Timestampnewtype implementing TTL-based expiration and capacity eviction. - Added unit tests covering eviction/refresh/TTL edge cases and internal index consistency.
- Exported the new module from
lib.rsfor reuse by later F3 state/endpoint wiring.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rs/ethereum/cketh/minter/src/timed_sized_map.rs | New bounded, time-expiring map implementation (TimedSizedMap) with time index for eviction. |
| rs/ethereum/cketh/minter/src/timed_sized_map/tests.rs | Unit tests validating expiration, capacity eviction, refresh semantics, and index consistency. |
| rs/ethereum/cketh/minter/src/lib.rs | Exposes the new timed_sized_map module publicly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Base automatically changed from
ic_DEFI-2920_deposit-address-derivation
to
master
July 9, 2026 16:06
Add a generic in-heap map bounded by both a hard capacity and a per-entry time-to-live, evicting expired entries first and then the oldest to make room. Time is caller-supplied (Timestamp newtype) so the structure holds no clock and is deterministic to test. This is the standalone data structure that F3's per-account deposit-address registry will use to bound the active scanning set (DEFI-2921); wiring it into the minter state and the deposit_erc20 endpoint follows in later PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
remove_from_time_index is only called after an entry was removed from the
primary map, so the timestamp bucket and the key must exist. Replace the
silent no-op guards with expect("BUG: ...") so an inconsistent index fails
loudly instead of drifting and panicking later in eviction.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…emantics
- Remove the existing key before evicting expired entries, so re-inserting a
key never reports its own previous value as an eviction (previously the
behavior depended on whether that value had expired).
- Replace the capacity loop's silent `break` with expect("BUG: ..."): an
empty time index while entries is non-empty is an index-corruption bug, not
a reason to insert past the hard capacity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aaf656e to
4411504
Compare
gregorydemay
commented
Jul 10, 2026
A still-armed deposit address must never be silently dropped from the watchlist: it may already have received funds. So TimedSizedMap::insert now evicts only expired entries and, if the map is still at capacity with only live entries, rejects a new key with an AtCapacity error rather than evicting a live one. Re-inserting an existing key still refreshes it (and is never rejected or reported as an eviction). Removes the now-unused evict_oldest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Relocate the file-module src/timed_sized_map.rs to src/timed_sized_map/mod.rs, alongside its tests submodule. No code change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removing an existing key already drops the map below capacity (len <= capacity is invariant), so the capacity check alone is sufficient; the separate refreshed flag never changed the outcome. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gregorydemay
commented
Jul 10, 2026
gregorydemay
commented
Jul 10, 2026
… test - Expire an entry only once its age strictly exceeds `ttl` (now - inserted_at > ttl), so an entry lives for the full ttl. Replaces the inclusive saturating_add comparison with a checked duration. - Derive Clone/Eq/PartialEq on TimedSizedMap and assert the whole map is unchanged after a rejected insertion (clone-before, compare-after). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gregorydemay
commented
Jul 10, 2026
A caller must not be able to refresh (extend) a still-live entry by re-inserting it, otherwise it could hold a slot indefinitely by repeatedly calling. insert now returns InsertError::AlreadyPresent for a live key; re-admission is only possible once the entry has expired. The error type becomes an enum (AlreadyPresent | AtCapacity), and the now-unused by-key remove helpers are dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to f9e944d. Security Overview
Detected Code Changes
|
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.
Adds a generic in-heap map bounded by both a hard entry cap and a per-entry time-to-live, which will be used to store addresses that needs to be watch over for deposits.
On insertion it first evicts entries that have outlived their TTL; it then rejects rather than evicts a live entry — returning
AtCapacityif the map is full of live entries, orAlreadyPresentif the key already has a live entry (a caller cannot refresh/extend an entry by re-inserting it; re-admission is only possible once it has expired). Time is supplied by the caller (aTimestampnewtype), so the structure keeps no internal clock and is deterministic to test.Modeled on the bounded cache in
dfinity/canhttp(not a workspace dependency, hence a small local structure).