diff --git a/integration_tests/tests/tests.rs b/integration_tests/tests/tests.rs index 55b5dbc4..1ebde00c 100644 --- a/integration_tests/tests/tests.rs +++ b/integration_tests/tests/tests.rs @@ -16,7 +16,9 @@ use cksol_types::{ UpdateBalanceArgs, UpdateBalanceError, WithdrawSolArgs, WithdrawSolError, WithdrawSolStatus, }; use cksol_types_internal::{UpgradeArgs, event::EventType, log::Priority}; -use ic_pocket_canister_runtime::{JsonRpcResponse, MockHttpOutcalls, MockHttpOutcallsBuilder}; +use ic_pocket_canister_runtime::{ + JsonRpcRequestMatcher, JsonRpcResponse, MockHttpOutcalls, MockHttpOutcallsBuilder, +}; use icrc_ledger_types::icrc1::account::Subaccount; use serde_json::json; use sol_rpc_types::{CommitmentLevel, ConsensusStrategy, GetTransactionEncoding, RpcConfig}; @@ -209,6 +211,8 @@ mod withdraw_sol_tests { use super::*; + const WITHDRAWAL_PROCESSING_DELAY: Duration = Duration::from_mins(1); + #[tokio::test] async fn should_validate_solana_address() { let setup = SetupBuilder::new().build().await; @@ -556,6 +560,76 @@ mod withdraw_sol_tests { setup.drop().await; } + + #[tokio::test] + async fn should_process_pending_withdrawals() { + const WITHDRAWAL_AMOUNT: u64 = 100_000_000; + const WITHDRAWAL_ADDRESS: &str = "E4MpwNnMWs2XtW5gVrxZvyS7fMq31QD5HvbxmwP45Tz3"; + + let setup = SetupBuilder::new() + .with_initial_ledger_balances(vec![( + DEFAULT_CALLER_ACCOUNT, + Nat::from(10 * WITHDRAWAL_AMOUNT), + )]) + .build() + .await; + + setup + .ledger() + .approve( + None, + WITHDRAWAL_AMOUNT, + Account { + owner: setup.minter_canister_id(), + subaccount: None, + }, + ) + .await; + + let WithdrawSolOk { block_index } = setup + .minter() + .withdraw_sol(WithdrawSolArgs { + from_subaccount: None, + amount: WITHDRAWAL_AMOUNT, + address: WITHDRAWAL_ADDRESS.to_string(), + }) + .await + .expect("withdraw_sol should succeed"); + + setup.advance_time(WITHDRAWAL_PROCESSING_DELAY).await; + setup + .execute_http_mocks(estimate_blockhash_http_mocks()) + .await; + + setup.minter().assert_that_events().await.satisfy(|events| { + check!(events.iter().any(|e| matches!( + e, + EventType::SentWithdrawalTransaction { + transactions, + .. + } if transactions.iter().any(|(idx, _)| *idx == block_index) + ))); + }); + + setup.drop().await; + } + + fn estimate_blockhash_http_mocks() -> MockHttpOutcalls { + let mut builder = MockHttpOutcallsBuilder::new(); + for id in 0..4u64 { + builder = builder + .given(JsonRpcRequestMatcher::with_method("getSlot").with_id(id)) + .respond_with(get_slot_response(1).with_id(id)) + } + for id in 4..8u64 { + builder = builder + .given(JsonRpcRequestMatcher::with_method("getBlock").with_id(id)) + .respond_with( + get_block_response("4sGjMW1sUnHzSxGspuhpqLDx6wiyjNtZAMdL4VZHirAn").with_id(id), + ) + } + builder.build() + } } mod update_balance_tests { diff --git a/libs/types-internal/src/event.rs b/libs/types-internal/src/event.rs index fae2d966..18b08fbb 100644 --- a/libs/types-internal/src/event.rs +++ b/libs/types-internal/src/event.rs @@ -91,6 +91,11 @@ pub enum EventType { /// and the amount consolidated from each account. deposits: Vec<(Account, Lamport)>, }, + /// A withdrawal transaction was signed and is ready to be sent to the network. + SentWithdrawalTransaction { + /// The burn block indices and corresponding transaction signatures. + transactions: Vec<(u64, Signature)>, + }, /// A previously submitted transaction was resubmitted with a new signature. ResubmittedTransaction { /// The signature of the old transaction being replaced. diff --git a/libs/types-internal/src/log.rs b/libs/types-internal/src/log.rs index 5c185e30..e38ab4b1 100644 --- a/libs/types-internal/src/log.rs +++ b/libs/types-internal/src/log.rs @@ -7,6 +7,9 @@ use std::{fmt, fmt::Formatter, str::FromStr}; /// The priority level of a log entry. #[derive(LogPriorityLevels, Serialize, Deserialize, PartialEq, Debug, Copy, Clone)] pub enum Priority { + /// Error log entries. + #[log_level(capacity = 1000, name = "ERROR")] + Error, /// Informational log entries. #[log_level(capacity = 1000, name = "INFO")] Info, @@ -26,6 +29,7 @@ impl FromStr for Priority { fn from_str(s: &str) -> Result { match s.to_lowercase().as_str() { + "error" => Ok(Priority::Error), "info" => Ok(Priority::Info), "debug" => Ok(Priority::Debug), _ => Err("could not recognize priority".to_string()), @@ -36,6 +40,7 @@ impl FromStr for Priority { impl fmt::Display for Priority { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { + Priority::Error => write!(f, "ERROR"), Priority::Info => write!(f, "INFO"), Priority::Debug => write!(f, "DEBUG"), } diff --git a/libs/types/src/lib.rs b/libs/types/src/lib.rs index c7676e36..d4b53d1f 100644 --- a/libs/types/src/lib.rs +++ b/libs/types/src/lib.rs @@ -207,10 +207,6 @@ pub enum WithdrawSolStatus { /// Withdrawal request is waiting to be processed. Pending, - /// Transaction fees were estimated and a Solana transaction was created. - /// Transaction is not signed yet. - TxCreated, - /// Solana transaction was signed and is sent to the network. TxSent(SolTransaction), diff --git a/minter/cksol-minter.did b/minter/cksol-minter.did index ec5374f9..d3c6c514 100644 --- a/minter/cksol-minter.did +++ b/minter/cksol-minter.did @@ -208,10 +208,6 @@ type WithdrawSolStatus = variant { // Withdrawal request is waiting to be processed. Pending; - // Transaction fees were estimated and a Solana transaction was created. - // Transaction is not signed yet. - TxCreated; - // Solana transaction was signed and is sent to the network. TxSent : SolTransaction; @@ -299,6 +295,11 @@ type EventType = variant { // and the amount consolidated from each account. deposits: vec record { Account; Lamport }; }; + // A withdrawal transaction was signed and is ready to be sent to the network. + SentWithdrawalTransaction : record { + // The burn block indices and corresponding transaction signatures. + transactions: vec record { nat64; Signature }; + }; // A previously submitted transaction was resubmitted with a new signature. ResubmittedTransaction : record { // The signature of the old transaction being replaced. diff --git a/minter/src/consolidate/mod.rs b/minter/src/consolidate/mod.rs index 4e6b8951..b3a9924d 100644 --- a/minter/src/consolidate/mod.rs +++ b/minter/src/consolidate/mod.rs @@ -1,4 +1,5 @@ use crate::{ + address::{derivation_path, derive_public_key, lazy_get_schnorr_master_key}, guard::TimerGuard, runtime::CanisterRuntime, sol_transfer::{CreateTransferError, MAX_SIGNATURES, create_signed_transfer_transaction}, @@ -9,6 +10,7 @@ use canlog::log; use cksol_types_internal::log::Priority; use icrc_ledger_types::icrc1::account::Account; use sol_rpc_types::{Lamport, Slot}; +use solana_address::Address; use solana_hash::Hash; use solana_signature::Signature; use std::time::Duration; @@ -101,10 +103,15 @@ async fn submit_consolidation_transaction( owner: runtime.canister_self(), subaccount: None, }; + let master_key = lazy_get_schnorr_master_key().await; + let minter_address = Address::from( + derive_public_key(&master_key, derivation_path(&minter_account)).serialize_raw(), + ); + let (transaction, signers) = create_signed_transfer_transaction( minter_account, &funds_to_consolidate, - minter_account, + minter_address, recent_blockhash, &runtime.signer(), ) diff --git a/minter/src/main.rs b/minter/src/main.rs index 0d4e047a..c57e5a27 100644 --- a/minter/src/main.rs +++ b/minter/src/main.rs @@ -2,6 +2,7 @@ use candid::Principal; use canlog::{Log, Sort}; use cksol_minter::consolidate::{DEPOSIT_CONSOLIDATION_DELAY, consolidate_deposits}; use cksol_minter::monitor::{MONITOR_SUBMITTED_TRANSACTIONS_DELAY, monitor_submitted_transactions}; +use cksol_minter::withdraw_sol::{WITHDRAWAL_PROCESSING_DELAY, process_pending_withdrawals}; use cksol_minter::{ address::lazy_get_schnorr_master_key, runtime::IcCanisterRuntime, state::read_state, }; @@ -72,7 +73,7 @@ async fn withdraw_sol(args: WithdrawSolArgs) -> Result Result WithdrawSolStatus { - read_state(|s| s.withdrawal_status(block_index)) +fn withdraw_sol_status(block_index: u64) -> WithdrawSolStatus { + cksol_minter::withdraw_sol::withdraw_sol_status(block_index) } #[ic_cdk::query] @@ -153,6 +154,14 @@ fn get_events( EventType::ConsolidatedDeposits { deposits } => { event::EventType::ConsolidatedDeposits { deposits } } + EventType::SentWithdrawalTransaction { transactions } => { + event::EventType::SentWithdrawalTransaction { + transactions: transactions + .iter() + .map(|(idx, sig)| (*idx.get(), sig.into())) + .collect(), + } + } EventType::ResubmittedTransaction { old_signature, new_signature, @@ -216,10 +225,12 @@ fn http_request(request: HttpRequest) -> HttpResponse { match request.raw_query_param("priority").map(Priority::from_str) { Some(Ok(priority)) => match priority { + Priority::Error => log.push_logs(Priority::Error), Priority::Info => log.push_logs(Priority::Info), Priority::Debug => log.push_logs(Priority::Debug), }, Some(Err(_)) | None => { + log.push_logs(Priority::Error); log.push_logs(Priority::Info); log.push_logs(Priority::Debug); } @@ -277,6 +288,9 @@ fn setup_timers() { ic_cdk_timers::set_timer_interval(DEPOSIT_CONSOLIDATION_DELAY, async || { consolidate_deposits(IcCanisterRuntime::new()).await; }); + ic_cdk_timers::set_timer_interval(WITHDRAWAL_PROCESSING_DELAY, async || { + process_pending_withdrawals(&IcCanisterRuntime::new()).await; + }); ic_cdk_timers::set_timer_interval(MONITOR_SUBMITTED_TRANSACTIONS_DELAY, async || { monitor_submitted_transactions(IcCanisterRuntime::new()).await; }); diff --git a/minter/src/sol_transfer/mod.rs b/minter/src/sol_transfer/mod.rs index 646efc7c..4dd7f765 100644 --- a/minter/src/sol_transfer/mod.rs +++ b/minter/src/sol_transfer/mod.rs @@ -29,8 +29,8 @@ pub enum CreateTransferError { } /// Creates a signed Solana transaction that transfers lamports from -/// each minter-controlled address (identified by its account) to the -/// destination account's derived address. +/// each minter-controlled address (identified by its account) +/// to `target_address` Solana address. /// /// Returns the signed transaction and the list of signer accounts /// (in signature order: fee payer first, then sources). @@ -41,7 +41,7 @@ pub enum CreateTransferError { pub async fn create_signed_transfer_transaction( fee_payer_account: Account, sources: &[(Account, Lamport)], - destination_account: Account, + target_address: Address, recent_blockhash: Hash, signer: &impl SchnorrSigner, ) -> Result<(Transaction, Vec), CreateTransferError> { @@ -52,7 +52,6 @@ pub async fn create_signed_transfer_transaction( (derivation_path, public_key.serialize_raw().into()) }; - let (_, target_address) = derive_address(&destination_account); let (fee_payer_derivation_path, fee_payer_address) = derive_address(&fee_payer_account); let (source_derivation_paths, source_addresses): (Vec<_>, Vec<_>) = sources diff --git a/minter/src/sol_transfer/tests.rs b/minter/src/sol_transfer/tests.rs index f082b220..39926bec 100644 --- a/minter/src/sol_transfer/tests.rs +++ b/minter/src/sol_transfer/tests.rs @@ -41,7 +41,7 @@ async fn should_create_signed_transaction_with_single_source() { let (tx, signers) = create_signed_transfer_transaction( source_account, &[(source_account, amount)], - target_account, + target_address, blockhash, &signer, ) @@ -100,7 +100,7 @@ async fn should_create_signed_transaction_with_multiple_sources() { let (tx, signers) = create_signed_transfer_transaction( account_1, &[(account_1, amount), (account_2, amount)], - target_account, + derive_address(&target_account), blockhash, &signer, ) @@ -155,7 +155,7 @@ async fn should_fail_when_signing_is_rejected() { let result = create_signed_transfer_transaction( source_account, &[(source_account, 500_000_000)], - target_account, + derive_address(&target_account), blockhash, &signer, ) @@ -191,7 +191,7 @@ async fn should_fail_when_second_signing_fails() { let result = create_signed_transfer_transaction( account_1, &[(account_1, 100_000_000), (account_2, 100_000_000)], - target_account, + derive_address(&target_account), blockhash, &signer, ) @@ -230,9 +230,14 @@ async fn should_fail_when_too_many_signatures() { subaccount: None, }; - let result = - create_signed_transfer_transaction(fee_payer, &sources, target_account, blockhash, &signer) - .await; + let result = create_signed_transfer_transaction( + fee_payer, + &sources, + derive_address(&target_account), + blockhash, + &signer, + ) + .await; assert!( matches!(result, Err(CreateTransferError::TooManySignatures { max: MAX_SIGNATURES, got }) if got == MAX_SIGNATURES + 1) @@ -270,9 +275,14 @@ async fn should_not_fail_for_max_signatures() { let signer = MockSchnorrSigner::with_signatures(vec![[0x11u8; 64]; MAX_SIGNATURES as usize]); - let result = - create_signed_transfer_transaction(fee_payer, &sources, target_account, blockhash, &signer) - .await; + let result = create_signed_transfer_transaction( + fee_payer, + &sources, + derive_address(&target_account), + blockhash, + &signer, + ) + .await; assert!(result.is_ok()); } @@ -309,7 +319,7 @@ async fn should_create_signed_transaction_with_fee_payer() { let (tx, signers) = create_signed_transfer_transaction( fee_payer_account, &sources, - target_account, + derive_address(&target_account), blockhash, &signer, ) diff --git a/minter/src/state/audit.rs b/minter/src/state/audit.rs index c2b4924a..362ecac5 100644 --- a/minter/src/state/audit.rs +++ b/minter/src/state/audit.rs @@ -52,6 +52,11 @@ fn apply_state_transition(state: &mut State, payload: &EventType) { EventType::ConsolidatedDeposits { deposits } => { state.process_consolidated_deposits(deposits); } + EventType::SentWithdrawalTransaction { transactions } => { + for (burn_block_index, signature) in transactions { + state.process_sent_withdrawal_transaction(burn_block_index, signature); + } + } EventType::ResubmittedTransaction { old_signature, new_signature, diff --git a/minter/src/state/event.rs b/minter/src/state/event.rs index a151457e..f78c1ed4 100644 --- a/minter/src/state/event.rs +++ b/minter/src/state/event.rs @@ -106,6 +106,13 @@ pub enum EventType { #[cbor(n(0), with = "cbor::signature")] signature: Signature, }, + /// A withdrawal transaction was signed and is ready to be sent to the network. + #[n(10)] + SentWithdrawalTransaction { + /// The burn block indices and corresponding transaction signatures. + #[cbor(n(0), with = "cbor::burn_index_signature_vec")] + transactions: Vec<(LedgerBurnIndex, Signature)>, + }, } /// Payload of the `AcceptedWithdrawSolRequest` event. diff --git a/minter/src/state/event/cbor/mod.rs b/minter/src/state/event/cbor/mod.rs index fcc89a0f..220883c7 100644 --- a/minter/src/state/event/cbor/mod.rs +++ b/minter/src/state/event/cbor/mod.rs @@ -53,6 +53,48 @@ pub mod signature { } } +pub mod burn_index_signature_vec { + use crate::numeric::LedgerBurnIndex; + use minicbor::{ + decode::{Decoder, Error}, + encode::{Encoder, Write}, + }; + use solana_signature::Signature; + + pub fn decode( + d: &mut Decoder<'_>, + _ctx: &mut Ctx, + ) -> Result, Error> { + let len = d + .array()? + .ok_or_else(|| Error::message("expected definite-length array"))?; + let mut result = Vec::with_capacity(len as usize); + for _ in 0..len { + d.array()?; + let burn_index = LedgerBurnIndex::new(d.u64()?); + let sig_bytes = d.bytes()?; + let signature = + Signature::try_from(sig_bytes).map_err(|e| Error::message(e.to_string()))?; + result.push((burn_index, signature)); + } + Ok(result) + } + + pub fn encode( + v: &Vec<(LedgerBurnIndex, Signature)>, + e: &mut Encoder, + _ctx: &mut Ctx, + ) -> Result<(), minicbor::encode::Error> { + e.array(v.len() as u64)?; + for (burn_index, signature) in v { + e.array(2)?; + e.u64(*burn_index.get())?; + e.bytes(signature.as_ref())?; + } + Ok(()) + } +} + pub mod message { use minicbor::{ decode::{Decoder, Error}, diff --git a/minter/src/state/mod.rs b/minter/src/state/mod.rs index 236b098a..f21b3c97 100644 --- a/minter/src/state/mod.rs +++ b/minter/src/state/mod.rs @@ -4,7 +4,7 @@ use crate::{ state::event::{DepositId, WithdrawSolRequest}, }; use candid::Principal; -use cksol_types::{DepositStatus, WithdrawSolStatus}; +use cksol_types::{DepositStatus, SolTransaction, WithdrawSolStatus}; use cksol_types_internal::{Ed25519KeyName, InitArgs, UpgradeArgs}; use ic_canister_runtime::Runtime; use ic_ed25519::PublicKey; @@ -88,6 +88,7 @@ pub struct State { quarantined_deposits: BTreeMap, minted_deposits: BTreeMap, pending_withdrawal_requests: BTreeMap, + sent_withdrawal_requests: BTreeMap, funds_to_consolidate: BTreeMap, submitted_transactions: BTreeMap, active_tasks: BTreeSet, @@ -324,15 +325,22 @@ impl State { } pub fn withdrawal_status(&self, block_index: u64) -> WithdrawSolStatus { - if self - .pending_withdrawal_requests - .contains_key(&LedgerBurnIndex::from(block_index)) - { + let burn_index = LedgerBurnIndex::from(block_index); + if self.pending_withdrawal_requests.contains_key(&burn_index) { return WithdrawSolStatus::Pending; } + if let Some(sent_signature) = self.sent_withdrawal_requests.get(&burn_index) { + return WithdrawSolStatus::TxSent(SolTransaction { + transaction_hash: sent_signature.to_string(), + }); + } WithdrawSolStatus::NotFound } + pub fn pending_withdrawal_requests(&self) -> &BTreeMap { + &self.pending_withdrawal_requests + } + fn process_accepted_withdrawal(&mut self, request: &WithdrawSolRequest) { assert_eq!( self.pending_withdrawal_requests @@ -388,6 +396,27 @@ impl State { ); } + fn process_sent_withdrawal_transaction( + &mut self, + burn_block_index: &LedgerBurnIndex, + signature: &Signature, + ) { + assert!( + self.pending_withdrawal_requests + .remove(burn_block_index) + .is_some(), + "Attempted to send transaction for unknown withdrawal request: {:?}", + burn_block_index + ); + assert_eq!( + self.sent_withdrawal_requests + .insert(*burn_block_index, *signature), + None, + "Attempted to send transaction for already sent withdrawal request: {:?}", + burn_block_index + ); + } + fn process_consolidated_deposits(&mut self, deposits: &[(Account, Lamport)]) { for (account, amount) in deposits { let remaining = self @@ -486,6 +515,7 @@ impl TryFrom for State { quarantined_deposits: BTreeMap::new(), minted_deposits: BTreeMap::new(), pending_withdrawal_requests: BTreeMap::new(), + sent_withdrawal_requests: BTreeMap::new(), funds_to_consolidate: BTreeMap::new(), submitted_transactions: BTreeMap::new(), active_tasks: BTreeSet::new(), @@ -518,6 +548,7 @@ pub enum TaskType { DepositConsolidation, Mint, MonitorSubmittedTransactions, + WithdrawalProcessing, } #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/minter/src/state/tests.rs b/minter/src/state/tests.rs index de55c096..1d2e129b 100644 --- a/minter/src/state/tests.rs +++ b/minter/src/state/tests.rs @@ -47,6 +47,7 @@ mod state_from_init_args { quarantined_deposits: BTreeMap::new(), minted_deposits: BTreeMap::new(), pending_withdrawal_requests: BTreeMap::new(), + sent_withdrawal_requests: BTreeMap::new(), funds_to_consolidate: BTreeMap::new(), submitted_transactions: BTreeMap::new(), active_tasks: BTreeSet::new(), diff --git a/minter/src/test_fixtures/mod.rs b/minter/src/test_fixtures/mod.rs index 85ceb9e7..ec05b55a 100644 --- a/minter/src/test_fixtures/mod.rs +++ b/minter/src/test_fixtures/mod.rs @@ -285,11 +285,15 @@ pub mod arb { }), prop::collection::vec((arb_account(), any::()), 1..10) .prop_map(|deposits| EventType::ConsolidatedDeposits { deposits }), + prop::collection::vec((arb_ledger_burn_index(), arb_signature()), 1..10) + .prop_map(|transactions| EventType::SentWithdrawalTransaction { transactions },), (arb_signature(), arb_signature(), any::()).prop_map( - |(old_signature, new_signature, new_slot)| EventType::ResubmittedTransaction { - old_signature, - new_signature, - new_slot, + |(old_signature, new_signature, new_slot)| { + EventType::ResubmittedTransaction { + old_signature, + new_signature, + new_slot, + } }, ), arb_signature().prop_map(|signature| EventType::FinalizedTransaction { signature }), diff --git a/minter/src/test_fixtures/runtime.rs b/minter/src/test_fixtures/runtime.rs index c2aeaa64..e89cfba5 100644 --- a/minter/src/test_fixtures/runtime.rs +++ b/minter/src/test_fixtures/runtime.rs @@ -2,6 +2,7 @@ use super::{signer::MockSchnorrSigner, stubs::Stubs}; use crate::{runtime::CanisterRuntime, signer::SchnorrSigner}; use candid::{CandidType, Principal}; use ic_canister_runtime::{IcError, Runtime, StubRuntime}; +use ic_cdk::management_canister::SignCallError; use std::time::Duration; pub const TEST_CANISTER_ID: Principal = Principal::from_slice(&[0xCA; 10]); @@ -57,6 +58,11 @@ impl TestCanisterRuntime { self.signer = self.signer.add_signature(signature); self } + + pub fn add_schnorr_signing_error(mut self, error: SignCallError) -> Self { + self.signer = self.signer.add_response(Err(error)); + self + } } impl CanisterRuntime for TestCanisterRuntime { diff --git a/minter/src/withdraw_sol/mod.rs b/minter/src/withdraw_sol/mod.rs index f6d9ea03..240989fc 100644 --- a/minter/src/withdraw_sol/mod.rs +++ b/minter/src/withdraw_sol/mod.rs @@ -1,28 +1,38 @@ use std::str::FromStr; +use std::time::Duration; use candid::Principal; -use cksol_types::{WithdrawSolError, WithdrawSolOk}; +use cksol_types::{WithdrawSolError, WithdrawSolOk, WithdrawSolStatus}; use icrc_ledger_types::icrc1::account::{Account, Subaccount}; use icrc_ledger_types::icrc2::transfer_from::TransferFromError; use num_traits::ToPrimitive; use solana_address::Address; +use canlog::log; +use cksol_types_internal::log::Priority; + use crate::{ - guard::withdraw_sol_guard, + guard::{TimerGuard, withdraw_sol_guard}, ledger::burn, runtime::CanisterRuntime, + sol_transfer::create_signed_transfer_transaction, state::{ + TaskType, audit::process_event, event::{EventType, WithdrawSolRequest}, mutate_state, read_state, }, }; +pub const WITHDRAWAL_PROCESSING_DELAY: Duration = Duration::from_mins(1); +// The maximum number of withdrawal requests to process in a single timer invocation. +const MAX_WITHDRAWALS_PER_BATCH: usize = 10; + #[cfg(test)] mod tests; pub async fn withdraw_sol( - runtime: R, + runtime: &R, minter_account: Account, caller: Principal, from_subaccount: Option, @@ -43,7 +53,7 @@ pub async fn withdraw_sol( let solana_address = Address::from_str(&address) .map_err(|e| WithdrawSolError::MalformedAddress(e.to_string()))?; - let block_index = burn(&runtime, minter_account, from, amount, solana_address) + let block_index = burn(runtime, minter_account, from, amount, solana_address) .await .map_err(|e| match e { crate::ledger::BurnError::IcError(ic_error) => { @@ -103,11 +113,123 @@ pub async fn withdraw_sol( withdrawal_amount: amount, withdrawal_fee, }), - &runtime, + runtime, ) }); - // TODO DEFI-2671: trigger the timer to process pending withdrawals. - Ok(WithdrawSolOk { block_index }) } + +pub async fn process_pending_withdrawals(runtime: &R) { + let _guard = match TimerGuard::new(TaskType::WithdrawalProcessing) { + Ok(guard) => guard, + Err(_) => { + log!( + Priority::Info, + "failed to obtain WithdrawalProcessing guard, exiting" + ); + return; + } + }; + + // TODO: we should batch requests into up to N chunks of size M, each chunk + // should be a separate transaction containing multiple withdrawal requests. + // M is the max withdrawals per tx, N is max tx per round. + + let pending_requests: Vec = read_state(|state| { + state + .pending_withdrawal_requests() + .values() + .take(MAX_WITHDRAWALS_PER_BATCH) + .cloned() + .collect() + }); + + if pending_requests.is_empty() { + return; + } + + let recent_blockhash = + match read_state(|state| state.sol_rpc_client(runtime.inter_canister_call_runtime())) + .estimate_recent_blockhash() + .send() + .await + { + Ok(blockhash) => blockhash, + Err(errors) => { + log!( + Priority::Error, + "Failed to estimate recent blockhash: {errors:?}" + ); + return; + } + }; + + let minter_account: Account = runtime.canister_self().into(); + let signer = runtime.signer(); + + // TODO: we need to check whether the minter has enough funds in the main account. + // We probably need to add a state.minter_balance variable and update it + // here and while consolidating funds. + // If there are not enough funds for the withdrawal we simply continue. + + let withdrawal_params: Vec<_> = pending_requests + .iter() + .map(|request| { + let destination = Address::from(request.solana_address); + let transfer_amount = request + .withdrawal_amount + .checked_sub(request.withdrawal_fee) + .expect("BUG: withdrawal_amount must be >= withdrawal_fee"); + let sources = vec![(minter_account, transfer_amount)]; + (sources, destination) + }) + .collect(); + + let sign_futures: Vec<_> = withdrawal_params + .iter() + .map(|(sources, destination)| { + create_signed_transfer_transaction( + minter_account, + sources, + *destination, + recent_blockhash, + &signer, + ) + }) + .collect(); + + let results = futures::future::join_all(sign_futures).await; + + for (request, result) in pending_requests.into_iter().zip(results) { + let transaction = match result { + Ok(tx) => tx, + Err(e) => { + log!( + Priority::Error, + "Failed to create withdrawal transaction for burn index {:?}: {e}", + request.burn_block_index + ); + continue; + } + }; + + let signature = transaction.0.signatures[0]; + + mutate_state(|state| { + process_event( + state, + EventType::SentWithdrawalTransaction { + transactions: vec![(request.burn_block_index, signature)], + }, + runtime, + ) + }); + + // TODO: Send the transaction to the Solana network via RPC + } +} + +pub fn withdraw_sol_status(block_index: u64) -> WithdrawSolStatus { + read_state(|s| s.withdrawal_status(block_index)) +} diff --git a/minter/src/withdraw_sol/tests.rs b/minter/src/withdraw_sol/tests.rs index 118e2168..ea9e29d7 100644 --- a/minter/src/withdraw_sol/tests.rs +++ b/minter/src/withdraw_sol/tests.rs @@ -1,13 +1,27 @@ +use crate::numeric::LedgerBurnIndex; +use crate::test_fixtures::EventsAssert; +use crate::withdraw_sol::MAX_WITHDRAWALS_PER_BATCH; use crate::{ - guard::withdraw_sol_guard, - test_fixtures::{MINTER_ACCOUNT, init_state, runtime::TestCanisterRuntime}, - withdraw_sol::withdraw_sol, + guard::{TimerGuard, withdraw_sol_guard}, + state::TaskType, + test_fixtures::{ + MINTER_ACCOUNT, WITHDRAWAL_FEE, init_schnorr_master_key, init_state, + runtime::TestCanisterRuntime, + }, + withdraw_sol::{process_pending_withdrawals, withdraw_sol}, }; +use crate::{state::event::EventType, withdraw_sol::withdraw_sol_status}; use assert_matches::assert_matches; use candid::{Nat, Principal}; +use canlog::Log; +use cksol_types::WithdrawSolStatus; use cksol_types::{WithdrawSolError, WithdrawSolOk}; +use cksol_types_internal::log::Priority; use ic_canister_runtime::IcError; +use ic_cdk::call::CallRejected; +use ic_cdk::management_canister::SignCallError; use icrc_ledger_types::{icrc1::account::Account, icrc2::transfer_from::TransferFromError}; +use sol_rpc_types::{ConfirmedBlock, MultiRpcResult, RpcError, Slot}; const VALID_ADDRESS: &str = "E4MpwNnMWs2XtW5gVrxZvyS7fMq31QD5HvbxmwP45Tz3"; @@ -22,7 +36,7 @@ async fn should_return_error_if_calling_ledger_fails() { let runtime = TestCanisterRuntime::new().add_stub_error(IcError::CallPerformFailed); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -46,7 +60,7 @@ async fn should_return_error_if_ledger_unavailable() { )); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -74,7 +88,7 @@ async fn should_return_error_if_insufficient_allowance() { )); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -100,7 +114,7 @@ async fn should_return_error_if_insufficient_funds() { )); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -127,7 +141,7 @@ async fn should_return_generic_error() { )); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -154,7 +168,7 @@ async fn should_return_ok_if_burn_succeeds() { .with_increasing_time(); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -178,7 +192,7 @@ async fn should_return_error_if_address_malformed() { let runtime = TestCanisterRuntime::new(); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, test_caller(), None, @@ -198,7 +212,7 @@ async fn should_panic_if_caller_is_anonymous() { let runtime = TestCanisterRuntime::new(); let _ = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, Principal::anonymous(), None, @@ -222,7 +236,7 @@ async fn should_return_error_if_already_processing() { let runtime = TestCanisterRuntime::new(); let result = withdraw_sol( - runtime, + &runtime, MINTER_ACCOUNT, caller, None, @@ -233,3 +247,271 @@ async fn should_return_error_if_already_processing() { assert_eq!(result, Err(WithdrawSolError::AlreadyProcessing)); } + +mod process_pending_withdrawals_tests { + use super::*; + + type SendSlotResult = MultiRpcResult; + type SendBlockResult = MultiRpcResult; + + #[tokio::test] + async fn should_do_nothing_if_no_pending_withdrawals() { + init_state(); + + let runtime = TestCanisterRuntime::new(); + process_pending_withdrawals(&runtime).await; + + EventsAssert::assert_no_events_recorded(); + } + + #[tokio::test] + async fn should_skip_if_already_processing() { + init_state(); + + let _guard = TimerGuard::new(TaskType::WithdrawalProcessing).unwrap(); + + let runtime = TestCanisterRuntime::new(); + process_pending_withdrawals(&runtime).await; + + let mut log: Log = Log::default(); + log.push_logs(Priority::Info); + assert!( + log.entries.iter().any(|e| e + .message + .contains("failed to obtain WithdrawalProcessing guard, exiting")), + "Expected info about failing to obtain guard, got: {:?}", + log.entries + ); + } + + #[tokio::test] + async fn should_acquire_and_release_guard() { + init_state(); + + let runtime = TestCanisterRuntime::new(); + process_pending_withdrawals(&runtime).await; + + // Guard should be released, so we can acquire it again + let _guard = TimerGuard::new(TaskType::WithdrawalProcessing).unwrap(); + } + + async fn withdraw(runtime: &TestCanisterRuntime, count: u8) { + for i in 1..count + 1 { + let _ = withdraw_sol( + runtime, + MINTER_ACCOUNT, + Principal::from_slice(&[1, i]), + None, + WITHDRAWAL_FEE + 1, + VALID_ADDRESS.to_string(), + ) + .await + .unwrap(); + } + } + + #[tokio::test] + async fn should_process_when_pending_withdrawals_exist() { + init_state(); + init_schnorr_master_key(); + + let fake_sig = [0x42; 64]; + + let runtime = TestCanisterRuntime::new() + // ledger burn response for withdraw_sol + .add_stub_response(Ok::(Nat::from(1u64))) + // responses for recent block hash + .add_stub_response(SendSlotResult::Consistent(Ok(1))) + .add_stub_response(SendBlockResult::Consistent(Ok(get_confirmed_block()))) + // schnorr signing response + .add_signature(fake_sig) + .with_increasing_time(); + + withdraw(&runtime, 1).await; + + process_pending_withdrawals(&runtime).await; + + EventsAssert::from_recorded() + .expect_event(|e| { + assert_matches!(e, EventType::AcceptedWithdrawSolRequest(req) => { + assert_eq!(req.withdrawal_amount, WITHDRAWAL_FEE + 1); + assert_eq!(req.withdrawal_fee, WITHDRAWAL_FEE); + }); + }) + .expect_event(|e| { + assert_matches!(e, EventType::SentWithdrawalTransaction { transactions, .. } => { + assert_eq!(transactions.len(), 1); + assert_eq!(transactions[0].1, fake_sig.into()); + }); + }) + .assert_no_more_events(); + + assert_matches!(withdraw_sol_status(1), WithdrawSolStatus::TxSent(_)); + } + + #[tokio::test] + async fn should_log_error_when_blockhash_fetch_fails() { + init_state(); + + let runtime = TestCanisterRuntime::new() + // ledger burn response for withdraw_sol + .add_stub_response(Ok::(Nat::from(1u64))) + // estimate_recent_blockhash retries getSlot 3 times before giving up + .add_stub_response(SendSlotResult::Consistent(Err(RpcError::ValidationError( + "slot unavailable".to_string(), + )))) + .add_stub_response(SendSlotResult::Consistent(Err(RpcError::ValidationError( + "slot unavailable".to_string(), + )))) + .add_stub_response(SendSlotResult::Consistent(Err(RpcError::ValidationError( + "slot unavailable".to_string(), + )))) + .with_increasing_time(); + + withdraw(&runtime, 1).await; + + process_pending_withdrawals(&runtime).await; + + // No withdrawal transaction event should be recorded + EventsAssert::from_recorded() + .expect_event(|e| { + assert_matches!(e, EventType::AcceptedWithdrawSolRequest(_)); + }) + .assert_no_more_events(); + + // An error should be logged + let mut log: Log = Log::default(); + log.push_logs(Priority::Error); + assert!( + log.entries + .iter() + .any(|e| e.message.contains("Failed to estimate recent blockhash")), + "Expected error log about blockhash failure, got: {:?}", + log.entries + ); + + assert_eq!(withdraw_sol_status(1), WithdrawSolStatus::Pending); + } + + #[tokio::test] + async fn should_not_process_pending_withdrawal_sig_error() { + init_state(); + init_schnorr_master_key(); + + let runtime = TestCanisterRuntime::new() + // responses for burn blocks + .add_stub_response(Ok::(Nat::from(1u64))) + .add_stub_response(Ok::(Nat::from(2u64))) + // responses for recent block hash + .add_stub_response(SendSlotResult::Consistent(Ok(1))) + .add_stub_response(SendBlockResult::Consistent(Ok(get_confirmed_block()))) + // one successful signature and one failed + .add_signature([0x42; 64]) + .add_schnorr_signing_error(SignCallError::CallFailed( + CallRejected::with_rejection(4, "signing service unavailable".to_string()).into(), + )) + .with_increasing_time(); + + withdraw(&runtime, 2).await; + + process_pending_withdrawals(&runtime).await; + + EventsAssert::from_recorded() + .expect_event(|e| { + assert_matches!(e, EventType::AcceptedWithdrawSolRequest(req) => { + assert_eq!(req.withdrawal_amount, WITHDRAWAL_FEE + 1); + assert_eq!(req.withdrawal_fee, WITHDRAWAL_FEE); + assert_eq!(req.account, Principal::from_slice(&[1, 1]).into()); + }); + }) + .expect_event(|e| { + assert_matches!(e, EventType::AcceptedWithdrawSolRequest(req) => { + assert_eq!(req.withdrawal_amount, WITHDRAWAL_FEE + 1); + assert_eq!(req.withdrawal_fee, WITHDRAWAL_FEE); + assert_eq!(req.account, Principal::from_slice(&[1, 2]).into()); + }); + }) + .expect_event(|e| { + assert_matches!(e, EventType::SentWithdrawalTransaction { transactions, .. } => { + assert_eq!(transactions.len(), 1); + assert_eq!(transactions[0].0, LedgerBurnIndex::from(1u64)); + }); + }) + .assert_no_more_events(); + + // An error should be logged + let mut log: Log = Log::default(); + log.push_logs(Priority::Error); + assert!( + log.entries.iter().any(|e| e + .message + .contains("Failed to create withdrawal transaction for burn index 2")), + "Expected error log about sig failure, got: {:?}", + log.entries + ); + + assert_matches!(withdraw_sol_status(1), WithdrawSolStatus::TxSent(_)); + assert_matches!(withdraw_sol_status(2), WithdrawSolStatus::Pending); + } + + #[tokio::test] + async fn should_process_up_to_max() { + init_state(); + init_schnorr_master_key(); + + let request_count = MAX_WITHDRAWALS_PER_BATCH as u64 + 1; + + let mut runtime = TestCanisterRuntime::new().with_increasing_time(); + // withdraw ledger burn responses + for i in 0..request_count { + runtime = runtime.add_stub_response(Ok::(Nat::from(i))); + } + // responses for recent block hash + runtime = runtime + .add_stub_response(SendSlotResult::Consistent(Ok(1))) + .add_stub_response(SendBlockResult::Consistent(Ok(get_confirmed_block()))); + // signatures for the first batch of withdrawals + for _ in 0..MAX_WITHDRAWALS_PER_BATCH { + runtime = runtime.add_signature([0x42; 64]); + } + // responses for recent block hash and signature for the second batch + runtime = runtime + .add_stub_response(SendSlotResult::Consistent(Ok(1))) + .add_stub_response(SendBlockResult::Consistent(Ok(get_confirmed_block()))) + .add_signature([0x42; 64]); + + withdraw(&runtime, request_count as u8).await; + + process_pending_withdrawals(&runtime).await; + + for i in 0..MAX_WITHDRAWALS_PER_BATCH { + assert_matches!(withdraw_sol_status(i as u64), WithdrawSolStatus::TxSent(_)); + } + // last withdrawal was not yet processed + assert_matches!( + withdraw_sol_status(MAX_WITHDRAWALS_PER_BATCH as u64), + WithdrawSolStatus::Pending + ); + + process_pending_withdrawals(&runtime).await; + + // all withdrawals are now processed + for i in 0..request_count { + assert_matches!(withdraw_sol_status(i), WithdrawSolStatus::TxSent(_)); + } + } + + fn get_confirmed_block() -> ConfirmedBlock { + ConfirmedBlock { + previous_blockhash: Default::default(), + blockhash: solana_hash::Hash::new_from_array([0x42; 32]).into(), + parent_slot: 0, + block_time: None, + block_height: None, + signatures: None, + rewards: None, + num_reward_partitions: None, + transactions: None, + } + } +}