refactor: extract shared deposit validation from manual flow

minter/src/constants.rs

@@ -6,6 +6,9 @@ pub const MAX_CONCURRENT_RPC_CALLS: usize = 10;
 /// https://docs.internetcomputer.org/references/ic-interface-spec#ic-http_request
 pub const MAX_HTTP_OUTCALL_RESPONSE_BYTES: u64 = 2_000_000;
 
+/// Cycles to attach for `getTransaction` RPC calls.
+pub const GET_TRANSACTION_CYCLES: u128 = 50_000_000_000;
+
 /// Cycles to attach for `getSignatureStatuses` RPC calls.
 pub const GET_SIGNATURE_STATUSES_CYCLES: u128 = 1_000_000_000_000;
 

minter/src/deposit/manual/mod.rs

@@ -1,10 +1,9 @@
 use crate::{
-    address::{account_address, lazy_get_schnorr_master_key},
+    constants::GET_TRANSACTION_CYCLES,
     cycles::{charge_caller_cycles, check_caller_available_cycles},
-    deposit::get_deposit_amount_to_address,
+    deposit::fetch_and_validate_deposit,
     guard::process_deposit_guard,
     ledger::mint,
-    rpc::get_transaction,
     runtime::CanisterRuntime,
     state::{
         Deposit,
@@ -35,7 +34,7 @@ pub async fn process_deposit<R: CanisterRuntime>(
         deposit_amount,
         amount_to_mint,
     } = match read_state(|state| state.deposit_status(&deposit_id)) {
-        None => try_accept_deposit(&runtime, account, signature, deposit_id).await?,
+        None => try_accept_deposit(&runtime, account, signature).await?,
         Some(DepositStatus::Processing {
             deposit_amount,
             amount_to_mint,
@@ -70,57 +69,29 @@ async fn try_accept_deposit<R: CanisterRuntime>(
     runtime: &R,
     account: Account,
     signature: Signature,
-    deposit_id: DepositId,
 ) -> Result<Deposit, ProcessDepositError> {
-    let (cycles_to_attach, deposit_consolidation_fee) = read_state(|state| {
+    let (cycles_to_attach, deposit_consolidation_fee, fee) = read_state(|state| {
         (
             state.process_deposit_required_cycles(),
             state.deposit_consolidation_fee(),
+            state.manual_deposit_fee(),
         )
     });
     check_caller_available_cycles(runtime, cycles_to_attach)?;
 
-    // Reserve the consolidation fee and forward the rest to the HTTP outcall
-    let cycles_for_rpc = cycles_to_attach.saturating_sub(deposit_consolidation_fee);
-    let maybe_transaction = get_transaction(runtime, signature, cycles_for_rpc)
-        .await
-        .map_err(|e| {
-            log!(
-                Priority::Info,
-                "Error fetching transaction for deposit {deposit_id:?}: {e}"
-            );
-            ProcessDepositError::from(e)
-        })?;
+    let result = fetch_and_validate_deposit(runtime, account, signature, fee).await;
 
-    // Charge the actual RPC cost plus the consolidation fee
-    let rpc_cost = cycles_for_rpc.saturating_sub(runtime.msg_cycles_refunded());
-    charge_caller_cycles(runtime, rpc_cost + deposit_consolidation_fee);
+    // Always charge for the RPC call; additionally charge the consolidation fee if a deposit is found
+    let rpc_cost = GET_TRANSACTION_CYCLES.saturating_sub(runtime.msg_cycles_refunded());
+    let cycles_to_charge = rpc_cost
+        + if result.is_ok() {
+            deposit_consolidation_fee
+        } else {
+            0
+        };
+    charge_caller_cycles(runtime, cycles_to_charge);
 
-    let transaction = match maybe_transaction {
-        Some(transaction) => Ok(transaction),
-        None => Err(ProcessDepositError::TransactionNotFound),
-    }?;
-
-    let master_key = lazy_get_schnorr_master_key(runtime).await;
-    let deposit_address = account_address(&master_key, &account);
-    let deposit_amount =
-        get_deposit_amount_to_address(transaction, deposit_address).map_err(|e| {
-            log!(
-                Priority::Info,
-                "Error parsing deposit transaction with signature {signature}: {e}"
-            );
-            ProcessDepositError::InvalidDepositTransaction(e.to_string())
-        })?;
-    let minimum_deposit_amount = read_state(|state| state.minimum_deposit_amount());
-    if deposit_amount < minimum_deposit_amount {
-        return Err(ProcessDepositError::ValueTooSmall {
-            minimum_deposit_amount,
-            deposit_amount,
-        });
-    }
-    let amount_to_mint = deposit_amount
-        .checked_sub(read_state(|state| state.manual_deposit_fee()))
-        .expect("BUG: deposit amount is less than manual deposit fee");
+    let (deposit_id, deposit_amount, amount_to_mint) = result?;
 
     mutate_state(|state| {
         process_event(

minter/src/deposit/manual/tests.rs

@@ -1,4 +1,5 @@
 use crate::{
+    constants::GET_TRANSACTION_CYCLES,
     deposit::manual::process_deposit,
     state::event::{DepositId, EventType},
     storage::reset_events,
@@ -64,7 +65,8 @@ async fn should_return_error_if_get_transaction_fails() {
     init_state();
     init_schnorr_master_key();
 
-    let runtime = runtime_with_time_and_cycles().add_stub_error(IcError::CallPerformFailed);
+    let runtime =
+        runtime_with_time_and_cycles_no_deposit().add_stub_error(IcError::CallPerformFailed);
 
     let result = process_deposit(
         runtime,
@@ -85,7 +87,7 @@ async fn should_return_error_if_transaction_not_found() {
     init_state();
     init_schnorr_master_key();
 
-    let runtime = runtime_with_time_and_cycles()
+    let runtime = runtime_with_time_and_cycles_no_deposit()
         .add_stub_response(GetTransactionResult::Consistent(Ok(None)));
 
     let result = process_deposit(
@@ -107,7 +109,8 @@ async fn should_return_error_if_transaction_not_valid_deposit() {
     let get_transaction_response = GetTransactionResult::Consistent(Ok(Some(
         deposit_transaction_to_wrong_address().try_into().unwrap(),
     )));
-    let runtime = runtime_with_time_and_cycles().add_stub_response(get_transaction_response);
+    let runtime =
+        runtime_with_time_and_cycles_no_deposit().add_stub_response(get_transaction_response);
 
     let result = process_deposit(
         runtime,
@@ -135,7 +138,8 @@ async fn should_fail_if_deposit_amount_is_below_minimum() {
     let get_transaction_response = GetTransactionResult::Consistent(Ok(Some(
         legacy_deposit_transaction().try_into().unwrap(),
     )));
-    let runtime = runtime_with_time_and_cycles().add_stub_response(get_transaction_response);
+    let runtime =
+        runtime_with_time_and_cycles_no_deposit().add_stub_response(get_transaction_response);
 
     let result = process_deposit(
         runtime,
@@ -443,14 +447,21 @@ async fn should_allow_deposits_to_multiple_accounts_with_single_transaction() {
 }
 
 fn runtime_with_time_and_cycles() -> TestCanisterRuntime {
-    // Cycles forwarded to the RPC call = total - consolidation fee
-    let cycles_for_rpc = PROCESS_DEPOSIT_REQUIRED_CYCLES - DEPOSIT_CONSOLIDATION_FEE;
-    // Simulate the RPC canister refunding most of the forwarded cycles
-    let refunded: u128 = cycles_for_rpc - 100_000_000_000;
-    let rpc_cost = cycles_for_rpc - refunded;
+    let rpc_cost = 25_000_000_000u128;
+    let refunded = GET_TRANSACTION_CYCLES - rpc_cost;
     TestCanisterRuntime::new()
         .with_increasing_time()
         .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
+        .add_msg_cycles_refunded(refunded)
         .add_msg_cycles_accept(rpc_cost + DEPOSIT_CONSOLIDATION_FEE)
+}
+
+fn runtime_with_time_and_cycles_no_deposit() -> TestCanisterRuntime {
+    let rpc_cost = 25_000_000_000u128;
+    let refunded = GET_TRANSACTION_CYCLES - rpc_cost;
+    TestCanisterRuntime::new()
+        .with_increasing_time()
+        .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
         .add_msg_cycles_refunded(refunded)
+        .add_msg_cycles_accept(rpc_cost)
 }

minter/src/deposit/mod.rs

@@ -1,5 +1,16 @@
+use crate::{
+    address::{account_address, lazy_get_schnorr_master_key},
+    rpc::get_transaction,
+    runtime::CanisterRuntime,
+    state::{event::DepositId, read_state},
+};
+use canlog::log;
+use cksol_types::ProcessDepositError;
+use cksol_types_internal::log::Priority;
+use icrc_ledger_types::icrc1::account::Account;
 use sol_rpc_types::Lamport;
 use solana_address::Address;
+use solana_signature::Signature;
 use solana_transaction_status_client_types::{
     EncodedConfirmedTransactionWithStatusMeta, UiTransactionError,
 };
@@ -11,6 +22,53 @@ mod tests;
 pub mod automatic;
 pub mod manual;
 
+pub async fn fetch_and_validate_deposit<R: CanisterRuntime>(
+    runtime: &R,
+    account: Account,
+    signature: Signature,
+    fee: Lamport,
+) -> Result<(DepositId, Lamport, Lamport), ProcessDepositError> {
+    let deposit_id = DepositId { account, signature };
+    let master_key = lazy_get_schnorr_master_key(runtime).await;
+    let deposit_address = account_address(&master_key, &account);
+
+    let maybe_transaction = get_transaction(runtime, signature).await.map_err(|e| {
+        log!(
+            Priority::Info,
+            "Error fetching transaction for deposit {deposit_id:?}: {e}"
+        );
+        ProcessDepositError::from(e)
+    })?;
+
+    let transaction = match maybe_transaction {
+        Some(t) => t,
+        None => return Err(ProcessDepositError::TransactionNotFound),
+    };
+
+    let deposit_amount =
+        get_deposit_amount_to_address(transaction, deposit_address).map_err(|e| {
+            log!(
+                Priority::Info,
+                "Error parsing deposit transaction with signature {signature}: {e}"
+            );
+            ProcessDepositError::InvalidDepositTransaction(e.to_string())
+        })?;
+
+    let minimum_deposit_amount = read_state(|s| s.minimum_deposit_amount());
+    if deposit_amount < minimum_deposit_amount {
+        return Err(ProcessDepositError::ValueTooSmall {
+            minimum_deposit_amount,
+            deposit_amount,
+        });
+    }
+
+    let amount_to_mint = deposit_amount
+        .checked_sub(fee)
+        .expect("BUG: deposit amount is less than fee");
+
+    Ok((deposit_id, deposit_amount, amount_to_mint))
+}
+
 pub fn get_deposit_amount_to_address(
     transaction: EncodedConfirmedTransactionWithStatusMeta,
     deposit_address: Address,

minter/src/rpc/mod.rs

@@ -1,5 +1,7 @@
 use crate::{
-    constants::{GET_SIGNATURE_STATUSES_CYCLES, MAX_HTTP_OUTCALL_RESPONSE_BYTES},
+    constants::{
+        GET_SIGNATURE_STATUSES_CYCLES, GET_TRANSACTION_CYCLES, MAX_HTTP_OUTCALL_RESPONSE_BYTES,
+    },
     runtime::CanisterRuntime,
     state::read_state,
 };
@@ -22,15 +24,14 @@ mod tests;
 pub async fn get_transaction<R: CanisterRuntime>(
     runtime: &R,
     signature: Signature,
-    cycles_to_attach: u128,
 ) -> Result<Option<EncodedConfirmedTransactionWithStatusMeta>, GetTransactionError> {
     let result = read_state(|state| state.sol_rpc_client(runtime.inter_canister_call_runtime()))
         .get_transaction(signature)
         .with_encoding(GetTransactionEncoding::Base64)
         .with_commitment(CommitmentLevel::Finalized)
         .with_max_supported_transaction_version(0)
         .with_response_size_estimate(MAX_HTTP_OUTCALL_RESPONSE_BYTES)
-        .with_cycles(cycles_to_attach)
+        .with_cycles(GET_TRANSACTION_CYCLES)
         .try_send()
         .await;
     match result? {

minter/src/rpc/tests.rs

@@ -5,7 +5,7 @@ use crate::{
         get_transaction, submit_transaction,
     },
     test_fixtures::{
-        PROCESS_DEPOSIT_REQUIRED_CYCLES, confirmed_block,
+        confirmed_block,
         deposit::{legacy_deposit_transaction, legacy_deposit_transaction_signature},
         init_state,
         runtime::TestCanisterRuntime,
@@ -28,16 +28,9 @@ mod get_transaction_tests {
     async fn should_fail_if_get_transaction_fails() {
         init_state();
 
-        let runtime = TestCanisterRuntime::new()
-            .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
-            .add_stub_error(IcError::CallPerformFailed);
+        let runtime = TestCanisterRuntime::new().add_stub_error(IcError::CallPerformFailed);
 
-        let result = get_transaction(
-            &runtime,
-            legacy_deposit_transaction_signature(),
-            PROCESS_DEPOSIT_REQUIRED_CYCLES,
-        )
-        .await;
+        let result = get_transaction(&runtime, legacy_deposit_transaction_signature()).await;
 
         assert_eq!(
             result,
@@ -56,15 +49,9 @@ mod get_transaction_tests {
         });
 
         let runtime = TestCanisterRuntime::new()
-            .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
             .add_stub_response(MultiRpcResult::Consistent(Err(rpc_error.clone())));
 
-        let result = get_transaction(
-            &runtime,
-            legacy_deposit_transaction_signature(),
-            PROCESS_DEPOSIT_REQUIRED_CYCLES,
-        )
-        .await;
+        let result = get_transaction(&runtime, legacy_deposit_transaction_signature()).await;
 
         assert_eq!(result, Err(GetTransactionError::RpcError(rpc_error)));
     }
@@ -84,16 +71,10 @@ mod get_transaction_tests {
             ),
         ];
 
-        let runtime = TestCanisterRuntime::new()
-            .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
-            .add_stub_response(MultiRpcResult::Inconsistent(results));
+        let runtime =
+            TestCanisterRuntime::new().add_stub_response(MultiRpcResult::Inconsistent(results));
 
-        let result = get_transaction(
-            &runtime,
-            legacy_deposit_transaction_signature(),
-            PROCESS_DEPOSIT_REQUIRED_CYCLES,
-        )
-        .await;
+        let result = get_transaction(&runtime, legacy_deposit_transaction_signature()).await;
 
         assert_eq!(result, Err(GetTransactionError::InconsistentRpcResults));
     }
@@ -102,16 +83,10 @@ mod get_transaction_tests {
     async fn should_return_empty_if_transaction_not_found() {
         init_state();
 
-        let runtime = TestCanisterRuntime::new()
-            .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
-            .add_stub_response(MultiRpcResult::Consistent(Ok(None)));
+        let runtime =
+            TestCanisterRuntime::new().add_stub_response(MultiRpcResult::Consistent(Ok(None)));
 
-        let result = get_transaction(
-            &runtime,
-            legacy_deposit_transaction_signature(),
-            PROCESS_DEPOSIT_REQUIRED_CYCLES,
-        )
-        .await;
+        let result = get_transaction(&runtime, legacy_deposit_transaction_signature()).await;
 
         assert_eq!(result, Ok(None))
     }
@@ -120,18 +95,11 @@ mod get_transaction_tests {
     async fn should_return_transaction() {
         init_state();
 
-        let runtime = TestCanisterRuntime::new()
-            .add_msg_cycles_available(PROCESS_DEPOSIT_REQUIRED_CYCLES)
-            .add_stub_response(MultiRpcResult::Consistent(Ok(Some(
-                legacy_deposit_transaction().try_into().unwrap(),
-            ))));
+        let runtime = TestCanisterRuntime::new().add_stub_response(MultiRpcResult::Consistent(Ok(
+            Some(legacy_deposit_transaction().try_into().unwrap()),
+        )));
 
-        let result = get_transaction(
-            &runtime,
-            legacy_deposit_transaction_signature(),
-            PROCESS_DEPOSIT_REQUIRED_CYCLES,
-        )
-        .await;
+        let result = get_transaction(&runtime, legacy_deposit_transaction_signature()).await;
 
         assert_eq!(result, Ok(Some(legacy_deposit_transaction())))
     }