diff --git a/Cargo.lock b/Cargo.lock index fc7896618068..597912f8c4a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7690,6 +7690,7 @@ version = "0.9.0" dependencies = [ "assert_matches", "async-trait", + "bitcoin-dogecoin", "bs58 0.5.1", "candid", "candid_parser", @@ -7703,12 +7704,14 @@ dependencies = [ "ic-management-canister-types 0.5.0", "ic-secp256k1", "icrc-ledger-types", + "mockall", "pocket-ic", "proptest", "serde", "serde_json", "sha2 0.10.9", "test-strategy 0.4.0", + "tokio", ] [[package]] diff --git a/rs/bitcoin/ckbtc/minter/src/lib.rs b/rs/bitcoin/ckbtc/minter/src/lib.rs index bd26d1933cf2..8a6c0d062bad 100644 --- a/rs/bitcoin/ckbtc/minter/src/lib.rs +++ b/rs/bitcoin/ckbtc/minter/src/lib.rs @@ -9,7 +9,6 @@ use candid::{CandidType, Deserialize, Principal}; use canlog::log; use ic_cdk::bitcoin_canister; use ic_cdk::management_canister::SignWithEcdsaArgs; -use ic_management_canister_types_private::DerivationPath; use icrc_ledger_types::icrc1::account::{Account, Subaccount}; use icrc_ledger_types::icrc1::transfer::Memo; use scopeguard::{ScopeGuard, guard}; @@ -22,6 +21,7 @@ use crate::fees::{BitcoinFeeEstimator, FeeEstimator}; use crate::state::eventlog::{CkBtcEventLogger, EventLogger}; use crate::state::utxos::UtxoSet; use crate::state::{CkBtcMinterState, mutate_state, read_state}; +use crate::tx::{BitcoinTransactionSigner, SignedRawTransaction, UnsignedTransaction}; use crate::updates::get_btc_address; use crate::updates::retrieve_btc::BtcAddressCheckStatus; pub use ic_btc_checker::CheckTransactionResponse; @@ -164,6 +164,7 @@ struct SignTxRequest { network: Network, ecdsa_public_key: ECDSAPublicKey, unsigned_tx: tx::UnsignedTransaction, + accounts: Vec, change_output: state::ChangeOutput, requests: state::SubmittedWithdrawalRequests, utxos: Vec, @@ -362,6 +363,7 @@ async fn submit_pending_requests(runtime: &R) { ecdsa_public_key, change_output, network: s.btc_network, + accounts: s.find_all_accounts(&unsigned_tx), unsigned_tx, requests: state::SubmittedWithdrawalRequests::ToConfirm { requests: batch.into_iter().collect(), @@ -488,22 +490,22 @@ async fn sign_and_submit_request( undo_withdrawal_request(reqs, utxos); }); - let txid = req.unsigned_tx.txid(); - let signed_tx = sign_transaction( - req.key_name, - &req.ecdsa_public_key, - |outpoint| state::read_state(|s| s.outpoint_account.get(outpoint).cloned()), - req.unsigned_tx, - runtime, - ) - .await - .inspect_err(|err| { - log!( - Priority::Info, - "[sign_and_submit_request]: failed to sign a Bitcoin transaction: {}", - err - ); - })?; + let signed_tx = runtime + .sign_transaction( + req.key_name, + req.ecdsa_public_key, + req.unsigned_tx, + req.accounts, + ) + .await + .inspect_err(|err| { + log!( + Priority::Info, + "[sign_and_submit_request]: failed to sign a Bitcoin transaction: {}", + err + ); + })?; + let txid = signed_tx.txid(); state::mutate_state(|s| { for block_index in requests_guard.0.iter_block_index() { @@ -514,10 +516,11 @@ async fn sign_and_submit_request( log!( Priority::Info, "[sign_and_submit_request]: sending a signed transaction {}", - hex::encode(tx::encode_into(&signed_tx, Vec::new())) + hex::encode(&signed_tx) ); + let signed_tx_bytes = signed_tx.into_bytes(); runtime - .send_transaction(&signed_tx, req.network) + .send_raw_transaction(signed_tx_bytes.clone(), req.network) .await .inspect_err(|err| { log!( @@ -538,7 +541,7 @@ async fn sign_and_submit_request( // Only fill signed_tx when it is a consolidation transaction. let signed_tx = match requests { - state::SubmittedWithdrawalRequests::ToConsolidate { .. } => Some(signed_tx.serialize()), + state::SubmittedWithdrawalRequests::ToConsolidate { .. } => Some(signed_tx_bytes), _ => None, }; @@ -698,7 +701,7 @@ async fn finalize_requests(runtime: &R) { // one confirmation. let main_utxos_zero_confirmations = match management::get_utxos( btc_network, - &main_address.display(btc_network), + &main_address_str, /*min_confirmations=*/ 0, management::CallSource::Minter, runtime, @@ -759,7 +762,6 @@ async fn finalize_requests(runtime: &R) { btc_network, state::read_state(|s| s.retrieve_btc_min_amount), maybe_finalized_transactions, - |outpoint| state::read_state(|s| s.outpoint_account.get(outpoint).cloned()), |old_txid, new_tx, reason| { state::mutate_state(|s| { state::audit::replace_transaction(s, old_txid, new_tx, reason, runtime); @@ -773,7 +775,6 @@ async fn finalize_requests(runtime: &R) { pub async fn resubmit_transactions< R: CanisterRuntime, - F: Fn(&OutPoint) -> Option, G: Fn(Txid, state::SubmittedBtcTransaction, state::eventlog::ReplacedReason), Fee: FeeEstimator, >( @@ -784,7 +785,6 @@ pub async fn resubmit_transactions< btc_network: Network, retrieve_btc_min_amount: u64, transactions: BTreeMap, - lookup_outpoint_account: F, replace_transaction: G, runtime: &R, fee_estimator: &Fee, @@ -921,15 +921,15 @@ pub async fn resubmit_transactions< } }; - let new_txid = unsigned_tx.txid(); - let maybe_signed_tx = sign_transaction( - key_name.to_string(), - &ecdsa_public_key, - &lookup_outpoint_account, - unsigned_tx, - runtime, - ) - .await; + let accounts = read_state(|s| s.find_all_accounts(&unsigned_tx)); + let maybe_signed_tx = runtime + .sign_transaction( + key_name.to_string(), + ecdsa_public_key.clone(), + unsigned_tx, + accounts, + ) + .await; let signed_tx = match maybe_signed_tx { Ok(tx) => tx, @@ -942,8 +942,13 @@ pub async fn resubmit_transactions< continue; } }; + let new_txid = signed_tx.txid(); - match runtime.send_transaction(&signed_tx, btc_network).await { + let signed_tx_hex = hex::encode(&signed_tx); + match runtime + .send_raw_transaction(signed_tx.into_bytes(), btc_network) + .await + { Ok(()) => { if old_txid == new_txid { // DEFENSIVE: We should never take this branch because we increase fees for @@ -952,18 +957,16 @@ pub async fn resubmit_transactions< // equality in case the fee computation rules change in the future. log!( Priority::Info, - "[finalize_requests]: resent transaction {} with a new signature. TX bytes: {}", + "[finalize_requests]: resent transaction {} with a new signature. TX bytes: {signed_tx_hex}", &new_txid, - hex::encode(tx::encode_into(&signed_tx, Vec::new())) ); continue; } log!( Priority::Info, - "[finalize_requests]: sent transaction {} to replace stuck transaction {}. TX bytes: {}", + "[finalize_requests]: sent transaction {} to replace stuck transaction {}. TX bytes: {signed_tx_hex}", &new_txid, &old_txid, - hex::encode(tx::encode_into(&signed_tx, Vec::new())) ); let new_tx = state::SubmittedBtcTransaction { requests: new_tx_requests, @@ -981,8 +984,7 @@ pub async fn resubmit_transactions< Err(err) => { log!( Priority::Info, - "[finalize_requests]: failed to send transaction bytes {} to replace stuck transaction {}: {}", - hex::encode(tx::encode_into(&signed_tx, Vec::new())), + "[finalize_requests]: failed to send transaction bytes {signed_tx_hex} to replace stuck transaction {}: {}", &old_txid, err, ); @@ -1065,59 +1067,6 @@ fn greedy(target: u64, available_utxos: &mut UtxoSet) -> Vec { solution } -/// Gathers ECDSA signatures for all the inputs in the specified unsigned -/// transaction. -/// -/// # Panics -/// -/// This function panics if the `output_account` map does not have an entry for -/// at least one of the transaction previous output points. -pub async fn sign_transaction Option>( - key_name: String, - ecdsa_public_key: &ECDSAPublicKey, - lookup_outpoint_account: F, - unsigned_tx: tx::UnsignedTransaction, - runtime: &R, -) -> Result { - use crate::address::{derivation_path, derive_public_key_from_account}; - - let mut signed_inputs = Vec::with_capacity(unsigned_tx.inputs.len()); - let sighasher = tx::TxSigHasher::new(&unsigned_tx); - for input in &unsigned_tx.inputs { - let outpoint = &input.previous_output; - - let account = lookup_outpoint_account(outpoint) - .unwrap_or_else(|| panic!("bug: no account for outpoint {outpoint:?}")); - - let path = derivation_path(&account); - let pubkey = - ByteBuf::from(derive_public_key_from_account(ecdsa_public_key, &account).public_key); - let pkhash = tx::hash160(&pubkey); - - let sighash = sighasher.sighash(input, &pkhash); - - let sec1_signature = management::sign_with_ecdsa( - key_name.clone(), - DerivationPath::new(path), - sighash, - runtime, - ) - .await?; - - signed_inputs.push(tx::SignedInput { - signature: signature::EncodedSignature::from_sec1(&sec1_signature), - pubkey, - previous_output: outpoint.clone(), - sequence: input.sequence, - }); - } - Ok(tx::SignedTransaction { - inputs: signed_inputs, - outputs: unsigned_tx.outputs, - lock_time: unsigned_tx.lock_time, - }) -} - pub fn fake_sign(unsigned_tx: &tx::UnsignedTransaction) -> tx::SignedTransaction { tx::SignedTransaction { inputs: unsigned_tx @@ -1556,6 +1505,7 @@ pub async fn consolidate_utxos( ecdsa_public_key, change_output, network: s.btc_network, + accounts: s.find_all_accounts(&unsigned_tx), unsigned_tx, requests: state::SubmittedWithdrawalRequests::ToConsolidate { request }, utxos, @@ -1660,6 +1610,14 @@ pub trait CanisterRuntime { memo: Memo, ) -> Result; + async fn sign_transaction( + &self, + key_name: String, + ecdsa_public_key: ECDSAPublicKey, + unsigned_tx: UnsignedTransaction, + accounts: Vec, + ) -> Result; + async fn sign_with_ecdsa( &self, key_name: String, @@ -1667,12 +1625,6 @@ pub trait CanisterRuntime { message_hash: [u8; 32], ) -> Result, CallError>; - async fn send_transaction( - &self, - transaction: &tx::SignedTransaction, - network: Network, - ) -> Result<(), CallError>; - async fn send_raw_transaction( &self, raw_transaction: Vec, @@ -1739,6 +1691,17 @@ impl CanisterRuntime for IcCanisterRuntime { updates::update_balance::mint(amount, to, memo).await } + async fn sign_transaction( + &self, + key_name: String, + ecdsa_public_key: ECDSAPublicKey, + unsigned_tx: UnsignedTransaction, + accounts: Vec, + ) -> Result { + let signer = BitcoinTransactionSigner::new(key_name, ecdsa_public_key); + signer.sign_transaction(unsigned_tx, accounts, self).await + } + async fn sign_with_ecdsa( &self, key_name: String, @@ -1758,14 +1721,6 @@ impl CanisterRuntime for IcCanisterRuntime { .map_err(CallError::from_sign_error) } - async fn send_transaction( - &self, - transaction: &tx::SignedTransaction, - network: Network, - ) -> Result<(), CallError> { - management::send_transaction(transaction, network).await - } - async fn send_raw_transaction( &self, transaction: Vec, diff --git a/rs/bitcoin/ckbtc/minter/src/management.rs b/rs/bitcoin/ckbtc/minter/src/management.rs index 48a1fb3ae177..08dddb21c4cb 100644 --- a/rs/bitcoin/ckbtc/minter/src/management.rs +++ b/rs/bitcoin/ckbtc/minter/src/management.rs @@ -238,14 +238,14 @@ pub async fn ecdsa_public_key( /// Signs a message hash using the tECDSA API. pub async fn sign_with_ecdsa( key_name: String, - derivation_path: DerivationPath, + derivation_path: Vec>, message_hash: [u8; 32], runtime: &R, ) -> Result, CallError> { let start_time = runtime.time(); let result = runtime - .sign_with_ecdsa(key_name, derivation_path.into_inner(), message_hash) + .sign_with_ecdsa(key_name, derivation_path, message_hash) .await; observe_sign_with_ecdsa_latency(&result, start_time, runtime.time()); diff --git a/rs/bitcoin/ckbtc/minter/src/signature.rs b/rs/bitcoin/ckbtc/minter/src/signature.rs index 18ab4d5ccd4b..4ffaa5881632 100644 --- a/rs/bitcoin/ckbtc/minter/src/signature.rs +++ b/rs/bitcoin/ckbtc/minter/src/signature.rs @@ -5,7 +5,7 @@ use std::fmt; /// The length of the transaction signature. pub const MAX_ENCODED_SIGNATURE_LEN: usize = 73; -const FAKE_SIG: [u8; MAX_ENCODED_SIGNATURE_LEN] = [ +pub const FAKE_SIG: [u8; MAX_ENCODED_SIGNATURE_LEN] = [ 0x30, 70, 0x02, 33, 0x00, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 33, 0x00, 0x8f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, diff --git a/rs/bitcoin/ckbtc/minter/src/state.rs b/rs/bitcoin/ckbtc/minter/src/state.rs index 19ee0d4ebe6b..757d8a2269ca 100644 --- a/rs/bitcoin/ckbtc/minter/src/state.rs +++ b/rs/bitcoin/ckbtc/minter/src/state.rs @@ -6,7 +6,7 @@ #[cfg(test)] mod tests; -use crate::Priority; +use crate::{Priority, tx}; use std::{ cell::RefCell, collections::{BTreeMap, BTreeSet, VecDeque}, @@ -1774,6 +1774,27 @@ impl CkBtcMinterState { "BUG: Reimbursement of withdrawal {reimbursement:?} was already completed!" ); } + + /// Find all accounts used for the transaction previous output points. + /// + /// # Panics + /// + /// This function panics if the `output_account` map does not have an entry for + /// at least one of the transaction previous output points. + pub fn find_all_accounts(&self, tx: &tx::UnsignedTransaction) -> Vec { + let mut accounts = Vec::with_capacity(tx.inputs.len()); + for input in &tx.inputs { + accounts.push( + self.outpoint_account + .get(&input.previous_output) + .copied() + .unwrap_or_else(|| { + panic!("BUG: no account for outpoint {:?}", &input.previous_output) + }), + ) + } + accounts + } } #[derive(Eq, PartialEq, Debug, Default)] diff --git a/rs/bitcoin/ckbtc/minter/src/test_fixtures.rs b/rs/bitcoin/ckbtc/minter/src/test_fixtures.rs index 231edd6ff7b4..ae532d224157 100644 --- a/rs/bitcoin/ckbtc/minter/src/test_fixtures.rs +++ b/rs/bitcoin/ckbtc/minter/src/test_fixtures.rs @@ -174,15 +174,16 @@ pub fn bitcoin_fee_estimator() -> BitcoinFeeEstimator { } pub mod mock { - use crate::CkBtcMinterState; use crate::fees::BitcoinFeeEstimator; use crate::management::CallError; use crate::state::eventlog::CkBtcEventLogger; + use crate::tx::{SignedRawTransaction, UnsignedTransaction}; use crate::updates::update_balance::UpdateBalanceError; use crate::{ BitcoinAddress, BtcAddressCheckStatus, CanisterRuntime, GetCurrentFeePercentilesRequest, - GetUtxosRequest, GetUtxosResponse, Network, tx, + GetUtxosRequest, GetUtxosResponse, Network, }; + use crate::{CkBtcMinterState, ECDSAPublicKey}; use async_trait::async_trait; use candid::Principal; use ic_btc_checker::CheckTransactionResponse; @@ -217,7 +218,7 @@ pub mod mock { async fn check_transaction(&self, btc_checker_principal: Option, utxo: &Utxo, cycle_payment: u128, ) -> Result; async fn mint_ckbtc(&self, amount: u64, to: Account, memo: Memo) -> Result; async fn sign_with_ecdsa(&self, key_name: String, derivation_path: Vec>, message_hash: [u8; 32]) -> Result, CallError>; - async fn send_transaction(&self, transaction: &tx::SignedTransaction, network: Network) -> Result<(), CallError>; + async fn sign_transaction( &self, key_name: String, ecdsa_public_key: ECDSAPublicKey, unsigned_tx: UnsignedTransaction, accounts: Vec) -> Result; async fn send_raw_transaction(&self, transaction: Vec, network: Network) -> Result<(), CallError>; async fn check_address( &self, btc_checker_principal: Option, address: String) -> Result; } diff --git a/rs/bitcoin/ckbtc/minter/src/tests.rs b/rs/bitcoin/ckbtc/minter/src/tests.rs index 122910d3d275..eaa689c26b65 100644 --- a/rs/bitcoin/ckbtc/minter/src/tests.rs +++ b/rs/bitcoin/ckbtc/minter/src/tests.rs @@ -532,7 +532,6 @@ proptest! { prop_assert_eq!(btc_tx.serialize(), tx_bytes); prop_assert_eq!(&decoded_btc_tx, &btc_tx); - prop_assert_eq!(&arb_tx.txid().as_ref().to_vec(), &*btc_tx.txid()); } #[test] @@ -598,6 +597,7 @@ proptest! { prop_assert_eq!(btc_tx.serialize(), tx_bytes); prop_assert_eq!(&decoded_btc_tx, &btc_tx); prop_assert_eq!(&arb_tx.wtxid(), &*btc_tx.wtxid()); + prop_assert_eq!(&<[u8;32]>::from(arb_tx.compute_txid()), &*btc_tx.txid()); prop_assert_eq!(arb_tx.vsize(), btc_tx.vsize()); } @@ -824,7 +824,9 @@ proptest! { fee_per_vbyte ) .expect("failed to build transaction"); - let mut txids = vec![tx.txid()]; + let signed_tx = fake_sign(&tx); + let mut txids = vec![signed_tx.compute_txid()]; + let submitted_at = 1_234_567_890; state.push_submitted_transaction(SubmittedBtcTransaction { @@ -850,8 +852,9 @@ proptest! { fee_per_vbyte + 1000 * i as u64, ) .expect("failed to build transaction"); + let new_signed_tx = fake_sign(&tx); - let new_txid = tx.txid(); + let new_txid = new_signed_tx.compute_txid(); state.replace_transaction(prev_txid, SubmittedBtcTransaction { requests: requests.clone().into(), diff --git a/rs/bitcoin/ckbtc/minter/src/tx.rs b/rs/bitcoin/ckbtc/minter/src/tx.rs index f54d546a7d8d..ee07c3d83125 100644 --- a/rs/bitcoin/ckbtc/minter/src/tx.rs +++ b/rs/bitcoin/ckbtc/minter/src/tx.rs @@ -7,14 +7,16 @@ use ic_crypto_sha2::Sha256; use serde_bytes::{ByteBuf, Bytes}; use std::fmt; +use crate::{CanisterRuntime, ECDSAPublicKey, management, signature, tx}; pub use ic_btc_interface::{OutPoint, Satoshi, Txid}; +use icrc_ledger_types::icrc1::account::Account; /// The current Bitcoin transaction encoding version. /// See https://github.com/bitcoin/bitcoin/blob/c90f86e4c7760a9f7ed0a574f54465964e006a64/src/primitives/transaction.h#L291. pub const TX_VERSION: u32 = 2; /// The length of the public key. -pub const PUBKEY_LEN: usize = 32; +pub const PUBKEY_LEN: usize = 33; // The marker indicating the segregated witness encoding. const MARKER: u8 = 0; @@ -340,20 +342,36 @@ impl<'a> TxSigHasher<'a> { } } -#[derive(Eq, PartialEq, Debug)] +#[derive(Clone, Eq, PartialEq, Debug)] pub struct UnsignedTransaction { pub inputs: Vec, pub outputs: Vec, pub lock_time: u32, } -impl UnsignedTransaction { +#[derive(Eq, PartialEq, Debug, Clone)] +pub struct SignedRawTransaction { + signed_tx: Vec, + txid: Txid, +} + +impl SignedRawTransaction { + pub fn new(signed_tx: Vec, txid: Txid) -> Self { + Self { signed_tx, txid } + } + pub fn txid(&self) -> Txid { - Sha256::hash(&encode_into(self, Sha256::new())).into() + self.txid } - pub fn serialized_len(&self) -> usize { - encode_into(self, CountBytes::default()) + pub fn into_bytes(self) -> Vec { + self.signed_tx + } +} + +impl AsRef<[u8]> for SignedRawTransaction { + fn as_ref(&self) -> &[u8] { + &self.signed_tx } } @@ -384,6 +402,15 @@ impl SignedTransaction { Sha256::hash(&encode_into(self, Sha256::new())) } + /// Computes the [`Txid`]. + /// + /// Hashes the transaction **excluding** the segwit data (i.e. the marker, flag bytes, and the + /// witness fields themselves). For non-segwit transactions which do not have any segwit data, + /// this will be equal to [`Self::wtxid`]. + pub fn compute_txid(&self) -> Txid { + Txid::from(Sha256::hash(&encode_into(&BaseTxView(self), Sha256::new()))) + } + /// Returns the virtual transaction size that nodes use to compute fees. pub fn vsize(&self) -> usize { // # Transaction size calculations @@ -547,3 +574,69 @@ impl Encode for SignedTransaction { self.lock_time.encode(buf) } } + +pub struct BitcoinTransactionSigner { + key_name: String, + ecdsa_public_key: ECDSAPublicKey, +} + +impl BitcoinTransactionSigner { + pub fn new(key_name: String, ecdsa_public_key: ECDSAPublicKey) -> Self { + Self { + key_name, + ecdsa_public_key, + } + } + + pub async fn sign_transaction( + &self, + unsigned_tx: crate::tx::UnsignedTransaction, + accounts: Vec, + runtime: &R, + ) -> Result { + use crate::address::{derivation_path, derive_public_key_from_account}; + + assert_eq!( + unsigned_tx.inputs.len(), + accounts.len(), + "BUG: expected on account per input" + ); + + let mut signed_inputs = Vec::with_capacity(unsigned_tx.inputs.len()); + let sighasher = tx::TxSigHasher::new(&unsigned_tx); + for (input, account) in unsigned_tx.inputs.iter().zip(accounts) { + let outpoint = &input.previous_output; + + let path = derivation_path(&account) + .into_iter() + .map(|buf| buf.to_vec()) + .collect(); + let pubkey = ByteBuf::from( + derive_public_key_from_account(&self.ecdsa_public_key, &account).public_key, + ); + let pkhash = tx::hash160(&pubkey); + + let sighash = sighasher.sighash(input, &pkhash); + + let sec1_signature = + management::sign_with_ecdsa(self.key_name.clone(), path, sighash, runtime).await?; + + signed_inputs.push(tx::SignedInput { + signature: signature::EncodedSignature::from_sec1(&sec1_signature), + pubkey, + previous_output: outpoint.clone(), + sequence: input.sequence, + }); + } + + let signed_tx = tx::SignedTransaction { + inputs: signed_inputs, + outputs: unsigned_tx.outputs, + lock_time: unsigned_tx.lock_time, + }; + Ok(SignedRawTransaction::new( + signed_tx.serialize(), + signed_tx.compute_txid(), + )) + } +} diff --git a/rs/bitcoin/ckbtc/minter/src/updates/tests.rs b/rs/bitcoin/ckbtc/minter/src/updates/tests.rs index 46df403711ae..401ec8520f50 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/tests.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/tests.rs @@ -17,7 +17,6 @@ mod update_balance { use crate::{CanisterRuntime, GetUtxosResponse, Timestamp}; use ic_btc_checker::{CheckTransactionResponse, CheckTransactionStatus}; use ic_btc_interface::Utxo; - use ic_management_canister_types_private::BoundedVec; use icrc_ledger_types::icrc1::account::Account; use std::iter; use std::time::Duration; @@ -640,7 +639,7 @@ mod update_balance { result: MetricsResult, ) -> Result, CallError> { let key_name = "test_key".to_string(); - let derivation_path = BoundedVec::new(vec![]); + let derivation_path = vec![]; let message_hash = [0u8; 32]; let mut runtime = MockCanisterRuntime::new(); diff --git a/rs/bitcoin/ckbtc/minter/tests/replay_events.rs b/rs/bitcoin/ckbtc/minter/tests/replay_events.rs index 8f5c24d1ac86..8bd9cd90568e 100644 --- a/rs/bitcoin/ckbtc/minter/tests/replay_events.rs +++ b/rs/bitcoin/ckbtc/minter/tests/replay_events.rs @@ -30,8 +30,9 @@ pub mod mock { use ic_ckbtc_minter::updates::retrieve_btc::BtcAddressCheckStatus; use ic_ckbtc_minter::updates::update_balance::UpdateBalanceError; use ic_ckbtc_minter::{ - CanisterRuntime, GetCurrentFeePercentilesRequest, GetUtxosRequest, GetUtxosResponse, - Network, state::eventlog::CkBtcEventLogger, tx, + CanisterRuntime, ECDSAPublicKey, GetCurrentFeePercentilesRequest, GetUtxosRequest, + GetUtxosResponse, Network, state::eventlog::CkBtcEventLogger, tx::SignedRawTransaction, + tx::UnsignedTransaction, }; use icrc_ledger_types::icrc1::account::Account; use icrc_ledger_types::icrc1::transfer::Memo; @@ -63,7 +64,7 @@ pub mod mock { async fn check_transaction(&self, btc_checker_principal: Option, utxo: &Utxo, cycle_payment: u128, ) -> Result; async fn mint_ckbtc(&self, amount: u64, to: Account, memo: Memo) -> Result; async fn sign_with_ecdsa(&self, key_name: String, derivation_path: Vec>, message_hash: [u8; 32]) -> Result, CallError>; - async fn send_transaction(&self, transaction: &tx::SignedTransaction, network: Network) -> Result<(), CallError>; + async fn sign_transaction( &self, key_name: String, ecdsa_public_key: ECDSAPublicKey, unsigned_tx: UnsignedTransaction, accounts: Vec) -> Result; async fn send_raw_transaction(&self, transaction: Vec, network: Network) -> Result<(), CallError>; async fn check_address( &self, btc_checker_principal: Option, address: String, ) -> Result; } diff --git a/rs/dogecoin/ckdoge/minter/BUILD.bazel b/rs/dogecoin/ckdoge/minter/BUILD.bazel index f04c4e84fe5b..66e29ad184f9 100644 --- a/rs/dogecoin/ckdoge/minter/BUILD.bazel +++ b/rs/dogecoin/ckdoge/minter/BUILD.bazel @@ -22,6 +22,7 @@ rust_library( "//packages/ic-secp256k1", "//packages/icrc-ledger-types:icrc_ledger_types", "//rs/bitcoin/ckbtc/minter:ckbtc_minter_lib", + "@crate_index//:bitcoin_dogecoin", "@crate_index//:bs58", "@crate_index//:candid", "@crate_index//:ciborium", @@ -50,10 +51,13 @@ rust_test( ], deps = [ # Keep sorted. + "//packages/ic-secp256k1", "//rs/types/base_types", "@crate_index//:assert_matches", + "@crate_index//:mockall", "@crate_index//:proptest", "@crate_index//:serde_json", + "@crate_index//:tokio", ], ) diff --git a/rs/dogecoin/ckdoge/minter/Cargo.toml b/rs/dogecoin/ckdoge/minter/Cargo.toml index 4e0fb25e4db5..b93eba823069 100644 --- a/rs/dogecoin/ckdoge/minter/Cargo.toml +++ b/rs/dogecoin/ckdoge/minter/Cargo.toml @@ -12,6 +12,7 @@ path = "src/main.rs" [dependencies] async-trait = { workspace = true } +bitcoin = {workspace = true} bs58 = "0.5.0" candid = { workspace = true } candid_parser = { workspace = true } @@ -30,10 +31,13 @@ assert_matches = { workspace = true } ic-base-types = { path = "../../../types/base_types" } ic-ckdoge-minter-test-utils = { path = "../test_utils" } ic-management-canister-types = { workspace = true } +ic-secp256k1 = { path = "../../../../packages/ic-secp256k1" } +mockall = { workspace = true } pocket-ic = { path = "../../../../packages/pocket-ic" } proptest = { workspace = true } serde_json = { workspace = true } test-strategy = "0.4.0" +tokio = { workspace = true } [features] self_check = [] diff --git a/rs/dogecoin/ckdoge/minter/src/address/mod.rs b/rs/dogecoin/ckdoge/minter/src/address/mod.rs index 3cf9a8a625d2..7df14102ef1c 100644 --- a/rs/dogecoin/ckdoge/minter/src/address/mod.rs +++ b/rs/dogecoin/ckdoge/minter/src/address/mod.rs @@ -115,7 +115,14 @@ impl DogecoinAddress { version_and_hash_to_address(prefix, self.as_array()) } - fn as_array(&self) -> &[u8; 20] { + pub fn as_array(&self) -> &[u8; 20] { + match self { + DogecoinAddress::P2pkh(data) => data, + DogecoinAddress::P2sh(data) => data, + } + } + + pub fn into_array(self) -> [u8; 20] { match self { DogecoinAddress::P2pkh(data) => data, DogecoinAddress::P2sh(data) => data, @@ -129,7 +136,8 @@ impl DogecoinAddress { } } - pub fn from_compressed_public_key(public_key: &[u8; 33]) -> Self { + /// Create a Dogecoin address from a **compressed** public key (33 bytes). + pub fn p2pkh_from_public_key(public_key: &[u8; 33]) -> Self { assert!(public_key[0] == 0x02 || public_key[0] == 0x03); DogecoinAddress::P2pkh(ic_ckbtc_minter::tx::hash160(public_key)) } diff --git a/rs/dogecoin/ckdoge/minter/src/fees/mod.rs b/rs/dogecoin/ckdoge/minter/src/fees/mod.rs index 4fbf52a485d4..491377308018 100644 --- a/rs/dogecoin/ckdoge/minter/src/fees/mod.rs +++ b/rs/dogecoin/ckdoge/minter/src/fees/mod.rs @@ -3,6 +3,7 @@ mod tests; use crate::candid_api::WithdrawalFee; use crate::lifecycle::init::Network; +use crate::transaction::DogecoinTransactionSigner; use crate::tx::UnsignedTransaction; use ic_ckbtc_minter::{ BuildTxError, MillisatoshiPerByte, Satoshi, address::BitcoinAddress, fees::FeeEstimator, @@ -119,7 +120,7 @@ impl FeeEstimator for DogecoinFeeEstimator { unsigned_tx: &UnsignedTransaction, fee_per_byte: u64, ) -> u64 { - let tx_size = ic_ckbtc_minter::fake_sign(unsigned_tx).serialized_len(); + let tx_size = DogecoinTransactionSigner::fake_sign(unsigned_tx).len(); (tx_size as u64 * fee_per_byte) / 1000 } diff --git a/rs/dogecoin/ckdoge/minter/src/lib.rs b/rs/dogecoin/ckdoge/minter/src/lib.rs index 5ff4af3d5823..1889f3745aa6 100644 --- a/rs/dogecoin/ckdoge/minter/src/lib.rs +++ b/rs/dogecoin/ckdoge/minter/src/lib.rs @@ -8,6 +8,7 @@ pub mod candid_api; pub mod event; pub mod fees; pub mod lifecycle; +pub mod transaction; pub mod updates; use crate::address::DogecoinAddress; @@ -15,13 +16,15 @@ use crate::dogecoin_canister::MillikoinuPerByte; use crate::event::CkDogeEventLogger; use crate::fees::DogecoinFeeEstimator; use crate::lifecycle::init::Network; +use crate::transaction::DogecoinTransactionSigner; use async_trait::async_trait; use candid::Principal; pub use dogecoin_canister::get_dogecoin_canister_id; use ic_cdk::management_canister::SignWithEcdsaArgs; +use ic_ckbtc_minter::tx::{SignedRawTransaction, UnsignedTransaction}; use ic_ckbtc_minter::{ - CanisterRuntime, CheckTransactionResponse, GetCurrentFeePercentilesRequest, GetUtxosRequest, - GetUtxosResponse, management::CallError, state::CkBtcMinterState, tx, + CanisterRuntime, CheckTransactionResponse, ECDSAPublicKey, GetCurrentFeePercentilesRequest, + GetUtxosRequest, GetUtxosResponse, management::CallError, state::CkBtcMinterState, tx, updates::retrieve_btc::BtcAddressCheckStatus, }; pub use ic_ckbtc_minter::{ @@ -97,6 +100,17 @@ impl CanisterRuntime for DogeCanisterRuntime { ic_ckbtc_minter::updates::update_balance::mint(amount, to, memo).await } + async fn sign_transaction( + &self, + key_name: String, + ecdsa_public_key: ECDSAPublicKey, + unsigned_tx: UnsignedTransaction, + accounts: Vec, + ) -> Result { + let signer = DogecoinTransactionSigner::new(key_name, ecdsa_public_key); + signer.sign_transaction(unsigned_tx, accounts, self).await + } + async fn sign_with_ecdsa( &self, key_name: String, @@ -116,19 +130,6 @@ impl CanisterRuntime for DogeCanisterRuntime { .map_err(CallError::from_sign_error) } - async fn send_transaction( - &self, - transaction: &tx::SignedTransaction, - network: ic_ckbtc_minter::Network, - ) -> Result<(), CallError> { - dogecoin_canister::dogecoin_send_transaction(&dogecoin_canister::SendTransactionRequest { - transaction: transaction.serialize(), - network: network.into(), - }) - .await - .map_err(|err| CallError::from_cdk_call_error("dogecoin_send_transaction", err)) - } - async fn send_raw_transaction( &self, transaction: Vec, diff --git a/rs/dogecoin/ckdoge/minter/src/test_fixtures/mock.rs b/rs/dogecoin/ckdoge/minter/src/test_fixtures/mock.rs new file mode 100644 index 000000000000..b787ca626c1e --- /dev/null +++ b/rs/dogecoin/ckdoge/minter/src/test_fixtures/mock.rs @@ -0,0 +1,46 @@ +use crate::{ + BitcoinAddress, BtcAddressCheckStatus, CanisterRuntime, CkDogeEventLogger, + GetCurrentFeePercentilesRequest, GetUtxosRequest, GetUtxosResponse, Utxo, + fees::DogecoinFeeEstimator, + tx::{SignedRawTransaction, UnsignedTransaction}, +}; +use async_trait::async_trait; +use candid::Principal; +use ic_ckbtc_minter::{ + CheckTransactionResponse, ECDSAPublicKey, management::CallError, state::CkBtcMinterState, + updates::update_balance::UpdateBalanceError, +}; +use icrc_ledger_types::icrc1::{account::Account, transfer::Memo}; +use mockall::mock; +use std::time::Duration; + +mock! { + #[derive(Debug)] + pub CanisterRuntime {} + + #[async_trait] + impl CanisterRuntime for CanisterRuntime { + type Estimator = DogecoinFeeEstimator; + type EventLogger = CkDogeEventLogger; + fn caller(&self) -> Principal; + fn id(&self) -> Principal; + fn time(&self) -> u64; + fn global_timer_set(&self, timestamp: u64); + fn parse_address(&self, address: &str, network: ic_ckbtc_minter::Network) -> Result; + fn block_time(&self, network: ic_ckbtc_minter::Network) -> Duration; + fn derive_user_address(&self, state: &CkBtcMinterState, account: &Account) -> String; + fn derive_minter_address(&self, state: &CkBtcMinterState) -> BitcoinAddress; + fn derive_minter_address_str(&self, state: &CkBtcMinterState) -> String; + fn refresh_fee_percentiles_frequency(&self) -> Duration; + fn fee_estimator(&self, state: &CkBtcMinterState) -> DogecoinFeeEstimator; + fn event_logger(&self) -> CkDogeEventLogger; + async fn get_current_fee_percentiles(&self, request: &GetCurrentFeePercentilesRequest) -> Result, CallError>; + async fn get_utxos(&self, request: &GetUtxosRequest) -> Result; + async fn check_transaction(&self, btc_checker_principal: Option, utxo: &Utxo, cycle_payment: u128, ) -> Result; + async fn mint_ckbtc(&self, amount: u64, to: Account, memo: Memo) -> Result; + async fn sign_with_ecdsa(&self, key_name: String, derivation_path: Vec>, message_hash: [u8; 32]) -> Result, CallError>; + async fn sign_transaction( &self, key_name: String, ecdsa_public_key: ECDSAPublicKey, unsigned_tx: UnsignedTransaction, accounts: Vec) -> Result; + async fn send_raw_transaction(&self, transaction: Vec, network: ic_ckbtc_minter::Network) -> Result<(), CallError>; + async fn check_address( &self, btc_checker_principal: Option, address: String) -> Result; + } +} diff --git a/rs/dogecoin/ckdoge/minter/src/test_fixtures/mod.rs b/rs/dogecoin/ckdoge/minter/src/test_fixtures/mod.rs index 5a0e4d9a5df8..6a0b7ca9d7fb 100644 --- a/rs/dogecoin/ckdoge/minter/src/test_fixtures/mod.rs +++ b/rs/dogecoin/ckdoge/minter/src/test_fixtures/mod.rs @@ -1,12 +1,41 @@ #[cfg(test)] mod tests; +use crate::address::DogecoinAddress; use crate::fees::DogecoinFeeEstimator; use crate::lifecycle::init::Network; +use ic_ckbtc_minter::ECDSAPublicKey; pub mod arbitrary; +pub mod mock; pub fn dogecoin_fee_estimator() -> DogecoinFeeEstimator { const RETRIEVE_DOGE_MIN_AMOUNT: u64 = 50 * 100_000_000; DogecoinFeeEstimator::new(Network::Mainnet, RETRIEVE_DOGE_MIN_AMOUNT) } + +pub fn dogecoin_address_to_bitcoin( + address: DogecoinAddress, +) -> ic_ckbtc_minter::address::BitcoinAddress { + match address { + DogecoinAddress::P2pkh(hash) => ic_ckbtc_minter::address::BitcoinAddress::P2pkh(hash), + DogecoinAddress::P2sh(hash) => ic_ckbtc_minter::address::BitcoinAddress::P2sh(hash), + } +} + +pub fn canister_public_key_pair() -> (ECDSAPublicKey, ic_secp256k1::PrivateKey) { + let canister_id = candid::Principal::from_text("ypu6c-niaaa-aaaar-qbzxa-cai").unwrap(); + let master_private_key = ic_secp256k1::PrivateKey::generate_from_seed(&[42; 32]); + let derivation_path = + ic_secp256k1::DerivationPath::from_canister_id_and_path(canister_id.as_slice(), &[]); + let (canister_private_key, chain_code) = master_private_key.derive_subkey(&derivation_path); + let canister_public_key = canister_private_key.public_key().serialize_sec1(true); + + ( + ECDSAPublicKey { + public_key: canister_public_key, + chain_code: chain_code.to_vec(), + }, + canister_private_key, + ) +} diff --git a/rs/dogecoin/ckdoge/minter/src/transaction/mod.rs b/rs/dogecoin/ckdoge/minter/src/transaction/mod.rs new file mode 100644 index 000000000000..265c185397ab --- /dev/null +++ b/rs/dogecoin/ckdoge/minter/src/transaction/mod.rs @@ -0,0 +1,153 @@ +#[cfg(test)] +mod tests; + +use crate::address::DogecoinAddress; +use ic_ckbtc_minter::{CanisterRuntime, ECDSAPublicKey, management, tx::SignedRawTransaction}; +use icrc_ledger_types::icrc1::account::Account; + +pub struct DogecoinTransactionSigner { + key_name: String, + ecdsa_public_key: ECDSAPublicKey, +} + +impl DogecoinTransactionSigner { + pub fn new(key_name: String, ecdsa_public_key: ECDSAPublicKey) -> Self { + Self { + key_name, + ecdsa_public_key, + } + } + + pub async fn sign_transaction( + &self, + unsigned_tx: ic_ckbtc_minter::tx::UnsignedTransaction, + accounts: Vec, + runtime: &R, + ) -> Result { + use bitcoin::hashes::Hash; + + assert_eq!( + unsigned_tx.inputs.len(), + accounts.len(), + "BUG: expected one account per input" + ); + + let dogecoin_tx = into_bitcoin_transaction(unsigned_tx); + let cache = bitcoin::sighash::SighashCache::new(&dogecoin_tx); + let mut script_sigs = Vec::with_capacity(accounts.len()); + let sighash_type = bitcoin::EcdsaSighashType::All; + + for (input_index, account) in accounts.into_iter().enumerate() { + let derivation_path = crate::updates::get_doge_address::derivation_path(&account); + let public_key = crate::updates::get_doge_address::derive_public_key( + &self.ecdsa_public_key, + &account, + ); + let address = DogecoinAddress::p2pkh_from_public_key(&public_key); + assert!( + matches!(address, DogecoinAddress::P2pkh(_)), + "BUG: expected P2PKH address. Other type of addresses would require other script_sig." + ); + let script_pubkey = bitcoin::ScriptBuf::new_p2pkh( + &bitcoin::PubkeyHash::from_byte_array(address.into_array()), + ); + let sighash = cache + .legacy_signature_hash(input_index, &script_pubkey, sighash_type.to_u32()) + .expect("BUG: sighash should not error"); + let sec1_signature = ic_ckbtc_minter::management::sign_with_ecdsa( + self.key_name.clone(), + derivation_path, + sighash.to_byte_array(), + runtime, + ) + .await?; + let mut signature = ic_ckbtc_minter::signature::sec1_to_der(&sec1_signature); + // The signature must end with a single byte indicating the SIGHASH type. + signature.push(sighash_type as u8); + debug_assert_eq!( + Ok(()), + ic_ckbtc_minter::signature::validate_encoded_signature(&signature) + ); + let sig_push_bytes: &bitcoin::script::PushBytes = signature + .as_slice() + .try_into() + .expect("BUG: validity check ensures signature contains at most 73 bytes"); + let script_sig = bitcoin::Script::builder() + .push_slice(sig_push_bytes) + .push_slice(public_key) + .into_script(); + script_sigs.push(script_sig); + } + + let mut signed_tx = dogecoin_tx; + signed_tx + .input + .iter_mut() + .zip(script_sigs) + .for_each(|(input, script_sig)| { + input.script_sig = script_sig; + }); + let txid = ic_ckbtc_minter::Txid::from(signed_tx.compute_txid().to_byte_array()); + + Ok(SignedRawTransaction::new( + bitcoin::consensus::encode::serialize(&signed_tx), + txid, + )) + } + + pub fn fake_sign(unsigned_tx: &ic_ckbtc_minter::tx::UnsignedTransaction) -> Vec { + const FAKE_PUBKEY: [u8; 33] = [0_u8; ic_ckbtc_minter::tx::PUBKEY_LEN]; + + let mut dogecoin_tx = into_bitcoin_transaction(unsigned_tx.clone()); + let max_size_script_sig = bitcoin::Script::builder() + .push_slice(ic_ckbtc_minter::signature::FAKE_SIG) + .push_slice(FAKE_PUBKEY) + .into_script(); + dogecoin_tx.input.iter_mut().for_each(|input| { + input.script_sig = max_size_script_sig.clone(); + }); + + bitcoin::consensus::encode::serialize(&dogecoin_tx) + } +} + +fn into_bitcoin_transaction( + unsigned_tx: ic_ckbtc_minter::tx::UnsignedTransaction, +) -> bitcoin::Transaction { + use bitcoin::hashes::Hash; + + bitcoin::Transaction { + // Dogecoin transactions use Version 1 (BIP-68 is not supported) + version: bitcoin::transaction::Version::ONE, + lock_time: bitcoin::absolute::LockTime::from_consensus(unsigned_tx.lock_time), + input: unsigned_tx + .inputs + .into_iter() + .map(|input| bitcoin::transaction::TxIn { + previous_output: bitcoin::transaction::OutPoint { + txid: bitcoin::Txid::from_byte_array(input.previous_output.txid.into()), + vout: input.previous_output.vout, + }, + script_sig: bitcoin::ScriptBuf::new(), + sequence: bitcoin::Sequence(input.sequence), + witness: bitcoin::Witness::default(), + }) + .collect(), + output: unsigned_tx + .outputs + .into_iter() + .map(|output| bitcoin::TxOut { + value: bitcoin::Amount::from_sat(output.value), + script_pubkey: match output.address { + ic_ckbtc_minter::address::BitcoinAddress::P2pkh(hash) => { + bitcoin::ScriptBuf::new_p2pkh(&bitcoin::PubkeyHash::from_byte_array(hash)) + } + ic_ckbtc_minter::address::BitcoinAddress::P2sh(hash) => { + bitcoin::ScriptBuf::new_p2sh(&bitcoin::ScriptHash::from_byte_array(hash)) + } + _ => panic!("BUG: Dogecoin does not support other address types"), + }, + }) + .collect(), + } +} diff --git a/rs/dogecoin/ckdoge/minter/src/transaction/tests.rs b/rs/dogecoin/ckdoge/minter/src/transaction/tests.rs new file mode 100644 index 000000000000..80bdc5aaa823 --- /dev/null +++ b/rs/dogecoin/ckdoge/minter/src/transaction/tests.rs @@ -0,0 +1,253 @@ +use crate::OutPoint; +use crate::address::DogecoinAddress; +use crate::lifecycle::init::Network; +use crate::test_fixtures::{dogecoin_address_to_bitcoin, mock::MockCanisterRuntime}; +use crate::transaction::DogecoinTransactionSigner; +use bitcoin::hashes::Hash; +use candid::Principal; +use ic_ckbtc_minter::Txid; +use ic_ckbtc_minter::tx::{TxOut, UnsignedInput, UnsignedTransaction}; +use icrc_ledger_types::icrc1::account::Account; + +#[tokio::test] +async fn should_be_noop_when_no_transactions() { + let runtime = MockCanisterRuntime::new(); + let (signer, _canister_private_key) = signer(); + let result = signer + .sign_transaction( + UnsignedTransaction { + inputs: vec![], + outputs: vec![], + lock_time: 0, + }, + vec![], + &runtime, + ) + .await + .unwrap(); + + let transaction: bitcoin::Transaction = + bitcoin::consensus::deserialize(result.as_ref()).unwrap(); + + assert_eq!( + transaction, + bitcoin::Transaction { + version: bitcoin::transaction::Version::ONE, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![], + output: vec![], + } + ); +} + +#[tokio::test] +async fn should_verify_signed_transaction() { + let (signer, canister_private_key) = signer(); + let chain_code: [u8; 32] = signer + .ecdsa_public_key + .chain_code + .clone() + .try_into() + .unwrap(); + let depositor = Account { + owner: Principal::from_text( + "2oyh2-miczk-rzcqm-zbkes-q3kyi-lmen7-slvvl-byown-zz6v6-razzx-vae", + ) + .unwrap(), + subaccount: Some([42_u8; 32]), + }; + let mut runtime = MockCanisterRuntime::new(); + runtime.expect_time().return_const(0_u64); + runtime + .expect_sign_with_ecdsa() + .times(1) + .withf(move |key_name, derivation_path, _message_hash| { + key_name == "key_1" + && derivation_path == &crate::updates::get_doge_address::derivation_path(&depositor) + }) + .returning(move |_key_name, derivation_path, message_hash| { + let account_private_key = canister_private_key + .derive_subkey_with_chain_code( + &ic_secp256k1::DerivationPath::new( + derivation_path + .into_iter() + .map(ic_secp256k1::DerivationIndex) + .collect(), + ), + &chain_code, + ) + .0; + Ok(account_private_key + .sign_digest_with_ecdsa(&message_hash) + .to_vec()) + }); + + let receiver = + DogecoinAddress::parse("D9Boe5MMx93BdZW1T94L4dyUUTfJqx8NFT", &Network::Mainnet).unwrap(); + let minter = + DogecoinAddress::parse("DJsTUj3DPhJG3GMDr66mqxnQGL7dF8N9eU", &Network::Mainnet).unwrap(); + let result = signer + .sign_transaction( + UnsignedTransaction { + inputs: vec![UnsignedInput { + previous_output: OutPoint { + txid: "a7612af24cd57190c18d1e5daa0e401754ab5ae41daf8f200ffc29408e1ae491" + .parse() + .unwrap(), + vout: 0, + }, + value: 13_785_800_000, + sequence: 0xFFFFFFFD, + }], + outputs: vec![ + TxOut { + value: 4_808_463_200, + address: dogecoin_address_to_bitcoin(receiver.clone()), + }, + TxOut { + value: 8_965_800_000, + address: dogecoin_address_to_bitcoin(minter.clone()), + }, + ], + lock_time: 0, + }, + vec![depositor], + &runtime, + ) + .await + .unwrap(); + + let transaction: bitcoin::Transaction = + bitcoin::consensus::deserialize(result.as_ref()).unwrap(); + + let public_key = + crate::updates::get_doge_address::derive_public_key(&signer.ecdsa_public_key, &depositor); + let signature: [u8; 72] = hex::decode("30450221008417fdd626ba643bc3300b7b2f77eced97cdcae4e93800d07a302711cd48e0b702204a211955b3eb5f60c8bcd82b1c3d8d003c1d2497a07d1d58898afbe67a4a916d01").unwrap().try_into().unwrap(); + assert_eq!( + transaction, + bitcoin::Transaction { + version: bitcoin::transaction::Version::ONE, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![bitcoin::TxIn { + previous_output: + "a7612af24cd57190c18d1e5daa0e401754ab5ae41daf8f200ffc29408e1ae491:0" + .parse() + .unwrap(), + script_sig: bitcoin::script::Builder::new() + .push_slice(signature) + .push_slice(public_key) + .into_script(), + sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, + witness: Default::default(), //no segwit + }], + output: vec![ + bitcoin::TxOut { + value: bitcoin::Amount::from_sat(4_808_463_200), + script_pubkey: bitcoin::ScriptBuf::new_p2pkh( + &bitcoin::PubkeyHash::from_byte_array(*receiver.as_array()) + ), + }, + bitcoin::TxOut { + value: bitcoin::Amount::from_sat(8_965_800_000), + script_pubkey: bitcoin::ScriptBuf::new_p2pkh( + &bitcoin::PubkeyHash::from_byte_array(*minter.as_array()) + ), + }, + ], + } + ); + + // Signature is DER-encoded. + // See BIP-0066: https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki + let sec1_signature: [u8; 64] = [&signature[5..=36], &signature[39..=70]] + .concat() + .try_into() + .unwrap(); + assert_eq!( + signature, + ic_ckbtc_minter::signature::EncodedSignature::from_sec1(&sec1_signature).as_slice() + ); + assert_eq!( + *signature.last().unwrap(), + bitcoin::EcdsaSighashType::All as u8 + ); + + // Verify signature is correct. + let depositor_address = DogecoinAddress::p2pkh_from_public_key(&public_key); + let cache = bitcoin::sighash::SighashCache::new(&transaction); + let sighash = cache + .legacy_signature_hash( + 0, + &bitcoin::ScriptBuf::new_p2pkh(&bitcoin::PubkeyHash::from_byte_array( + *depositor_address.as_array(), + )), + bitcoin::EcdsaSighashType::All.to_u32(), + ) + .expect("BUG: sighash should not error") + .to_byte_array(); + let account_public_key = ic_secp256k1::PublicKey::deserialize_sec1(&public_key).unwrap(); + assert!(account_public_key.verify_ecdsa_signature_prehashed(&sighash, &sec1_signature)) +} + +#[tokio::test] +async fn should_be_similar_to_fake_sign() { + // DOGE mainnet transaction [32d24dcb68fae3cac41caa55c9f9ed39eb4ee21689ba4d989c53df243b3b7364](https://chain.so/tx/DOGE/32d24dcb68fae3cac41caa55c9f9ed39eb4ee21689ba4d989c53df243b3b7364). + let signed_transaction: bitcoin::Transaction = bitcoin::consensus::deserialize(&hex::decode("010000000191e41a8e4029fc0f208faf1de45aab5417400eaa5d1e8dc19071d54cf22a61a7000000006b483045022100921b10e76fdb449fad2518ff321b9072842775f020a1fc3713283bc1bf94f2ff02200f51a76a40c0d2778c44e89ed56d757feac8231db9668631366ba385606adf35012103c0ba3fcf0ac8219fef80d979dcc5bacf6a77be5637191364bb1b70f0275d4275fdffffff0260539b1e010000001976a9142c63a4d417d41515cf1f6de60831d578ad8a0f9588ac40406716020000001976a914969c95abfe91b2019cc64be25920830ce516558688ac00000000").unwrap()).unwrap(); + let unsigned_transaction = UnsignedTransaction { + inputs: signed_transaction + .input + .clone() + .into_iter() + .map(|input| UnsignedInput { + previous_output: OutPoint { + txid: Txid::from(input.previous_output.txid.to_byte_array()), + vout: input.previous_output.vout, + }, + value: 0, //not relevant + sequence: input.sequence.0, + }) + .collect(), + outputs: signed_transaction + .output + .clone() + .into_iter() + .map(|output| TxOut { + value: output.value.to_sat(), + address: ic_ckbtc_minter::address::BitcoinAddress::parse( + &bitcoin::Address::from_script( + &output.script_pubkey, + bitcoin::Network::Bitcoin, + ) + .unwrap() + .to_string(), + ic_ckbtc_minter::Network::Mainnet, + ) + .unwrap(), + }) + .collect(), + lock_time: 0, + }; + + let fake_signed_transaction: bitcoin::Transaction = bitcoin::consensus::deserialize( + &DogecoinTransactionSigner::fake_sign(&unsigned_transaction), + ) + .unwrap(); + + assert_eq!( + signed_transaction.compute_ntxid(), + fake_signed_transaction.compute_ntxid() + ); + let signed_tx_len = bitcoin::consensus::encode::serialize(&signed_transaction).len(); + let fake_signed_tx_len = bitcoin::consensus::encode::serialize(&fake_signed_transaction).len(); + let error_margin = signed_tx_len / 20; // 5% + assert!( + signed_tx_len <= fake_signed_tx_len && fake_signed_tx_len <= signed_tx_len + error_margin + ); +} + +fn signer() -> (DogecoinTransactionSigner, ic_secp256k1::PrivateKey) { + let (canister_public_key, canister_private_key) = + crate::test_fixtures::canister_public_key_pair(); + let signer = DogecoinTransactionSigner::new("key_1".to_string(), canister_public_key); + (signer, canister_private_key) +} diff --git a/rs/dogecoin/ckdoge/minter/src/updates/get_doge_address.rs b/rs/dogecoin/ckdoge/minter/src/updates/get_doge_address.rs index eeec1ae86647..d646a0028fa8 100644 --- a/rs/dogecoin/ckdoge/minter/src/updates/get_doge_address.rs +++ b/rs/dogecoin/ckdoge/minter/src/updates/get_doge_address.rs @@ -32,16 +32,12 @@ pub fn account_to_p2pkh_address_from_state( .as_ref() .cloned() .expect("bug: the ECDSA public key must be initialized"); - let public_key: [u8; 33] = derive_public_key(&ecdsa_public_key, account) - .public_key - .try_into() - .expect("BUG: invalid ECDSA compressed public key"); - DogecoinAddress::from_compressed_public_key(&public_key) + DogecoinAddress::p2pkh_from_public_key(&derive_public_key(&ecdsa_public_key, account)) } /// Returns the derivation path that should be used to sign a message from a /// specified account. -fn derivation_path(account: &Account) -> Vec> { +pub fn derivation_path(account: &Account) -> Vec> { const SCHEMA_V1: u8 = 1; const PREFIX: [u8; 4] = *b"doge"; @@ -53,7 +49,7 @@ fn derivation_path(account: &Account) -> Vec> { ] } -fn derive_public_key(ecdsa_public_key: &ECDSAPublicKey, account: &Account) -> ECDSAPublicKey { +pub fn derive_public_key(ecdsa_public_key: &ECDSAPublicKey, account: &Account) -> [u8; 33] { use ic_secp256k1::{DerivationIndex, DerivationPath}; let path = DerivationPath::new( @@ -63,4 +59,7 @@ fn derive_public_key(ecdsa_public_key: &ECDSAPublicKey, account: &Account) -> EC .collect(), ); ic_ckbtc_minter::address::derive_public_key(ecdsa_public_key, &path) + .public_key + .try_into() + .expect("BUG: invalid ECDSA compressed public key") } diff --git a/rs/dogecoin/ckdoge/minter/src/updates/mod.rs b/rs/dogecoin/ckdoge/minter/src/updates/mod.rs index 6d367e01041b..0d22d6ba78cc 100644 --- a/rs/dogecoin/ckdoge/minter/src/updates/mod.rs +++ b/rs/dogecoin/ckdoge/minter/src/updates/mod.rs @@ -1,3 +1,6 @@ +#[cfg(test)] +mod tests; + pub mod get_doge_address; pub use get_doge_address::{account_to_p2pkh_address_from_state, get_doge_address}; diff --git a/rs/dogecoin/ckdoge/minter/src/updates/tests.rs b/rs/dogecoin/ckdoge/minter/src/updates/tests.rs new file mode 100644 index 000000000000..8b43d0e21ef4 --- /dev/null +++ b/rs/dogecoin/ckdoge/minter/src/updates/tests.rs @@ -0,0 +1,72 @@ +mod derivation { + use crate::address::DogecoinAddress; + use crate::lifecycle::init::Network; + use crate::updates::get_doge_address::{derivation_path, derive_public_key}; + use candid::Principal; + use icrc_ledger_types::icrc1::account::Account; + + #[test] + fn should_be_stable() { + let (canister_public_key, _) = crate::test_fixtures::canister_public_key_pair(); + let user_with_subaccount = Account { + owner: Principal::from_text( + "2oyh2-miczk-rzcqm-zbkes-q3kyi-lmen7-slvvl-byown-zz6v6-razzx-vae", + ) + .unwrap(), + subaccount: Some([42_u8; 32]), + }; + let user_without_subaccount = Account::from(user_with_subaccount.owner); + + assert_eq!( + derivation_path(&user_with_subaccount), + vec![ + vec![1], + b"doge".to_vec(), + hex::decode("02caa39141990a89286d5842d846fe4bad561c3acdce7d5f4419cdea02").unwrap(), + hex::decode("2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a") + .unwrap(), + ] + ); + let derived_public_key = derive_public_key(&canister_public_key, &user_with_subaccount); + assert_eq!( + derived_public_key.to_vec(), + hex::decode("03e62317d6e4feb57c8d5face3f16d26abbc30609e9abd38fc8c7e3f04502f36cc") + .unwrap() + ); + let derived_address = DogecoinAddress::p2pkh_from_public_key(&derived_public_key); + assert_eq!( + derived_address.display(&Network::Mainnet), + "DSdZym6ZBa4QNPnE7jpuryF6fRtVmvGgre" + ); + assert_eq!( + derived_address.display(&Network::Regtest), + "n31RjZEthBbNcW5F6ioj98HpeHkuJsPBJm" + ); + + assert_eq!( + derivation_path(&user_without_subaccount), + vec![ + vec![1], + b"doge".to_vec(), + hex::decode("02caa39141990a89286d5842d846fe4bad561c3acdce7d5f4419cdea02").unwrap(), + hex::decode("0000000000000000000000000000000000000000000000000000000000000000") + .unwrap(), + ] + ); + let derived_public_key = derive_public_key(&canister_public_key, &user_without_subaccount); + assert_eq!( + derived_public_key.to_vec(), + hex::decode("02db987e631a12327a64695d96f7efaf355554633f8cdc37e1570a97b303cb8de8") + .unwrap() + ); + let derived_address = DogecoinAddress::p2pkh_from_public_key(&derived_public_key); + assert_eq!( + derived_address.display(&Network::Mainnet), + "D7BZ4HNX9W1KSYv8gXi6yTSRg8Zwh6AFKw" + ); + assert_eq!( + derived_address.display(&Network::Regtest), + "mhZQp5Wrf7YHgfD9fWgvFcV9ezSMJeHaNC" + ); + } +} diff --git a/rs/dogecoin/ckdoge/minter/tests/tests.rs b/rs/dogecoin/ckdoge/minter/tests/tests.rs index 20462392fbf7..81b832e5b92f 100644 --- a/rs/dogecoin/ckdoge/minter/tests/tests.rs +++ b/rs/dogecoin/ckdoge/minter/tests/tests.rs @@ -477,7 +477,7 @@ fn should_estimate_withdrawal_fee() { let expected_fee = WithdrawalFee { minter_fee: 180_000_000, - dogecoin_fee: 11_450_000, + dogecoin_fee: 11_350_000, }; assert_eq!( estimate_withdrawal_fee_and_check(&minter, RETRIEVE_DOGE_MIN_AMOUNT), diff --git a/rs/dogecoin/ckdoge/test_utils/src/dogecoin.rs b/rs/dogecoin/ckdoge/test_utils/src/dogecoin.rs index df9e1515b7dd..f4e6ec5350ea 100644 --- a/rs/dogecoin/ckdoge/test_utils/src/dogecoin.rs +++ b/rs/dogecoin/ckdoge/test_utils/src/dogecoin.rs @@ -53,8 +53,6 @@ impl DogecoinCanister { } pub fn mempool(&self) -> Mempool { - use bitcoin::consensus::Decodable; - fn vec_to_txid(vec: Vec) -> Txid { let bytes: [u8; 32] = vec.try_into().expect("Vector length must be exactly 32"); bytes.into() @@ -73,8 +71,7 @@ impl DogecoinCanister { response .into_iter() .map(|tx_bytes| { - let tx = bitcoin::Transaction::consensus_decode(&mut &tx_bytes[..]) - .expect("failed to parse a dogecoin transaction"); + let tx = decode_dogecoin_transaction(&tx_bytes); (vec_to_txid(tx.compute_txid().as_byte_array().to_vec()), tx) }) @@ -101,3 +98,22 @@ impl DogecoinCanister { } } } + +fn decode_dogecoin_transaction(tx_bytes: &[u8]) -> bitcoin::Transaction { + use bitcoin::consensus::Decodable; + + let tx = bitcoin::Transaction::consensus_decode(&mut &tx_bytes[..]) + .expect("failed to parse a dogecoin transaction"); + assert_eq!( + tx.version, + bitcoin::transaction::Version::ONE, + "Dogecoin does not support BIP-68" + ); + for input in &tx.input { + assert!( + input.witness.is_empty() && !input.script_sig.is_empty(), + "Dogecoin does not support segwit" + ); + } + tx +}