diff --git a/docs/src/development/design.md b/docs/src/development/design.md index df5720fcb..59fe48697 100644 --- a/docs/src/development/design.md +++ b/docs/src/development/design.md @@ -171,7 +171,7 @@ Entries leave the watchlist when a scan queues a deposit, when a `deposit_sol` c **Detection.** A tick collects the due entries, reads the balances of their deposit addresses with `getMultipleAccounts` at the `finalized` commitment level, up to 100 addresses per call and at most `MAX_CONCURRENT_RPC_CALLS` calls concurrently, so that one tick covers the full watchlist, and treats a missing account as a zero balance. Deposit addresses are plain system accounts without data, so the request asks for no account data and the response is about 150 bytes per address; the cycles attached to a call are sized by the number of addresses it carries, while its response size is left to the default estimate of the SOL RPC canister, as for every other call (see [Section 3.3.2](#332-cksol-minter-fees)). The call uses the typed `getMultipleAccounts` endpoint of the SOL RPC canister, so the automatic flow requires a version of that canister which offers it. A call that fails counts as a scan of its entries, which advance to their next slot as after a balance below the minimum, so that a persistent failure of the SOL RPC canister costs the ckSOL minter no more than the schedule and costs the user the scans lost to the failure. -An address whose balance is at or above the minimum deposit amount in force at that moment is queued exactly as `deposit_sol` queues it: the sweepable amount is the balance minus the rent exemption threshold, the deposit gets the next deposit id, and the `QueuedDeposit` event is recorded, carrying in addition the automatic deposit fee in force at that moment. The recorded fee is the one deducted when the deposit is credited, so a fee change by upgrade after detection does not affect it: a deposit that an upgrade finds queued, swept or finalized is credited afterwards with the fee restored from its `QueuedDeposit`, and a deposit already credited has its mint amount recorded in `CreditedSweep`, as for any sweep; the constraint of [Section 3.3.4](#334-parameter-constraints) is checked against the current parameters at every init and upgrade, so a deposit detected under them always mints a positive amount. The trust model is that of the manual flow: the balance is read at `finalized` with the 3-out-of-4 provider consensus and becomes the planned sweep amount, and a wrong balance cannot cause an over-mint, since ckSOL is minted only after the sweep has been finalized and settled against the finalized transaction. This differs from the ckETH minter, where the scanned balance is only a trigger. An address whose balance is below the minimum, including one that holds only the rent exemption threshold left by a previous sweep, advances to its next scan. +An address whose balance is at or above the minimum deposit amount in force at that moment is queued exactly as `deposit_sol` queues it: the sweepable amount is the balance minus the rent exemption threshold, the deposit gets the next deposit id, and the `QueuedDeposit` event is recorded, carrying in addition the automatic deposit fee in force at that moment and the origin of the deposit, automatic rather than manual. The origin is recorded on its own rather than inferred from the presence of a fee, so that what depends on the flow, such as the re-arm of a dropped sweep below, stays right should a fee ever be charged for manual deposits. The recorded fee is the one deducted when the deposit is credited, so a fee change by upgrade after detection does not affect it: a deposit that an upgrade finds queued, swept or finalized is credited afterwards with the fee restored from its `QueuedDeposit`, and a deposit already credited has its mint amount recorded in `CreditedSweep`, as for any sweep; the constraint of [Section 3.3.4](#334-parameter-constraints) is checked against the current parameters at every init and upgrade, so a deposit detected under them always mints a positive amount. The trust model is that of the manual flow: the balance is read at `finalized` with the 3-out-of-4 provider consensus and becomes the planned sweep amount, and a wrong balance cannot cause an over-mint, since ckSOL is minted only after the sweep has been finalized and settled against the finalized transaction. This differs from the ckETH minter, where the scanned balance is only a trigger. An address whose balance is below the minimum, including one that holds only the rent exemption threshold left by a previous sweep, advances to its next scan. **Concurrency.** Scan ticks never overlap: the scan runs under the same single-round guard as the sweep and finalization timers, so a tick that fires while the previous one is still waiting for its calls returns immediately, and the bound of `MAX_CONCURRENT_RPC_CALLS` in-flight calls holds. The scan and `deposit_sol`, however, may read the balance of the same address at the same time, since both span an inter-canister call, and neither path may assert on the other. Before its call, the scan skips the accounts for which a `deposit_sol` call is in progress; after the call, it checks each entry again and skips it silently if it is no longer watched under the same registration, if its window has passed in the meantime, or if the account has a deposit in flight; the window end thus holds even for a read that started just before it, at the price of discarding that one read. Symmetrically, `deposit_sol` records the id of the latest deposit of the account before its `getBalance` call and compares it after: if it changed, a scan queued a deposit in the meantime, so the call returns that id, whatever state the deposit has reached since, and charges only the RPC call. Comparing the id rather than checking whether a deposit is in flight also covers the theoretical case of the scan's deposit being minted or dropped before the callback runs, which would otherwise free the account for a second deposit from a stale balance. An account registered while a tick is running is scanned at the next tick. Since the watchlist is heap state, an upgrade in the middle of a tick loses nothing but that tick: the snapshot taken before the upgrade carries the watchlist, and the scan is redone afterwards. @@ -181,7 +181,7 @@ An address whose balance is at or above the minimum deposit amount in force at t The event log is not a vector: a registration records nothing, so an adversary cannot grow the log for free, and the only events the automatic flow adds to the ones a detected deposit shares with a manual one (the submission, settlement, mint and possible failure or expiry of its sweep) are the `QueuedDeposit` of the detected deposit, which requires at least the minimum deposit amount on chain, and the watchlist snapshot taken once per upgrade, whose size is bounded by the capacity, see [Section 3.5](#35-events). A full watchlist blocks honest registrations, and the manual flow remains available in that case; since filling it is free, the number of live registrations is exposed as a metric so that an alert can fire before the capacity is reached. -**Compatibility.** The automatic flow is added to a ckSOL minter that is deployed on mainnet, so it only extends the interface and the state: one new endpoint; a new optional `automatic_deposit_fee` field in the init and upgrade arguments and in `get_minter_info`, together with the collected fees; an optional field of the `QueuedDeposit` event carrying the fee of an automatic deposit, absent for manual deposits; a new snapshot event recorded by a new `pre_upgrade` hook; and a new timer. The types of `deposit_sol` and `deposit_sol_status` and the stable memory layout are unchanged, and an event log written before the upgrade replays unchanged. +**Compatibility.** The automatic flow is added to a ckSOL minter that is deployed on mainnet, so it only extends the interface and the state: one new endpoint; a new optional `automatic_deposit_fee` field in the init and upgrade arguments and in `get_minter_info`, together with the collected fees; two fields of the `QueuedDeposit` event, an optional one carrying the fee of the deposit, absent if none is charged, and one carrying its origin, which events recorded before the automatic flow lack and which then reads as manual, since all of them were queued by `deposit_sol`; a new snapshot event recorded by a new `pre_upgrade` hook; and a new timer. The types of `deposit_sol` and `deposit_sol_status` and the stable memory layout are unchanged, and an event log written before the upgrade replays unchanged. #### 3.1.2. Manual Flow diff --git a/integration_tests/tests/snapshot.rs b/integration_tests/tests/snapshot.rs index 07cf80d62..3da3993da 100644 --- a/integration_tests/tests/snapshot.rs +++ b/integration_tests/tests/snapshot.rs @@ -3,7 +3,7 @@ use cksol_int_tests::CkSolMinter; use cksol_types::MinterInfo; use cksol_types_internal::{ UpgradeArgs, - event::{Event, EventType}, + event::{DepositOrigin, Event, EventType}, }; use pocket_ic::PocketIcBuilder; @@ -44,7 +44,12 @@ async fn should_load_staging_snapshot_and_upgrade_to_current_wasm() { } ); let (replayed_events, new_events) = state_after.events.split_at(state_before.events.len()); - assert_eq!(replayed_events, state_before.events); + let expected_events: Vec<_> = state_before + .events + .into_iter() + .map(with_recorded_origin) + .collect(); + assert_eq!(replayed_events, expected_events); assert_matches!( new_events, [Event { payload: EventType::Upgrade(args), .. }] if *args == upgrade_args @@ -52,3 +57,12 @@ async fn should_load_staging_snapshot_and_upgrade_to_current_wasm() { env.drop().await; } + +/// The event as reported by the current wasm: a `QueuedDeposit` recorded without an origin +/// predates the automatic flow, so it reports the deposit as queued by `deposit_sol`. +fn with_recorded_origin(mut event: Event) -> Event { + if let EventType::QueuedDeposit { origin, .. } = &mut event.payload { + origin.get_or_insert(DepositOrigin::Manual); + } + event +} diff --git a/integration_tests/tests/tests.rs b/integration_tests/tests/tests.rs index c8cdf8d9f..879572c6e 100644 --- a/integration_tests/tests/tests.rs +++ b/integration_tests/tests/tests.rs @@ -16,7 +16,7 @@ use cksol_types::{ }; use cksol_types_internal::{ UpgradeArgs, - event::{EventType, TransactionPurpose}, + event::{DepositOrigin, EventType, TransactionPurpose}, log::Priority, }; use icrc_ledger_types::icrc1::account::{Account, Subaccount}; @@ -870,6 +870,7 @@ mod deposit_sol_tests { address: deposit_address.clone(), balance: BALANCE_ABOVE_MINIMUM, deposit_fee: None, + origin: Some(DepositOrigin::Manual), }] ); }); @@ -951,6 +952,7 @@ mod deposit_sol_tests { address: deposit_address.clone(), balance: BALANCE_ABOVE_MINIMUM, deposit_fee: None, + origin: Some(DepositOrigin::Manual), }] ); }); @@ -1325,6 +1327,7 @@ mod automatic_deposit_sol_tests { address: DEFAULT_CALLER_DEPOSIT_ADDRESS.parse().unwrap(), balance: BALANCE_ABOVE_MINIMUM, deposit_fee: Some(Setup::DEFAULT_AUTOMATIC_DEPOSIT_FEE), + origin: Some(DepositOrigin::Automatic), }] ); }); diff --git a/libs/types-internal/src/event.rs b/libs/types-internal/src/event.rs index 24d1d8772..3017f763b 100644 --- a/libs/types-internal/src/event.rs +++ b/libs/types-internal/src/event.rs @@ -76,9 +76,11 @@ pub enum EventType { address: Address, /// The balance of the deposit address when the deposit was queued. balance: Lamport, - /// The fee in lamports deducted from the minted amount; absent for a deposit queued via - /// `deposit_sol`. + /// The fee in lamports deducted from the minted amount; absent if none is charged. deposit_fee: Option, + /// The flow that queued the deposit; absent in events recorded before the automatic + /// flow, whose deposits were all queued via `deposit_sol`. + origin: Option, }, /// The minter read the amount that the finalized sweep transaction moved to its /// main account and enqueued a pending mint for each deposit of that sweep. @@ -155,6 +157,15 @@ pub struct CreditedDeposit { pub amount_to_mint: Lamport, } +/// The flow that queued a deposit. +#[derive(Clone, Copy, Debug, PartialEq, Eq, CandidType, Deserialize)] +pub enum DepositOrigin { + /// Queued by a user via `deposit_sol`. + Manual, + /// Queued by a balance scan of the automatic flow. + Automatic, +} + /// The key that produced one signature of a submitted Solana transaction. #[derive(Clone, Debug, PartialEq, CandidType, Deserialize)] pub enum Signer { diff --git a/minter/cksol_minter.did b/minter/cksol_minter.did index 46ee24fd8..e5732ed0a 100644 --- a/minter/cksol_minter.did +++ b/minter/cksol_minter.did @@ -418,6 +418,17 @@ type VersionedTransactionMessage = variant { Legacy : blob; }; +// The flow that queued a deposit. +// +// *WARNING*: This type is used exclusively by the debug `get_events` endpoint. +// Backwards-compatibility is not guaranteed. +type DepositOrigin = variant { + // Queued by a user via `deposit_sol`. + Manual; + // Queued by a balance scan of the automatic flow. + Automatic; +}; + // The key that produced one signature of a submitted Solana transaction. // // *WARNING*: This type is used exclusively by the debug `get_events` endpoint. @@ -508,9 +519,12 @@ type EventType = variant { address: Address; // The balance of the deposit address when the deposit was queued. balance: Lamport; - // The fee in lamports deducted from the minted amount; absent for a deposit - // queued via `deposit_sol`. + // The fee in lamports deducted from the minted amount; absent if none is + // charged. deposit_fee: opt Lamport; + // The flow that queued the deposit; absent in events recorded before the + // automatic flow, whose deposits were all queued via `deposit_sol`. + origin: opt DepositOrigin; }; // The minter read the amount that the finalized sweep transaction moved to // its main account and enqueued a pending mint for each deposit of that sweep. diff --git a/minter/src/canbench.rs b/minter/src/canbench.rs index 16f53dd10..61b2cbb06 100644 --- a/minter/src/canbench.rs +++ b/minter/src/canbench.rs @@ -7,7 +7,7 @@ use crate::{ runtime::IcCanisterRuntime, sol_transfer::build_batch_withdrawal_message, state::{ - DepositBalance, QueuedDeposit, SchnorrPublicKey, Sweep, + DepositBalance, DepositOrigin, QueuedDeposit, SchnorrPublicKey, Sweep, audit::{process_event, replay_events}, event::{ CreditedDeposit, EventType, Signer, TransactionPurpose, VersionedMessage, @@ -109,6 +109,7 @@ fn queue_and_sweep(deposit_id: u64, account_index: usize, amount: u64, sig: Sign address: deposit_address(account_index), balance: DepositBalance::new(amount + RENT_EXEMPTION_THRESHOLD) .expect("BUG: the balance covers the rent exemption threshold"), + origin: DepositOrigin::Manual, deposit_fee: None, }; record(EventType::QueuedDeposit { @@ -117,6 +118,7 @@ fn queue_and_sweep(deposit_id: u64, account_index: usize, amount: u64, sig: Sign address: deposit.address, balance: deposit.balance, deposit_fee: None, + origin: DepositOrigin::Manual, }); let sweep = Sweep::plan([(deposit_id, deposit)], minter_address(&master_key())); record(EventType::SubmittedTransaction { diff --git a/minter/src/deposit/automatic/scan/mod.rs b/minter/src/deposit/automatic/scan/mod.rs index 44ebe443c..7963482e9 100644 --- a/minter/src/deposit/automatic/scan/mod.rs +++ b/minter/src/deposit/automatic/scan/mod.rs @@ -5,8 +5,8 @@ use crate::{ rpc::{GetMultipleAccountsError, get_balances}, runtime::CanisterRuntime, state::{ - DepositBalance, MAX_WATCHED_ADDRESSES, QueuedDeposit, State, TaskType, WatchedAddress, - mutate_state, read_state, + DepositBalance, DepositOrigin, MAX_WATCHED_ADDRESSES, QueuedDeposit, State, TaskType, + WatchedAddress, mutate_state, read_state, }, storage::{ScanOutcome, record_deposit_address_scans}, utils::timed_sized_map::Timestamp, @@ -153,6 +153,7 @@ fn apply_balance_read( account: watched.account, address: watched.address, balance, + origin: DepositOrigin::Automatic, deposit_fee: Some(automatic_deposit_fee), }, ); diff --git a/minter/src/deposit/automatic/scan/tests.rs b/minter/src/deposit/automatic/scan/tests.rs index 2c83d0890..5573e3086 100644 --- a/minter/src/deposit/automatic/scan/tests.rs +++ b/minter/src/deposit/automatic/scan/tests.rs @@ -6,7 +6,7 @@ use crate::{ Scan, apply_balance_read, record_failed_read, scan_watched_addresses, }, guard::deposit_sol_guard, - state::{TaskType, WatchedAddress, event::EventType, mutate_state, read_state}, + state::{DepositOrigin, TaskType, WatchedAddress, event::EventType, mutate_state, read_state}, storage::{ScanOutcome, deposit_address_scan_count}, test_fixtures::{ AUTOMATIC_DEPOSIT_FEE, EventsAssert, MINIMUM_DEPOSIT_AMOUNT, MINUTE_NANOS, REGISTERED_AT, @@ -85,6 +85,7 @@ async fn should_queue_a_deposit_reaching_the_minimum_with_the_fee_in_force() { address: expected.address, balance: expected.balance, deposit_fee: Some(AUTOMATIC_DEPOSIT_FEE), + origin: DepositOrigin::Automatic, }) .assert_no_more_events(); read_state(|s| { diff --git a/minter/src/deposit/sweep/mod.rs b/minter/src/deposit/sweep/mod.rs index fa2d0b73a..ad699d400 100644 --- a/minter/src/deposit/sweep/mod.rs +++ b/minter/src/deposit/sweep/mod.rs @@ -6,7 +6,7 @@ use crate::{ rpc::get_balance, runtime::CanisterRuntime, state::{ - DepositBalance, InFlightDeposit, QueuedDeposit, State, audit::process_event, + DepositBalance, DepositOrigin, InFlightDeposit, QueuedDeposit, State, audit::process_event, event::EventType, mutate_state, read_state, }, utils::assert_valid_deposit_owner, @@ -90,6 +90,7 @@ pub async fn deposit_sol( account, address: deposit_address, balance, + origin: DepositOrigin::Manual, deposit_fee: None, }, ) @@ -119,6 +120,7 @@ pub(crate) fn queue_deposit( address: deposit.address, balance: deposit.balance, deposit_fee: deposit.deposit_fee, + origin: deposit.origin, }, runtime, ); diff --git a/minter/src/deposit/sweep/tests.rs b/minter/src/deposit/sweep/tests.rs index 24f8a98f5..3c62e5832 100644 --- a/minter/src/deposit/sweep/tests.rs +++ b/minter/src/deposit/sweep/tests.rs @@ -2,7 +2,7 @@ use crate::{ constants::{GET_BALANCE_CYCLES, RENT_EXEMPTION_THRESHOLD}, deposit::sweep::{deposit_sol, deposit_status}, guard::deposit_sol_guard, - state::{event::EventType, read_state}, + state::{DepositOrigin, event::EventType, read_state}, storage::with_event_iter, test_fixtures::{ AUTOMATIC_DEPOSIT_FEE, DEPOSIT_SOL_FEE, DEPOSIT_SOL_REQUIRED_CYCLES, EventsAssert, @@ -247,6 +247,7 @@ async fn assert_yields_to_the_deposit_queued_by_a_scan_during_the_read( address: scanned.address, balance: queued_deposit_of(DEPOSITOR_ACCOUNT, scanned.sweepable_amount).balance, deposit_fee: Some(AUTOMATIC_DEPOSIT_FEE), + origin: DepositOrigin::Automatic, }) .assert_no_more_events(); assert_eq!( @@ -345,6 +346,7 @@ fn queued_deposit_event(deposit_id: u64, account: Account, sweepable_amount: Lam address: deposit.address, balance: deposit.balance, deposit_fee: None, + origin: DepositOrigin::Manual, } } diff --git a/minter/src/state/audit.rs b/minter/src/state/audit.rs index e22d9bf6b..7c97c5d36 100644 --- a/minter/src/state/audit.rs +++ b/minter/src/state/audit.rs @@ -100,6 +100,7 @@ fn apply_state_transition(state: &mut State, payload: &EventType, timestamp: u64 address, balance, deposit_fee, + origin, } => { state.process_queued_deposit( *deposit_id, @@ -107,6 +108,7 @@ fn apply_state_transition(state: &mut State, payload: &EventType, timestamp: u64 account: *account, address: *address, balance: *balance, + origin: *origin, deposit_fee: *deposit_fee, }, timestamp, diff --git a/minter/src/state/deposits/mod.rs b/minter/src/state/deposits/mod.rs index 701dc6af9..7f375ef5e 100644 --- a/minter/src/state/deposits/mod.rs +++ b/minter/src/state/deposits/mod.rs @@ -8,6 +8,7 @@ use crate::{ }; use cksol_types::{DepositSolId, DepositSolStatus}; use icrc_ledger_types::icrc1::account::Account; +use minicbor::{Decode, Encode}; use sol_rpc_types::Lamport; use solana_address::Address; use solana_signature::Signature; @@ -421,8 +422,9 @@ pub struct QueuedDeposit { pub address: Address, /// The balance of the deposit address when the deposit was queued. pub balance: DepositBalance, - /// The fee in lamports deducted from the minted amount, absent for a deposit queued by - /// `deposit_sol`. + /// The flow that queued the deposit. + pub origin: DepositOrigin, + /// The fee in lamports deducted from the minted amount, absent if none is charged. pub deposit_fee: Option, } @@ -431,12 +433,25 @@ impl QueuedDeposit { self.balance.sweepable_amount() } - /// The fee deducted from the mint, zero for a deposit queued by `deposit_sol`. + /// The fee deducted from the mint, zero if none is charged. pub fn deposit_fee(&self) -> Lamport { self.deposit_fee.unwrap_or(0) } } +/// 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. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Decode, Encode)] +#[cbor(index_only)] +pub enum DepositOrigin { + /// Queued by a user via `deposit_sol`. + #[n(0)] + Manual, + /// Queued by a balance scan of the automatic flow. + #[n(1)] + Automatic, +} + impl From for Lamport { fn from(balance: DepositBalance) -> Self { balance.0 diff --git a/minter/src/state/deposits/sweeps/mod.rs b/minter/src/state/deposits/sweeps/mod.rs index fc8f21646..e92c76573 100644 --- a/minter/src/state/deposits/sweeps/mod.rs +++ b/minter/src/state/deposits/sweeps/mod.rs @@ -1,7 +1,7 @@ use crate::{ constants::{FEE_PER_SIGNATURE, RENT_EXEMPTION_THRESHOLD}, state::{ - QueuedDeposit, + DepositOrigin, QueuedDeposit, event::{CreditedDeposit, VersionedMessage}, }, }; @@ -355,6 +355,15 @@ impl Sweep { pub fn deposit_fees(&self) -> Lamport { self.deposits.values().map(QueuedDeposit::deposit_fee).sum() } + + /// The part of [`Self::deposit_fees`] charged for deposits queued by the automatic flow. + pub fn automatic_deposit_fees(&self) -> Lamport { + self.deposits + .values() + .filter(|deposit| deposit.origin == DepositOrigin::Automatic) + .map(QueuedDeposit::deposit_fee) + .sum() + } } /// A sweep whose outcome matched its plan, with what the minter received and mints for it. diff --git a/minter/src/state/deposits/sweeps/tests.rs b/minter/src/state/deposits/sweeps/tests.rs index 9e292c0ae..6e2d39200 100644 --- a/minter/src/state/deposits/sweeps/tests.rs +++ b/minter/src/state/deposits/sweeps/tests.rs @@ -102,7 +102,7 @@ mod plan { planned_sweep, queued_deposit_of, }; use crate::{ - state::event::CreditedDeposit, + state::{QueuedDeposit, event::CreditedDeposit}, test_fixtures::{ arb::{arb_address, arb_sweep_deposits}, automatic_deposit_of, @@ -189,6 +189,23 @@ mod plan { assert_eq!(sweep.deposit_fees(), 0); } + #[test] + fn should_count_only_the_fees_of_automatic_deposits_as_automatic() { + let sweep = planned_sweep([ + ( + 0, + QueuedDeposit { + deposit_fee: Some(1_000), + ..queued_deposit_of(account(1), 1_000_000) + }, + ), + (1, automatic_deposit_of(account(2), 2_000_000, 2_000)), + ]); + + assert_eq!(sweep.deposit_fees(), 3_000); + assert_eq!(sweep.automatic_deposit_fees(), 2_000); + } + #[test] fn should_have_no_mints_when_the_fees_exceed_the_sweepable_amount() { let deposit_fee = 1_000_000; @@ -334,7 +351,10 @@ mod settle { SweepMismatch, SweepSettlementError, TransactionError, UiTransactionError, UnreadableOutcome, account, devnet_sweep, planned_sweep, queued_deposit_of, }; - use crate::{state::QueuedDeposit, test_fixtures::fetched}; + use crate::{ + state::{DepositOrigin, QueuedDeposit}, + test_fixtures::fetched, + }; type Deviation = fn(&mut EncodedConfirmedTransactionWithStatusMeta); @@ -386,15 +406,16 @@ mod settle { #[test] fn should_reject_a_matching_outcome_whose_fees_exceed_a_sweepable_amount() { let deposits = devnet_sweep::deposits().into_iter().map(|(id, deposit)| { - let deposit_fee = - (deposit.address == SECOND_DEPOSIT).then_some(SECOND_SWEEPABLE_AMOUNT); - ( - id, + let deposit = if deposit.address == SECOND_DEPOSIT { QueuedDeposit { - deposit_fee, + origin: DepositOrigin::Automatic, + deposit_fee: Some(SECOND_SWEEPABLE_AMOUNT), ..deposit - }, - ) + } + } else { + deposit + }; + (id, deposit) }); let fetched = fetched(devnet_sweep::outcome()); diff --git a/minter/src/state/event.rs b/minter/src/state/event.rs index a3cb5e6c5..19fa268f8 100644 --- a/minter/src/state/event.rs +++ b/minter/src/state/event.rs @@ -3,7 +3,7 @@ use crate::{ constants::FEE_PER_SIGNATURE, numeric::{LedgerBurnIndex, LedgerMintIndex}, rpc::BlockHeight, - state::DepositBalance, + state::{DepositBalance, DepositOrigin}, }; use cksol_types::DepositSolId; use cksol_types_internal::{InitArgs, UpgradeArgs}; @@ -148,9 +148,14 @@ pub enum EventType { address: Address, #[cbor(n(3), with = "cbor::deposit_balance")] balance: DepositBalance, - /// The fee deducted from the minted amount, absent for a deposit queued via `deposit_sol`. + /// The fee deducted from the minted amount, absent if none is charged. #[n(4)] deposit_fee: Option, + /// The flow that queued the deposit. Always recorded, it decodes as + /// [`DepositOrigin::Manual`] when absent: every event recorded without it predates the + /// automatic flow. + #[cbor(n(5), with = "cbor::deposit_origin", has_nil)] + origin: DepositOrigin, }, /// The minter read the amount that the finalized sweep transaction moved to its /// main account and enqueued a pending mint for each deposit of that sweep. diff --git a/minter/src/state/event/candid/mod.rs b/minter/src/state/event/candid/mod.rs index 85171ab82..78b4ee37c 100644 --- a/minter/src/state/event/candid/mod.rs +++ b/minter/src/state/event/candid/mod.rs @@ -5,7 +5,7 @@ use crate::{ numeric::{LedgerBurnIndex, LedgerMintIndex}, rpc::BlockHeight, state::{ - DepositBalance, + DepositBalance, DepositOrigin, event::{ CreditedDeposit, Event, EventType, Signer, TransactionPurpose, VersionedMessage, WithdrawalRequest, @@ -13,9 +13,9 @@ use crate::{ }, }; use cksol_types_internal::event::{ - CreditedDeposit as CandidCreditedDeposit, Event as CandidEvent, EventType as CandidEventType, - Signer as CandidSigner, TransactionPurpose as CandidTransactionPurpose, - VersionedTransactionMessage, + CreditedDeposit as CandidCreditedDeposit, DepositOrigin as CandidDepositOrigin, + Event as CandidEvent, EventType as CandidEventType, Signer as CandidSigner, + TransactionPurpose as CandidTransactionPurpose, VersionedTransactionMessage, }; use ic_ed25519::PublicKey; use sol_rpc_types::{ @@ -77,12 +77,14 @@ impl From for CandidEventType { address, balance, deposit_fee, + origin, } => CandidEventType::QueuedDeposit { deposit_id, account, address: CandidAddress::from(address), balance: Lamport::from(balance), deposit_fee, + origin: Some(CandidDepositOrigin::from(origin)), }, EventType::CreditedSweep { signature, @@ -136,6 +138,15 @@ impl From for VersionedTransactionMessage { } } +impl From for CandidDepositOrigin { + fn from(origin: DepositOrigin) -> Self { + match origin { + DepositOrigin::Manual => CandidDepositOrigin::Manual, + DepositOrigin::Automatic => CandidDepositOrigin::Automatic, + } + } +} + impl From for CandidSigner { fn from(signer: Signer) -> Self { match signer { @@ -232,6 +243,7 @@ impl TryFrom for EventType { address, balance, deposit_fee, + origin, } => EventType::QueuedDeposit { deposit_id, account, @@ -240,6 +252,7 @@ impl TryFrom for EventType { InvalidCandidEvent::DepositBalanceBelowRentExemption(balance), )?, deposit_fee, + origin: origin.map_or(DepositOrigin::Manual, DepositOrigin::from), }, CandidEventType::CreditedSweep { signature, @@ -297,6 +310,15 @@ impl TryFrom for VersionedMessage { } } +impl From for DepositOrigin { + fn from(origin: CandidDepositOrigin) -> Self { + match origin { + CandidDepositOrigin::Manual => DepositOrigin::Manual, + CandidDepositOrigin::Automatic => DepositOrigin::Automatic, + } + } +} + impl From for Signer { fn from(signer: CandidSigner) -> Self { match signer { diff --git a/minter/src/state/event/candid/tests.rs b/minter/src/state/event/candid/tests.rs index 46371fc67..fd856bfb8 100644 --- a/minter/src/state/event/candid/tests.rs +++ b/minter/src/state/event/candid/tests.rs @@ -1,6 +1,9 @@ use crate::{ constants::RENT_EXEMPTION_THRESHOLD, - state::event::{Event, EventType, candid::InvalidCandidEvent}, + state::{ + DepositBalance, DepositOrigin, + event::{Event, EventType, candid::InvalidCandidEvent}, + }, test_fixtures::arb::{arb_account, arb_address, arb_event, arb_signature}, }; use assert_matches::assert_matches; @@ -51,6 +54,7 @@ proptest! { address: address.into(), balance, deposit_fee, + origin: None, }; prop_assert_eq!( EventType::try_from(payload).unwrap_err(), @@ -58,6 +62,34 @@ proptest! { ); } + #[test] + fn should_convert_a_queued_deposit_without_origin_to_a_manual_deposit( + account in arb_account(), + address in arb_address(), + deposit_fee in option::of(any::()), + ) { + let balance = RENT_EXEMPTION_THRESHOLD + 1; + let payload = CandidEventType::QueuedDeposit { + deposit_id: 0, + account, + address: address.into(), + balance, + deposit_fee, + origin: None, + }; + prop_assert_eq!( + EventType::try_from(payload).unwrap(), + EventType::QueuedDeposit { + deposit_id: 0, + account, + address, + balance: DepositBalance::new(balance).unwrap(), + deposit_fee, + origin: DepositOrigin::Manual, + } + ); + } + #[test] fn should_reject_a_chain_code_that_is_not_32_bytes( len in (0_usize..64).prop_filter("32 bytes is a valid chain code", |len| *len != 32) diff --git a/minter/src/state/event/cbor/mod.rs b/minter/src/state/event/cbor/mod.rs index 4a3101966..910dce643 100644 --- a/minter/src/state/event/cbor/mod.rs +++ b/minter/src/state/event/cbor/mod.rs @@ -148,6 +148,40 @@ pub mod deposit_balance { } } +/// Decodes a missing origin as [`DepositOrigin::Manual`]: the events recorded without one +/// predate the automatic flow, so their deposits were all queued by `deposit_sol`. +/// +/// [`DepositOrigin::Manual`]: crate::state::DepositOrigin::Manual +pub mod deposit_origin { + use crate::state::DepositOrigin; + use minicbor::{ + Decode, Encode, + decode::{Decoder, Error}, + encode::{Encoder, Write}, + }; + + pub fn decode(d: &mut Decoder<'_>, ctx: &mut Ctx) -> Result { + DepositOrigin::decode(d, ctx) + } + + pub fn encode( + v: &DepositOrigin, + e: &mut Encoder, + ctx: &mut Ctx, + ) -> Result<(), minicbor::encode::Error> { + v.encode(e, ctx) + } + + pub fn nil() -> Option { + Some(DepositOrigin::Manual) + } + + /// Never skips the origin, so that every new event records it, `Manual` included. + pub fn is_nil(_v: &DepositOrigin) -> bool { + false + } +} + pub mod id_vec { use minicbor::{ decode::{Decoder, Error}, diff --git a/minter/src/state/event/cbor/tests.rs b/minter/src/state/event/cbor/tests.rs index 21a7623fc..7d23d97e6 100644 --- a/minter/src/state/event/cbor/tests.rs +++ b/minter/src/state/event/cbor/tests.rs @@ -215,3 +215,88 @@ mod versioned_message_tests { minicbor::decode(bytes).unwrap() } } + +mod deposit_origin_tests { + use super::*; + use crate::{ + state::{DepositOrigin, event::EventType}, + test_fixtures::{account, deposit_address}, + }; + use icrc_ledger_types::icrc1::account::Account; + use minicbor::Encode; + use sol_rpc_types::Lamport; + use solana_address::Address; + + /// The `QueuedDeposit` event as recorded before it had an origin. + #[derive(Encode)] + enum LegacyEventType { + #[n(8)] + QueuedDeposit { + #[n(0)] + deposit_id: u64, + #[n(1)] + account: Account, + #[cbor(n(2), with = "cbor::address")] + address: Address, + #[cbor(n(3), with = "cbor::deposit_balance")] + balance: DepositBalance, + #[n(4)] + deposit_fee: Option, + }, + } + + #[test] + fn should_decode_a_queued_deposit_without_origin_as_manual() { + let balance = DepositBalance::new(RENT_EXEMPTION_THRESHOLD + 1_000_000).unwrap(); + let legacy = encode(&LegacyEventType::QueuedDeposit { + deposit_id: 3, + account: account(1), + address: deposit_address(account(1)), + balance, + deposit_fee: None, + }); + + let decoded: EventType = minicbor::decode(&legacy).unwrap(); + + assert_eq!( + decoded, + EventType::QueuedDeposit { + deposit_id: 3, + account: account(1), + address: deposit_address(account(1)), + balance, + deposit_fee: None, + origin: DepositOrigin::Manual, + } + ); + } + + #[test] + fn should_record_the_origin_of_a_manual_deposit() { + let balance = DepositBalance::new(RENT_EXEMPTION_THRESHOLD + 1_000_000).unwrap(); + let legacy = encode(&LegacyEventType::QueuedDeposit { + deposit_id: 3, + account: account(1), + address: deposit_address(account(1)), + balance, + deposit_fee: None, + }); + + let manual = encode(&EventType::QueuedDeposit { + deposit_id: 3, + account: account(1), + address: deposit_address(account(1)), + balance, + deposit_fee: None, + origin: DepositOrigin::Manual, + }); + + assert!(manual.len() > legacy.len()); + } + + fn encode>(value: &T) -> Vec { + let mut buf = Vec::new(); + minicbor::encode(value, &mut buf).unwrap(); + buf + } +} diff --git a/minter/src/state/mod.rs b/minter/src/state/mod.rs index 11a992ac3..a06a3a901 100644 --- a/minter/src/state/mod.rs +++ b/minter/src/state/mod.rs @@ -43,9 +43,9 @@ mod nonce_pool; mod watchlist; pub use deposits::{ - DepositBalance, Deposits, InFlightDeposit, MintedSweep, PendingMint, QuarantineCause, - QuarantinedDeposit, QueuedDeposit, SettledSweep, Sweep, SweepMismatch, SweepRecoveryError, - SweepSettlementError, Sweeps, SweptDeposit, Transfer, UnreadableOutcome, + DepositBalance, DepositOrigin, Deposits, InFlightDeposit, MintedSweep, PendingMint, + QuarantineCause, QuarantinedDeposit, QueuedDeposit, SettledSweep, Sweep, SweepMismatch, + SweepRecoveryError, SweepSettlementError, Sweeps, SweptDeposit, Transfer, UnreadableOutcome, }; pub use nonce_pool::{DurableNoncePool, NoncePoolError, NonceRead}; pub use watchlist::{ @@ -483,7 +483,7 @@ impl State { .is_some_and(|total| total <= amount_received), "Attempted to credit sweep {signature} with mints of {amount_to_mint} lamports and deposit fees of {deposit_fees} lamports exceeding the {amount_received} lamports received" ); - self.collected_automatic_deposit_fees += deposit_fees; + self.collected_automatic_deposit_fees += sweep.automatic_deposit_fees(); self.balance += amount_received; } diff --git a/minter/src/state/tests.rs b/minter/src/state/tests.rs index 4ab4c0730..309fa0aff 100644 --- a/minter/src/state/tests.rs +++ b/minter/src/state/tests.rs @@ -111,6 +111,7 @@ mod queued_deposits { address: queued_deposit(deposit_id).address, balance: queued_deposit(deposit_id).balance, deposit_fee: None, + origin: DepositOrigin::Manual, }, }; let init = Event { @@ -176,7 +177,7 @@ mod swept_deposits { DEFAULT_BLOCK_HEIGHT, REGISTERED_AT, WATCH_WINDOW_NANOS, at, events::{credit_sweep, quarantine_sweep, queue_deposits, submit_sweep}, flow::deposit::{CreditedSweepFlow, DepositFlow, SweepFlow}, - replayed_recorded_log, sweep_message, watch, + queued_deposit_of, replayed_recorded_log, sweep_message, watch, }, }; @@ -533,6 +534,39 @@ mod swept_deposits { read_state(|live_state| assert_eq!(&replayed, live_state)); } + #[test] + fn should_not_collect_the_fee_of_a_manual_deposit_as_an_automatic_deposit_fee() { + init_state(); + init_schnorr_master_key(); + let manual = DepositFlow::queue_deposit(QueuedDeposit { + deposit_fee: Some(AUTOMATIC_DEPOSIT_FEE), + ..queued_deposit_of(account(1), 10_000_000) + }); + let automatic = + DepositFlow::queue_automatic(account(2), 20_000_000, AUTOMATIC_DEPOSIT_FEE / 2); + + let credited = SweepFlow::of([manual, automatic]) + .submit(signature(SWEEP_SIGNATURE_INDEX)) + .succeed() + .credit(); + + let [manual_mint, _] = credited.pending_mints(); + assert_eq!( + manual_mint.amount_to_mint, + 10_000_000 - FEE_PER_SIGNATURE - AUTOMATIC_DEPOSIT_FEE + ); + read_state(|s| { + assert_eq!( + s.collected_automatic_deposit_fees(), + AUTOMATIC_DEPOSIT_FEE / 2 + ); + assert_eq!(s.balance(), credited.amount_received); + }); + + let replayed = replayed_recorded_log(); + read_state(|live_state| assert_eq!(&replayed, live_state)); + } + fn credit_sweep_of_two_deposits(credited_at: u64) -> CreditedSweepFlow { init_schnorr_master_key(); let deposits = [1, 2].map(|i| DepositFlow::queue(account(i), 1_000_000 * i as u64)); @@ -1782,6 +1816,7 @@ mod withdrawal_transactions { address: funding_deposit.address, balance: funding_deposit.balance, deposit_fee: None, + origin: DepositOrigin::Manual, }, EventType::SubmittedTransaction { signature: signature(9), diff --git a/minter/src/state/watchlist/mod.rs b/minter/src/state/watchlist/mod.rs index 79e847019..a0e6010ff 100644 --- a/minter/src/state/watchlist/mod.rs +++ b/minter/src/state/watchlist/mod.rs @@ -1,5 +1,5 @@ use crate::{ - state::QueuedDeposit, + state::{DepositOrigin, QueuedDeposit}, utils::timed_sized_map::{Entry, InsertError, TimedSizedMap, Timestamp}, }; use icrc_ledger_types::icrc1::account::Account; @@ -148,7 +148,7 @@ impl Watchlist { now: Timestamp, dropped: &QueuedDeposit, ) -> Result { - if dropped.deposit_fee.is_none() { + if dropped.origin == DepositOrigin::Manual { return Err(RearmError::ManualDeposit); } self.register(now, dropped.account, dropped.address) diff --git a/minter/src/state/watchlist/tests.rs b/minter/src/state/watchlist/tests.rs index c5d0bd7dc..579f194a7 100644 --- a/minter/src/state/watchlist/tests.rs +++ b/minter/src/state/watchlist/tests.rs @@ -1,6 +1,6 @@ use crate::{ state::{ - QueuedDeposit, + DepositOrigin, QueuedDeposit, watchlist::{ MAX_WATCHED_ADDRESSES, RearmError, WatchedAddress, Watchlist, WatchlistFull, scan_slots, }, @@ -238,12 +238,13 @@ fn should_not_rearm_an_account_when_full_of_live_entries() { } #[test] -fn should_not_rearm_a_manual_deposit() { +fn should_not_rearm_a_manual_deposit_even_with_a_fee() { let mut watchlist = Watchlist::default(); let manual = QueuedDeposit { - deposit_fee: None, + origin: DepositOrigin::Manual, ..dropped(account(1)) }; + assert_eq!(manual.deposit_fee, Some(AUTOMATIC_DEPOSIT_FEE)); let result = watchlist.rearm(at(REGISTERED_AT), &manual); @@ -251,6 +252,19 @@ fn should_not_rearm_a_manual_deposit() { assert!(watchlist.is_empty()); } +#[test] +fn should_rearm_an_automatic_deposit_without_a_fee() { + let mut watchlist = Watchlist::default(); + let automatic = QueuedDeposit { + deposit_fee: None, + ..dropped(account(1)) + }; + + let result = watchlist.rearm(at(REGISTERED_AT), &automatic); + + assert_eq!(result.map(|watched| watched.account), Ok(account(1))); +} + fn dropped(account: Account) -> QueuedDeposit { automatic_deposit_of(account, MINIMUM_DEPOSIT_AMOUNT, AUTOMATIC_DEPOSIT_FEE) } diff --git a/minter/src/test_fixtures/flow/deposit.rs b/minter/src/test_fixtures/flow/deposit.rs index 2ad29b7a8..0326f1468 100644 --- a/minter/src/test_fixtures/flow/deposit.rs +++ b/minter/src/test_fixtures/flow/deposit.rs @@ -37,7 +37,8 @@ impl DepositFlow { Self::queue_deposit(automatic_deposit_of(account, sweepable_amount, deposit_fee)) } - fn queue_deposit(deposit: QueuedDeposit) -> QueuedDepositFlow { + /// Queues `deposit` under the next deposit id of the state. + pub fn queue_deposit(deposit: QueuedDeposit) -> QueuedDepositFlow { let deposit_id = read_state(|state| state.deposits().next_id()); queue(deposit_id, deposit); QueuedDepositFlow { diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index 10d162283..18643b49c 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -4,7 +4,7 @@ use crate::{ rpc::{BlockHeight, FetchedTransaction}, sol_transfer::build_batch_withdrawal_message, state::{ - DepositBalance, QueuedDeposit, SchnorrPublicKey, State, Sweep, WATCH_WINDOW, + DepositBalance, DepositOrigin, QueuedDeposit, SchnorrPublicKey, State, Sweep, WATCH_WINDOW, WatchedAddress, Watchlist, audit::replay_events, event::{Event, EventType, VersionedMessage}, @@ -356,6 +356,7 @@ pub fn queued_deposit_of(account: Account, sweepable_amount: Lamport) -> QueuedD address: deposit_address(account), balance: DepositBalance::new(sweepable_amount + RENT_EXEMPTION_THRESHOLD) .expect("BUG: the balance covers the rent exemption threshold"), + origin: DepositOrigin::Manual, deposit_fee: None, } } @@ -368,6 +369,7 @@ pub fn automatic_deposit_of( deposit_fee: Lamport, ) -> QueuedDeposit { QueuedDeposit { + origin: DepositOrigin::Automatic, deposit_fee: Some(deposit_fee), ..queued_deposit_of(account, sweepable_amount) } @@ -537,7 +539,9 @@ pub fn fetched(outcome: EncodedConfirmedTransactionWithStatusMeta) -> FetchedTra /// from that response. pub mod devnet_sweep { use super::account; - use crate::state::{DepositBalance, QueuedDeposit, Sweep, event::CreditedDeposit}; + use crate::state::{ + DepositBalance, DepositOrigin, QueuedDeposit, Sweep, event::CreditedDeposit, + }; use base64::{Engine, engine::general_purpose::STANDARD}; use cksol_types::DepositSolId; use icrc_ledger_types::icrc1::account::Account; @@ -623,6 +627,7 @@ pub mod devnet_sweep { address: crate::address::account_address(&master_key(), &account), balance: DepositBalance::new(balance) .expect("BUG: the balance covers the rent exemption threshold"), + origin: DepositOrigin::Manual, deposit_fee: None, }, ) @@ -640,6 +645,7 @@ pub mod devnet_sweep { sweepable_amount + crate::constants::RENT_EXEMPTION_THRESHOLD, ) .expect("BUG: the balance covers the rent exemption threshold"), + origin: DepositOrigin::Manual, deposit_fee: None, } } @@ -746,6 +752,7 @@ pub mod devnet_sweep { address, balance: DepositBalance::new(balance) .expect("BUG: the balance covers the rent exemption threshold"), + origin: DepositOrigin::Manual, deposit_fee: None, }, ) @@ -962,6 +969,7 @@ pub mod events { address: deposit.address, balance: deposit.balance, deposit_fee: deposit.deposit_fee, + origin: deposit.origin, }, &runtime(), ) @@ -1342,7 +1350,7 @@ pub mod arb { rpc::BlockHeight, sol_transfer::MAX_SIGNATURES, state::{ - DepositBalance, QueuedDeposit, + DepositBalance, DepositOrigin, QueuedDeposit, event::{ CreditedDeposit, Event, EventType, Signer, TransactionPurpose, WithdrawalRequest, }, @@ -1408,15 +1416,23 @@ pub mod arb { arb_account(), arb_address(), MIN_BALANCE..=MAX_BALANCE, + arb_deposit_origin(), prop::option::of(arb_deposit_fee()), ) - .prop_map(|(account, address, balance, deposit_fee)| QueuedDeposit { - account, - address, - balance: DepositBalance::new(balance) - .expect("BUG: the balance covers the rent exemption threshold"), - deposit_fee, - }) + .prop_map( + |(account, address, balance, origin, deposit_fee)| QueuedDeposit { + account, + address, + balance: DepositBalance::new(balance) + .expect("BUG: the balance covers the rent exemption threshold"), + origin, + deposit_fee, + }, + ) + } + + pub fn arb_deposit_origin() -> impl Strategy { + prop_oneof![Just(DepositOrigin::Manual), Just(DepositOrigin::Automatic)] } /// A deposit fee that fits in the sweepable amount of [`arb_queued_deposit`] @@ -1627,16 +1643,20 @@ pub mod arb { arb_address(), arb_deposit_balance(), prop::option::of(arb_deposit_fee()), + arb_deposit_origin(), ) - .prop_map(|(deposit_id, account, address, balance, deposit_fee)| { - EventType::QueuedDeposit { - deposit_id, - account, - address, - balance, - deposit_fee, + .prop_map( + |(deposit_id, account, address, balance, deposit_fee, origin)| { + EventType::QueuedDeposit { + deposit_id, + account, + address, + balance, + deposit_fee, + origin, + } } - },), + ), ( arb_signature(), any::(),