Repository navigation
feat(minter): record the origin of a queued deposit - #291
Draft
mbjorkqvist wants to merge 118 commits into
Draft
mbjorkqvist wants to merge 118 commits into
mbjorkqvist wants to merge 118 commits into
Conversation
Replace the outdated section 3.1.1, which described per-transaction crediting with getSignaturesForAddress and quotas replenished by the removed process_deposit endpoint, with a design that builds on the balance sweep of deposit_sol: a free registration endpoint, a scan timer reading watched deposit addresses in batches with getMultipleAccounts, a schedule adapted from the ckETH minter, a fixed fee in SOL deducted from the minted amount, and a watchlist persisted only as a bounded snapshot at pre_upgrade. Every change to the interface, the arguments and the event log is additive, since the minter is deployed on mainnet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
State that a re-armed account is reported as watching again, that the main account receives the sweepable amount minus the fee share, correct the worst-case cost arithmetic and state its success assumption, and distinguish a failed upgrade from a reinstallation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ring Move the sequence diagram of the automatic flow ahead of its explanation, remove the semicolon that broke the mermaid parser on GitHub, and define the deposit id where it is first used in the section. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…he constraint State that scan ticks are guarded like the other timers so that they never overlap, and base the automatic deposit fee constraint on the single-signature fee share that a pending mint actually bears. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Drop the separate status query: calling the registration endpoint again returns the stored registration or the id of the deposit in flight, as deposit_eth does in the ckETH minter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… signature lookup Spell out that scan ticks never overlap, that the fee derivation is a single-attempt estimate with re-arm costs absorbed by the minter, and that only manual deposits are retried by the caller. Remove getSignaturesForAddress from the RPC inventory and cost tables, since the minter no longer uses it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A re-armed entry restarts its schedule from the first gap, bounded by the remaining window, and a re-arm only ever follows a detection. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Record the watchlist snapshot on every upgrade so that an older one cannot be replayed after the watchlist emptied, and let the transaction submission section account for deposits queued by the balance scan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The window ends with the last scan, and the next registration call starts a new window rather than reporting a registration without a next scan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…it ids Follow the ckETH minter: a registration freezes neither the minimum deposit amount nor the automatic deposit fee, both are quoted and the values in force at detection apply, the fee being recorded with the queued deposit. deposit_sol compares the latest deposit id of the account before and after its balance read, and a registration is allowed while a deposit_sol call is in progress. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…edule Measure gaps from the previous scan as before, but end the window at a fixed 24 hours after registration, as the ckETH minter does, so that the accumulated tick and call delays have slack and clients and the re-arm share one unambiguous boundary. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ence Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…owner rationale The fee recorded at detection matters for deposits an upgrade finds not yet credited; a credited deposit has its mint amount in CreditedSweep. The minter's principal is excluded as a deposit owner because its default account is the ledger's minting account, not because of an address collision. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…xample with the automatic flow Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…er a concurrent scan Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n delay and the fee worst case Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…or to new registrations Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…vents the automatic flow adds Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…tric to alert on Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… of chaining gaps Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The re-arm registers the account again instead of restoring the original window, so the queued deposit carries only the automatic deposit fee. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… classification from deposit_sol Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The re-arm registers the account again instead of restoring the window end recorded with the deposit, so a drop after the original window re-arms as well and the window-closed error disappears. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…w name Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t per address The balances are paired with the addresses by position, and the SOL RPC canister does not guarantee one account per requested address in the pinned version, so a short response would leave the addresses past its end unread. A response with the wrong number of accounts is now an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pins the parameters, the response size estimate and the cycles the minter chooses for 1 and 100 addresses, which the tests of the response alone leave unchecked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e to the SOL RPC canister As for every other call, the SOL RPC canister prices the call with its own default estimate, which allows 192 bytes per account without data in its upcoming release, close to what the minter estimated, so the minter no longer mirrors the canister's internal sizes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Recording it when the responses arrive skipped a slot that passed while the reads were in flight. The registration the reads were made for must still be live when they arrive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a failed read A tick that finds the previous one still waiting for its calls makes no call, and a failed read of a registration that was replaced or expired meanwhile leaves the current registration untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…balance read of deposit_sol Pins why deposit_sol compares the latest deposit id of the account rather than the one in flight: a deposit queued by a scan and minted before the balance read returns must not free the account for a second deposit from the stale balance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d one Since a balance scan of the automatic flow can queue the deposit an account's deposit_sol call returns, its mint may be reduced by the automatic deposit fee. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follows the minter, which no longer passes a response size estimate with getMultipleAccounts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d-sweeps Keeps the version of Watchlist::register of the base branch, without the Expired arm that the base removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the state transition The re-arm ran in the state transition of FailedTransaction and ExpiredTransaction, so post_upgrade replayed it against a watchlist without the transient registrations it was made against, and replayed its log line at every upgrade. The live caller now records the drop and then re-arms through process_dropped_sweep, which logs the outcome once, with a refused re-arm at Info. The state transition only drops the deposits again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The docs of the swept deposits and of the Dropped status still said that only deposit_sol follows a dropped sweep. The re-arm no longer claims to keep an existing registration, since the account of a deposit in flight is never watched, and the test of that impossible case goes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… event log Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e live one Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the pins to the dfinity/sol-rpc-canister#394 commit with the released sol_rpc_client 7.0.0 and sol_rpc_types 4.0.0, and run the integration tests against the SOL RPC canister v1.4.0 release wasm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ing it from its fee Whether a deposit was queued by `deposit_sol` or by the automatic flow was only implied by its fee being absent or present, which the re-arm of a dropped sweep relied on. A shared `DepositOrigin` now records the flow on its own, in memory and in the `QueuedDeposit` event, so that a fee can be charged in either flow. Events recorded without an origin predate the automatic flow and decode as manual deposits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nual Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eichhorl
approved these changes
Nov 9, 2026
Comment on lines
+442
to
+443
| /// The flow that queued a deposit, recorded on its own rather than inferred from the | ||
| /// deposit fee, so that a fee can be charged in either flow. |
Contributor
There was a problem hiding this comment.
🤖 With a fee now possible in either flow, collected_automatic_deposit_fees still relies on the rule this PR removes from the re-arm: process_credited_sweep adds sweep.deposit_fees(), i.e. the fees of every deposit of the sweep, whatever its origin. A fee charged for manual deposits would then be reported as automatic deposit fees in get_minter_info, the metrics and the dashboard, and that figure is what section 3.3.2 of the design recalibrates the automatic deposit fee from. Counting only the fees of DepositOrigin::Automatic deposits there would make the origin the single source of truth; if a total of all deposit fees is wanted instead, the counter needs a new name while MinterInfo.collected_automatic_deposit_fees is still unreleased. Either way the deployed logs replay to the same value, since they hold no automatic deposit yet.
Contributor
Author
There was a problem hiding this comment.
🤖 Thanks, good catch! Fixed in 7e08725: the counter now only adds the fees of the sweep's deposits whose recorded origin is Automatic, so it keeps its name and meaning, and a fee charged for manual deposits would no longer be reported as an automatic deposit fee. The check that the mints and fees don't exceed the amount received still uses the fees of all deposits, since every fee stays on the main account. A new state test credits a sweep with a manual deposit charged a fee and an automatic one, and checks that only the automatic fee is collected and that the event log replays to the same state.
… deposit fees The collected automatic deposit fees summed the fees of every deposit of a credited sweep, which relied on only automatic deposits being charged a fee. Count the fees by the recorded origin instead, so that a fee charged for manual deposits is not reported as an automatic deposit fee. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-origin # Conflicts: # integration_tests/tests/tests.rs # minter/src/deposit/automatic/scan/mod.rs # minter/src/deposit/automatic/scan/tests.rs # minter/src/deposit/sweep/tests.rs # minter/src/state/tests.rs # minter/src/state/watchlist/mod.rs # minter/src/state/watchlist/tests.rs
This branch has not been deployed
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.
Whether a deposit was queued manually via
deposit_solor by the automatic flow is currently implied by its fee: no fee means manual. The re-arm of a dropped sweep (#284) and the planned metrics (#287) rely on that rule, which nothing enforces and which breaks as soon as the fee of either flow is chosen otherwise: a manual flow that charges a fee, as the manual ckSPL deposits may, or an automatic flow that charges none, as a ckSPL automatic flow might at launch, like the automatic ckERC20 deposits, which are live and charge no fee yet.This PR records the origin of a queued deposit explicitly, in memory and in the
QueuedDepositevent, next to the fee instead of being inferred from it, so that the fees of both flows can be chosen freely and the same origin can be shared with ckSPL deposits. AQueuedDepositevent recorded without an origin predates the automatic flow and is read as a manual deposit.This must be merged before any minter version recording automatic deposits is deployed, so that no deployed event log holds an automatic deposit without its origin. Recording the origin later would require keeping the fee rule forever to read the events recorded in between.
🤖 Generated with Claude Code