From 45d6711002baed195e1a5e4d6c50675751ddb83e Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:43:55 +0200 Subject: [PATCH 01/22] Revert "chore: reset rescan db" This reverts commit 71348560da52f03dee4b668e3c08cd22dd340cc6. --- vault/src/issue.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vault/src/issue.rs b/vault/src/issue.rs index 51e6fa764..ccb9b7dee 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -86,7 +86,7 @@ struct RescanStatus { impl RescanStatus { // there was a bug pre-v2 that set rescanning status to an invalid range. // by changing the keyname we effectively force a reset - const KEY: &str = "rescan-status-v4"; + const KEY: &str = "rescan-status-v3"; fn update(&mut self, mut issues: Vec, current_bitcoin_height: usize) { // Only look at issues that haven't been processed yet issues.retain(|issue| issue.opentime > self.newest_issue_height); From 21442d85fb6ac12844abf45ececc33bab75bd7b3 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:02 +0200 Subject: [PATCH 02/22] Revert "chore: use 0 minconf for listunspent" This reverts commit 9d8d669772579922d05b8d6f19fd6200e9dc7453. --- bitcoin/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 05da3fd28..4651c2f37 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -1064,7 +1064,7 @@ impl BitcoinCoreApi for BitcoinCore { } async fn sweep_funds(&self, address: Address) -> Result { - let unspent = self.rpc.list_unspent(Some(0), None, None, None, None)?; + let unspent = self.rpc.list_unspent(None, None, None, None, None)?; let mut amount = Amount::ZERO; let mut utxos = Vec::::new(); From 4b8b27a4c1b03046b91930d7c771349825167d44 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:08 +0200 Subject: [PATCH 03/22] Revert "chore: rescan issues for old shared wallet" This reverts commit 238f8477682c5d35c5e1c126a2ad64d08bc849c6. --- vault/src/issue.rs | 79 +-------------------------------------------- vault/src/system.rs | 14 ++------ 2 files changed, 3 insertions(+), 90 deletions(-) diff --git a/vault/src/issue.rs b/vault/src/issue.rs index ccb9b7dee..4d9ee5d4b 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -144,84 +144,7 @@ impl RescanStatus { } } -pub async fn add_keys_from_past_issue_request_old( - bitcoin_core: &DynBitcoinCoreApi, - btc_parachain: &InterBtcParachain, - db: &DatabaseConfig, -) -> Result<(), Error> { - let account_id = btc_parachain.get_account_id(); - let mut scanning_status = RescanStatus::get(&account_id, db)?; - tracing::info!("Scanning: {scanning_status:?}"); - - let issue_requests = btc_parachain.get_vault_issue_requests(account_id.clone()).await?; - - for (issue_id, request) in issue_requests.clone().into_iter() { - if let Err(e) = add_new_deposit_key(bitcoin_core, issue_id, request.btc_public_key).await { - tracing::error!("Failed to add deposit key #{}: {}", issue_id, e.to_string()); - } - } - - // read height only _after_ the last add_new_deposit_key. If a new block arrives - // while we rescan, bitcoin core will correctly recognize addressed associated with the - // privkey - let btc_end_height = bitcoin_core.get_block_count().await? as usize; - let btc_pruned_start_height = bitcoin_core.get_pruned_height().await? as usize; - - let issues = issue_requests.clone().into_iter().map(|(_key, issue)| issue).collect(); - scanning_status.update(issues, btc_end_height); - - // use electrs to scan the portion that is not scannable by bitcoin core - if let Some((start, end)) = scanning_status.prune(btc_pruned_start_height) { - tracing::info!( - "Also checking electrs for issue requests between {} and {}...", - start, - end - ); - bitcoin_core - .rescan_electrs_for_addresses( - issue_requests - .into_iter() - .filter_map(|(_, request)| { - // only import if BEFORE current pruning height - if (request.btc_height as usize) < btc_pruned_start_height { - Some(request.btc_address.to_address(bitcoin_core.network()).ok()?) - } else { - None - } - }) - .collect(), - ) - .await?; - } - - // save progress s.t. we don't rescan pruned range again if we crash now - scanning_status.store(account_id, db)?; - - let mut chunk_size = 1; - // rescan the blockchain in chunks, so that we can save progress. The code below - // aims to have each chunk take about 10 seconds (arbitrarily chosen value). - while let Some((chunk_start, chunk_end)) = scanning_status.process_blocks(chunk_size) { - tracing::info!("Rescanning bitcoin chain from {} to {}...", chunk_start, chunk_end); - - let start_time = Instant::now(); - - bitcoin_core.rescan_blockchain(chunk_start, chunk_end).await?; - - // with the code below the rescan time should remain between 5 and 20 seconds - // after the first couple of rounds. - if start_time.elapsed() < Duration::from_secs(10) { - chunk_size = chunk_size.saturating_mul(2); - } else { - chunk_size = (chunk_size.checked_div(2).ok_or(Error::ArithmeticUnderflow)?).max(1); - } - - scanning_status.store(account_id, db)?; - } - - Ok(()) -} - -pub async fn add_keys_from_past_issue_request_new( +pub async fn add_keys_from_past_issue_request( bitcoin_core: &DynBitcoinCoreApi, btc_parachain: &InterBtcParachain, db: &DatabaseConfig, diff --git a/vault/src/system.rs b/vault/src/system.rs index b61ed0146..bce894e03 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -459,7 +459,6 @@ impl VaultIdManager { pub struct VaultService { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, @@ -572,7 +571,6 @@ impl VaultService { Self { btc_parachain: btc_parachain.clone(), btc_rpc_master_wallet: btc_rpc_master_wallet.clone(), - btc_rpc_shared_wallet: btc_rpc_shared_wallet.clone(), btc_rpc_shared_wallet_v2: btc_rpc_shared_wallet_v2.clone(), config, monitoring_config, @@ -694,18 +692,10 @@ impl VaultService { }?; // purposefully _after_ maybe_register_vault and _before_ other calls - self.vault_id_manager.fetch_vault_ids(self.config.only_migrate).await?; + self.vault_id_manager.fetch_vault_ids().await?; tracing::info!("Adding keys from past issues..."); - issue::add_keys_from_past_issue_request_old( - &self.btc_rpc_shared_wallet, - &self.btc_parachain, - &self.vault_id_manager.db, - ) - .await?; - - self.vault_id_manager.sweep_shared_wallet().await?; - issue::add_keys_from_past_issue_request_new( + issue::add_keys_from_past_issue_request( &self.btc_rpc_shared_wallet_v2, &self.btc_parachain, &self.vault_id_manager.db, From 7d2c7e55f0d8594fada21f16ca2343ff6490f302 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:15 +0200 Subject: [PATCH 04/22] Revert "chore: update tests" This reverts commit 00c7d92f3edf56b0a5f58616c19493a8d0e9b9f2. --- vault/tests/vault_integration_tests.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/vault/tests/vault_integration_tests.rs b/vault/tests/vault_integration_tests.rs index fd195a48e..1c249c3e8 100644 --- a/vault/tests/vault_integration_tests.rs +++ b/vault/tests/vault_integration_tests.rs @@ -96,7 +96,6 @@ async fn test_redeem_succeeds() { let vault_id_manager = VaultIdManager::from_map( vault_provider.clone(), btc_rpc_master_wallet.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_redeem_succeeds", @@ -167,7 +166,6 @@ async fn test_replace_succeeds() { let _vault_id_manager = VaultIdManager::from_map( new_vault_provider.clone(), new_btc_rpc_master_wallet.clone(), - new_btc_rpc_master_wallet.clone(), new_btc_rpc_master_wallet, btc_rpcs, "test_replace_succeeds1", @@ -182,7 +180,6 @@ async fn test_replace_succeeds() { let vault_id_manager = VaultIdManager::from_map( old_vault_provider.clone(), old_btc_rpc_master_wallet.clone(), - old_btc_rpc_master_wallet.clone(), old_btc_rpc_master_wallet, btc_rpcs, "test_replace_succeeds2", @@ -358,7 +355,6 @@ async fn test_cancellation_succeeds() { let vault_id_manager = VaultIdManager::from_map( new_vault_provider.clone(), new_btc_rpc_master_wallet.clone(), - new_btc_rpc_master_wallet.clone(), new_btc_rpc_master_wallet, btc_rpcs, "test_cancellation_succeeds", @@ -628,7 +624,6 @@ async fn test_automatic_issue_execution_succeeds() { let vault_id_manager = VaultIdManager::from_map( vault2_provider.clone(), btc_rpc_master_wallet.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_automatic_issue_execution_succeeds", @@ -729,7 +724,6 @@ async fn test_automatic_issue_execution_succeeds_with_big_transaction() { let vault_id_manager = VaultIdManager::from_map( vault2_provider.clone(), btc_rpc_master_wallet.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_automatic_issue_execution_succeeds_with_big_transaction", @@ -819,7 +813,6 @@ async fn test_execute_open_requests_succeeds() { let vault_id_manager = VaultIdManager::from_map( vault_provider.clone(), btc_rpc_master_wallet.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_execute_open_requests_succeeds", @@ -1221,7 +1214,6 @@ mod test_with_bitcoind { let vault_id_manager = VaultIdManager::from_map( vault_provider.clone(), btc_rpc_master_wallet.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_automatic_rbf_succeeds", From 78e4818fe58cabc7664957010c262925d87f9152 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:27 +0200 Subject: [PATCH 05/22] Revert "chore: add missing methods to replace mock" This reverts commit 475f6dc97791862ba064c43c0949ee66ace8e82d. --- vault/src/replace.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/vault/src/replace.rs b/vault/src/replace.rs index d7e413dd8..37d61d2a0 100644 --- a/vault/src/replace.rs +++ b/vault/src/replace.rs @@ -249,8 +249,6 @@ mod tests { async fn get_block_hash(&self, height: u32) -> Result; async fn get_pruned_height(&self) -> Result; async fn get_new_address(&self) -> Result; - async fn get_new_sweep_address(&self) -> Result; - async fn get_last_sweep_height(&self) -> Result, BitcoinError>; async fn get_new_public_key(&self) -> Result; fn dump_private_key(&self, address: &Address) -> Result; fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError>; From 1bf15a0528f438571dea7adbaf49e11088b8e6f8 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:37 +0200 Subject: [PATCH 06/22] Revert "chore: clippy" This reverts commit 273aa65673dce501609097242f3d6dd42e2153b3. --- vault/src/system.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vault/src/system.rs b/vault/src/system.rs index bce894e03..a622bf752 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -380,10 +380,11 @@ impl VaultIdManager { // only run AFTER the separate currency wallet sweeps async fn sweep_shared_wallet(&self) -> Result<(), Error> { - if self.btc_rpc_shared_wallet.get_pruned_height().await? == 0 - || self.btc_rpc_shared_wallet_v2.get_last_sweep_height().await?.is_some() - { - // no need to sweep, full node can rescan or already has sweep tx + if self.btc_rpc_shared_wallet.get_pruned_height().await? == 0 { + // no need to sweep, full node can rescan + return Ok(()); + } else if self.btc_rpc_shared_wallet_v2.get_last_sweep_height().await?.is_some() { + // already has sweep tx return Ok(()); } From 9c4cd17b82d7b63444724f61f59a52824f92de33 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:46 +0200 Subject: [PATCH 07/22] Revert "chore: wait for last sweep tx inclusion" This reverts commit c5b78e4e35be9e3ec77dab56a27da026eb5b1104. --- bitcoin/src/iter.rs | 2 +- bitcoin/src/lib.rs | 7 ++++--- bitcoin/src/light/mod.rs | 4 ++-- runtime/src/integration/bitcoin_simulator.rs | 4 ++-- vault/src/execution.rs | 2 +- vault/src/metrics.rs | 2 +- vault/src/replace.rs | 2 +- vault/src/system.rs | 12 ++---------- 8 files changed, 14 insertions(+), 21 deletions(-) diff --git a/bitcoin/src/iter.rs b/bitcoin/src/iter.rs index 94994c153..93de8ad41 100644 --- a/bitcoin/src/iter.rs +++ b/bitcoin/src/iter.rs @@ -253,7 +253,7 @@ mod tests { fee_rate: SatPerVbyte, num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; + async fn sweep_funds(&self, address: Address) -> Result<(), Error>; async fn create_or_load_wallet(&self) -> Result<(), Error>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), Error>; async fn rescan_electrs_for_addresses( diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 4651c2f37..0ed6390c1 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -213,7 +213,7 @@ pub trait BitcoinCoreApi { num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; + async fn sweep_funds(&self, address: Address) -> Result<(), Error>; async fn create_or_load_wallet(&self) -> Result<(), Error>; @@ -1063,7 +1063,7 @@ impl BitcoinCoreApi for BitcoinCore { .await?) } - async fn sweep_funds(&self, address: Address) -> Result { + async fn sweep_funds(&self, address: Address) -> Result<(), Error> { let unspent = self.rpc.list_unspent(None, None, None, None, None)?; let mut amount = Amount::ZERO; @@ -1118,9 +1118,10 @@ impl BitcoinCoreApi for BitcoinCore { let transaction = signed_funded_raw_tx.transaction()?; let txid = self.rpc.send_raw_transaction(&transaction)?; + log::info!("Sent sweep tx: {txid}"); - Ok(txid) + Ok(()) } /// Create or load a wallet on Bitcoin Core. diff --git a/bitcoin/src/light/mod.rs b/bitcoin/src/light/mod.rs index bb5324f85..1f98434b5 100644 --- a/bitcoin/src/light/mod.rs +++ b/bitcoin/src/light/mod.rs @@ -339,8 +339,8 @@ impl BitcoinCoreApi for BitcoinLight { .await?) } - async fn sweep_funds(&self, _address: Address) -> Result { - Ok(Txid::all_zeros()) + async fn sweep_funds(&self, _address: Address) -> Result<(), BitcoinError> { + Ok(()) } async fn create_or_load_wallet(&self) -> Result<(), BitcoinError> { diff --git a/runtime/src/integration/bitcoin_simulator.rs b/runtime/src/integration/bitcoin_simulator.rs index 36891a013..4950fe80a 100644 --- a/runtime/src/integration/bitcoin_simulator.rs +++ b/runtime/src/integration/bitcoin_simulator.rs @@ -523,8 +523,8 @@ impl BitcoinCoreApi for MockBitcoinCore { .unwrap(); Ok(metadata) } - async fn sweep_funds(&self, _address: Address) -> Result { - Ok(Txid::all_zeros()) + async fn sweep_funds(&self, _address: Address) -> Result<(), BitcoinError> { + Ok(()) } async fn create_or_load_wallet(&self) -> Result<(), BitcoinError> { Ok(()) diff --git a/vault/src/execution.rs b/vault/src/execution.rs index 26bdb2128..71db81d6c 100644 --- a/vault/src/execution.rs +++ b/vault/src/execution.rs @@ -822,7 +822,7 @@ mod tests { async fn wait_for_transaction_metadata(&self, txid: Txid, num_confirmations: u32, block_hash: Option, is_wallet: bool) -> Result; async fn create_and_send_transaction(&self, address: Address, sat: u64, fee_rate: SatPerVbyte, request_id: Option) -> Result; async fn send_to_address(&self, address: Address, sat: u64, request_id: Option, fee_rate: SatPerVbyte, num_confirmations: u32) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; + async fn sweep_funds(&self, address: Address) -> Result<(), BitcoinError>; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index 9a37b7003..77929a21b 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -795,7 +795,7 @@ mod tests { async fn wait_for_transaction_metadata(&self, txid: Txid, num_confirmations: u32, block_hash: Option, is_wallet: bool) -> Result; async fn create_and_send_transaction(&self, address: Address, sat: u64, fee_rate: SatPerVbyte, request_id: Option) -> Result; async fn send_to_address(&self, address: Address, sat: u64, request_id: Option, fee_rate: SatPerVbyte, num_confirmations: u32) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; + async fn sweep_funds(&self, address: Address) -> Result<(), BitcoinError>; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/replace.rs b/vault/src/replace.rs index 37d61d2a0..c3ed6a771 100644 --- a/vault/src/replace.rs +++ b/vault/src/replace.rs @@ -285,7 +285,7 @@ mod tests { fee_rate: SatPerVbyte, num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; + async fn sweep_funds(&self, address: Address) -> Result<(), BitcoinError>; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/system.rs b/vault/src/system.rs index a622bf752..8322aa911 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -378,7 +378,6 @@ impl VaultIdManager { Ok(()) } - // only run AFTER the separate currency wallet sweeps async fn sweep_shared_wallet(&self) -> Result<(), Error> { if self.btc_rpc_shared_wallet.get_pruned_height().await? == 0 { // no need to sweep, full node can rescan @@ -390,15 +389,8 @@ impl VaultIdManager { // sweep funds from shared wallet to shared-v2 let shared_v2_wallet_address = self.btc_rpc_shared_wallet_v2.get_new_sweep_address().await?; - match self.btc_rpc_shared_wallet.sweep_funds(shared_v2_wallet_address).await { - Ok(txid) => { - self.btc_rpc_shared_wallet - .wait_for_transaction_metadata(txid, 1, None, true) - .await?; - } - Err(err) => { - tracing::error!("Could not sweep funds: {err}"); - } + if let Err(err) = self.btc_rpc_shared_wallet.sweep_funds(shared_v2_wallet_address).await { + tracing::error!("Could not sweep funds: {err}"); } Ok(()) From 613ffcb40367496ec4fd236e02396f34f839582b Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:53 +0200 Subject: [PATCH 08/22] Revert "refactor: don't fail if we can't sweep funds" This reverts commit 83472c45ef1e07d1cdce99c8a34bf867998f40b3. --- bitcoin/src/iter.rs | 2 +- bitcoin/src/lib.rs | 10 +++------- bitcoin/src/light/mod.rs | 4 ++-- runtime/src/integration/bitcoin_simulator.rs | 4 ++-- vault/src/execution.rs | 2 +- vault/src/metrics.rs | 2 +- vault/src/replace.rs | 2 +- vault/src/system.rs | 14 +++++--------- 8 files changed, 16 insertions(+), 24 deletions(-) diff --git a/bitcoin/src/iter.rs b/bitcoin/src/iter.rs index 93de8ad41..94994c153 100644 --- a/bitcoin/src/iter.rs +++ b/bitcoin/src/iter.rs @@ -253,7 +253,7 @@ mod tests { fee_rate: SatPerVbyte, num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result<(), Error>; + async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), Error>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), Error>; async fn rescan_electrs_for_addresses( diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 0ed6390c1..c214ff02f 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -213,7 +213,7 @@ pub trait BitcoinCoreApi { num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result<(), Error>; + async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), Error>; @@ -1063,7 +1063,7 @@ impl BitcoinCoreApi for BitcoinCore { .await?) } - async fn sweep_funds(&self, address: Address) -> Result<(), Error> { + async fn sweep_funds(&self, address: Address) -> Result { let unspent = self.rpc.list_unspent(None, None, None, None, None)?; let mut amount = Amount::ZERO; @@ -1117,11 +1117,7 @@ impl BitcoinCoreApi for BitcoinCore { } let transaction = signed_funded_raw_tx.transaction()?; - let txid = self.rpc.send_raw_transaction(&transaction)?; - - log::info!("Sent sweep tx: {txid}"); - - Ok(()) + self.rpc.send_raw_transaction(&transaction).map_err(Into::into) } /// Create or load a wallet on Bitcoin Core. diff --git a/bitcoin/src/light/mod.rs b/bitcoin/src/light/mod.rs index 1f98434b5..bb5324f85 100644 --- a/bitcoin/src/light/mod.rs +++ b/bitcoin/src/light/mod.rs @@ -339,8 +339,8 @@ impl BitcoinCoreApi for BitcoinLight { .await?) } - async fn sweep_funds(&self, _address: Address) -> Result<(), BitcoinError> { - Ok(()) + async fn sweep_funds(&self, _address: Address) -> Result { + Ok(Txid::all_zeros()) } async fn create_or_load_wallet(&self) -> Result<(), BitcoinError> { diff --git a/runtime/src/integration/bitcoin_simulator.rs b/runtime/src/integration/bitcoin_simulator.rs index 4950fe80a..36891a013 100644 --- a/runtime/src/integration/bitcoin_simulator.rs +++ b/runtime/src/integration/bitcoin_simulator.rs @@ -523,8 +523,8 @@ impl BitcoinCoreApi for MockBitcoinCore { .unwrap(); Ok(metadata) } - async fn sweep_funds(&self, _address: Address) -> Result<(), BitcoinError> { - Ok(()) + async fn sweep_funds(&self, _address: Address) -> Result { + Ok(Txid::all_zeros()) } async fn create_or_load_wallet(&self) -> Result<(), BitcoinError> { Ok(()) diff --git a/vault/src/execution.rs b/vault/src/execution.rs index 71db81d6c..26bdb2128 100644 --- a/vault/src/execution.rs +++ b/vault/src/execution.rs @@ -822,7 +822,7 @@ mod tests { async fn wait_for_transaction_metadata(&self, txid: Txid, num_confirmations: u32, block_hash: Option, is_wallet: bool) -> Result; async fn create_and_send_transaction(&self, address: Address, sat: u64, fee_rate: SatPerVbyte, request_id: Option) -> Result; async fn send_to_address(&self, address: Address, sat: u64, request_id: Option, fee_rate: SatPerVbyte, num_confirmations: u32) -> Result; - async fn sweep_funds(&self, address: Address) -> Result<(), BitcoinError>; + async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index 77929a21b..9a37b7003 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -795,7 +795,7 @@ mod tests { async fn wait_for_transaction_metadata(&self, txid: Txid, num_confirmations: u32, block_hash: Option, is_wallet: bool) -> Result; async fn create_and_send_transaction(&self, address: Address, sat: u64, fee_rate: SatPerVbyte, request_id: Option) -> Result; async fn send_to_address(&self, address: Address, sat: u64, request_id: Option, fee_rate: SatPerVbyte, num_confirmations: u32) -> Result; - async fn sweep_funds(&self, address: Address) -> Result<(), BitcoinError>; + async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/replace.rs b/vault/src/replace.rs index c3ed6a771..37d61d2a0 100644 --- a/vault/src/replace.rs +++ b/vault/src/replace.rs @@ -285,7 +285,7 @@ mod tests { fee_rate: SatPerVbyte, num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result<(), BitcoinError>; + async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/system.rs b/vault/src/system.rs index 8322aa911..3c8c78f32 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -329,14 +329,11 @@ impl VaultIdManager { } // only sweep if using pruned node and there is no sweep tx yet to shared-v2 - if btc_rpc_shared.get_pruned_height().await? != 0 - && self.btc_rpc_shared_wallet_v2.get_last_sweep_height().await?.is_none() - { + if btc_rpc_shared.get_pruned_height().await? != 0 && self.btc_rpc_shared_wallet_v2.get_last_sweep_height().await?.is_none() { // sweep to old shared wallet which will then sweep again to the v2 wallet let shared_wallet_address = btc_rpc_shared.get_new_address().await?; - if let Err(err) = btc_rpc.sweep_funds(shared_wallet_address).await { - tracing::error!("Could not sweep funds: {err}"); - } + let txid = btc_rpc.sweep_funds(shared_wallet_address).await?; + tracing::info!("Sent sweep tx: {txid}"); } tracing::info!("Initializing metrics..."); @@ -389,9 +386,8 @@ impl VaultIdManager { // sweep funds from shared wallet to shared-v2 let shared_v2_wallet_address = self.btc_rpc_shared_wallet_v2.get_new_sweep_address().await?; - if let Err(err) = self.btc_rpc_shared_wallet.sweep_funds(shared_v2_wallet_address).await { - tracing::error!("Could not sweep funds: {err}"); - } + let txid = self.btc_rpc_shared_wallet.sweep_funds(shared_v2_wallet_address).await?; + tracing::info!("Sent sweep tx: {txid}"); Ok(()) } From 75b0ccdd2c0d1d1d2ba4af84c4c92662fb3e176b Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:44:59 +0200 Subject: [PATCH 09/22] Revert "chore: import derivation key to v2 shared wallet" This reverts commit 82542593a37252a277893523f8bd65c874099dd3. --- vault/src/system.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/vault/src/system.rs b/vault/src/system.rs index 3c8c78f32..41171be60 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -309,7 +309,6 @@ impl VaultIdManager { Ok(private_key) => { // TODO: remove this after the migration is complete btc_rpc_shared.import_private_key(&private_key, true)?; - self.btc_rpc_shared_wallet_v2.import_private_key(&private_key, true)?; } Err(err) => { tracing::error!("Could not find the derivation key in the bitcoin wallet"); From 9837577cf85f013c74b345fc618376605ec3f59a Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:45:07 +0200 Subject: [PATCH 10/22] Revert "fix: also sweep from currency specific wallets" This reverts commit c0c94fae4bd2e25876bb0fdc313b54ba68a349b8. --- bitcoin/src/lib.rs | 7 ++----- vault/src/issue.rs | 4 ++-- vault/src/lib.rs | 1 + vault/src/system.rs | 14 +++----------- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index c214ff02f..6439ad9e0 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -811,11 +811,7 @@ impl BitcoinCoreApi for BitcoinCore { .rpc .list_transactions(Some(SWEEP_ADDRESS), Some(DEFAULT_MAX_TX_COUNT), None, None)? .into_iter() - // we want to return None if there is no sweep tx for full nodes or new - // pruned nodes and we should return an error if any tx is still in the mempool - .map(|tx| tx.info.blockheight.ok_or(Error::ConfirmationError)) - .collect::, _>>()? - .into_iter() + .filter_map(|tx| tx.info.blockheight) .min()) } @@ -1084,6 +1080,7 @@ impl BitcoinCoreApi for BitcoinCore { } log::info!("Sweeping {} from {} utxos", amount, utxos.len()); + let mut outputs = serde_json::Map::::new(); outputs.insert(address.to_string(), serde_json::Value::from(amount.to_btc())); diff --git a/vault/src/issue.rs b/vault/src/issue.rs index 4d9ee5d4b..6f135ee10 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -166,7 +166,7 @@ pub async fn add_keys_from_past_issue_request( // privkey let btc_end_height = bitcoin_core.get_block_count().await? as usize; let btc_pruned_start_height = bitcoin_core.get_pruned_height().await? as usize; - let btc_last_sweep_height = bitcoin_core.get_last_sweep_height().await?; + let btc_max_sweep_height = bitcoin_core.get_last_sweep_height().await?; let issues = issue_requests.clone().into_iter().map(|(_key, issue)| issue).collect(); scanning_status.update(issues, btc_end_height); @@ -184,7 +184,7 @@ pub async fn add_keys_from_past_issue_request( .into_iter() .filter_map(|(_, request)| { // only import if address is AFTER last sweep height and BEFORE current pruning height - if btc_last_sweep_height.map_or(true, |sweep_height| request.btc_height > sweep_height) + if btc_max_sweep_height.is_some_and(|sweep_height| request.btc_height > sweep_height) && (request.btc_height as usize) < btc_pruned_start_height { Some(request.btc_address.to_address(bitcoin_core.network()).ok()?) diff --git a/vault/src/lib.rs b/vault/src/lib.rs index 8dac97932..a93685643 100644 --- a/vault/src/lib.rs +++ b/vault/src/lib.rs @@ -1,5 +1,6 @@ #![recursion_limit = "256"] #![feature(array_zip)] +#![feature(is_some_and)] mod cancellation; mod cli; diff --git a/vault/src/system.rs b/vault/src/system.rs index 41171be60..79d766219 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -327,14 +327,6 @@ impl VaultIdManager { btc_rpc_shared.import_private_key(&private_key, false)?; } - // only sweep if using pruned node and there is no sweep tx yet to shared-v2 - if btc_rpc_shared.get_pruned_height().await? != 0 && self.btc_rpc_shared_wallet_v2.get_last_sweep_height().await?.is_none() { - // sweep to old shared wallet which will then sweep again to the v2 wallet - let shared_wallet_address = btc_rpc_shared.get_new_address().await?; - let txid = btc_rpc.sweep_funds(shared_wallet_address).await?; - tracing::info!("Sent sweep tx: {txid}"); - } - tracing::info!("Initializing metrics..."); let metrics = PerCurrencyMetrics::new(&vault_id); let data = VaultData { @@ -447,7 +439,7 @@ impl VaultIdManager { pub struct VaultService { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, + btc_rpc_shared_wallet: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -559,7 +551,7 @@ impl VaultService { Self { btc_parachain: btc_parachain.clone(), btc_rpc_master_wallet: btc_rpc_master_wallet.clone(), - btc_rpc_shared_wallet_v2: btc_rpc_shared_wallet_v2.clone(), + btc_rpc_shared_wallet: btc_rpc_shared_wallet.clone(), config, monitoring_config, shutdown, @@ -684,7 +676,7 @@ impl VaultService { tracing::info!("Adding keys from past issues..."); issue::add_keys_from_past_issue_request( - &self.btc_rpc_shared_wallet_v2, + &self.btc_rpc_shared_wallet, &self.btc_parachain, &self.vault_id_manager.db, ) From f4fdd095a293051c851822b8b5c3cd06a11c3930 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:45:19 +0200 Subject: [PATCH 11/22] Revert "chore: add more logging" This reverts commit 156c9641633287a651653dd990b2b2af0f3a609a. --- bitcoin/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 6439ad9e0..1ba838dcf 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -1067,7 +1067,6 @@ impl BitcoinCoreApi for BitcoinCore { for entry in unspent { if self.electrs_client.is_tx_output_spent(&entry.txid, entry.vout).await? { - log::info!("{}:{} already spent", entry.txid, entry.vout); // skip if already spent continue; } @@ -1079,8 +1078,6 @@ impl BitcoinCoreApi for BitcoinCore { }) } - log::info!("Sweeping {} from {} utxos", amount, utxos.len()); - let mut outputs = serde_json::Map::::new(); outputs.insert(address.to_string(), serde_json::Value::from(amount.to_btc())); From 3822cb5e9228e73d3c4a6625caceb3a081207bc8 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:45:24 +0200 Subject: [PATCH 12/22] Revert "fix: sweep funds into shared wallet v2" This reverts commit a86c737a93453798af2549f8c398be97a399e818. --- bitcoin/src/electrs/mod.rs | 5 - bitcoin/src/electrs/types.rs | 12 --- bitcoin/src/iter.rs | 3 - bitcoin/src/lib.rs | 99 ++++---------------- bitcoin/src/light/mod.rs | 12 --- bitcoin/tests/integration_test.rs | 55 +---------- runtime/src/integration/bitcoin_simulator.rs | 13 --- vault/src/connection_manager.rs | 5 - vault/src/execution.rs | 3 - vault/src/issue.rs | 6 +- vault/src/lib.rs | 1 - vault/src/metrics.rs | 3 - vault/src/replace.rs | 1 - vault/src/system.rs | 37 ++------ 14 files changed, 30 insertions(+), 225 deletions(-) diff --git a/bitcoin/src/electrs/mod.rs b/bitcoin/src/electrs/mod.rs index 7092a5646..536d3defe 100644 --- a/bitcoin/src/electrs/mod.rs +++ b/bitcoin/src/electrs/mod.rs @@ -105,11 +105,6 @@ impl ElectrsClient { Ok(ret) } - pub async fn is_tx_output_spent(&self, txid: &Txid, vout: u32) -> Result { - let spending_value: SpendingValue = self.get_and_decode(&format!("/tx/{txid}/outspend/{vout}")).await?; - Ok(spending_value.spent) - } - pub async fn get_blocks_tip_height(&self) -> Result { Ok(self.get("/blocks/tip/height").await?.parse()?) } diff --git a/bitcoin/src/electrs/types.rs b/bitcoin/src/electrs/types.rs index bb72552d7..83586a20d 100644 --- a/bitcoin/src/electrs/types.rs +++ b/bitcoin/src/electrs/types.rs @@ -66,15 +66,3 @@ pub struct UtxoValue { pub status: TransactionStatus, pub value: u64, } - -// https://github.com/Blockstream/electrs/blob/adedee15f1fe460398a7045b292604df2161adc0/src/rest.rs#L448-L457 -#[derive(Deserialize)] -pub struct SpendingValue { - pub spent: bool, - #[serde(skip_serializing_if = "Option::is_none")] - pub txid: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub vin: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub status: Option, -} diff --git a/bitcoin/src/iter.rs b/bitcoin/src/iter.rs index 94994c153..81dba865e 100644 --- a/bitcoin/src/iter.rs +++ b/bitcoin/src/iter.rs @@ -214,8 +214,6 @@ mod tests { async fn get_proof(&self, txid: Txid, block_hash: &BlockHash) -> Result, Error>; async fn get_block_hash(&self, height: u32) -> Result; async fn get_new_address(&self) -> Result; - async fn get_new_sweep_address(&self) -> Result; - async fn get_last_sweep_height(&self) -> Result, Error>; async fn get_new_public_key(&self) -> Result; fn dump_private_key(&self, address: &Address) -> Result; fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), Error>; @@ -253,7 +251,6 @@ mod tests { fee_rate: SatPerVbyte, num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), Error>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), Error>; async fn rescan_electrs_for_addresses( diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 1ba838dcf..0ef0c1b6b 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -101,8 +101,6 @@ const RANDOMIZATION_FACTOR: f64 = 0.25; const DERIVATION_KEY_LABEL: &str = "derivation-key"; const DEPOSIT_LABEL: &str = "deposit"; -const SWEEP_ADDRESS: &str = "sweep-address"; - fn get_exponential_backoff() -> ExponentialBackoff { ExponentialBackoff { current_interval: INITIAL_INTERVAL, @@ -162,10 +160,6 @@ pub trait BitcoinCoreApi { async fn get_new_address(&self) -> Result; - async fn get_new_sweep_address(&self) -> Result; - - async fn get_last_sweep_height(&self) -> Result, Error>; - async fn get_new_public_key(&self) -> Result; fn dump_private_key(&self, address: &Address) -> Result; @@ -213,8 +207,6 @@ pub trait BitcoinCoreApi { num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; - async fn create_or_load_wallet(&self) -> Result<(), Error>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), Error>; @@ -370,7 +362,7 @@ impl BitcoinCoreBuilder { #[derive(Clone)] pub struct BitcoinCore { - pub rpc: Arc, + rpc: Arc, wallet_name: Option, network: Network, transaction_creation_lock: Arc>, @@ -799,22 +791,6 @@ impl BitcoinCoreApi for BitcoinCore { .require_network(self.network)?) } - async fn get_new_sweep_address(&self) -> Result { - Ok(self - .rpc - .get_new_address(Some(SWEEP_ADDRESS), Some(AddressType::Bech32))? - .require_network(self.network)?) - } - - async fn get_last_sweep_height(&self) -> Result, Error> { - Ok(self - .rpc - .list_transactions(Some(SWEEP_ADDRESS), Some(DEFAULT_MAX_TX_COUNT), None, None)? - .into_iter() - .filter_map(|tx| tx.info.blockheight) - .min()) - } - /// Gets a new public key for an address in the wallet async fn get_new_public_key(&self) -> Result { let address = self @@ -1059,61 +1035,6 @@ impl BitcoinCoreApi for BitcoinCore { .await?) } - async fn sweep_funds(&self, address: Address) -> Result { - let unspent = self.rpc.list_unspent(None, None, None, None, None)?; - - let mut amount = Amount::ZERO; - let mut utxos = Vec::::new(); - - for entry in unspent { - if self.electrs_client.is_tx_output_spent(&entry.txid, entry.vout).await? { - // skip if already spent - continue; - } - amount += entry.amount; - utxos.push(json::CreateRawTransactionInput { - txid: entry.txid, - vout: entry.vout, - sequence: None, - }) - } - - let mut outputs = serde_json::Map::::new(); - outputs.insert(address.to_string(), serde_json::Value::from(amount.to_btc())); - - let args = [ - serde_json::to_value::<&[json::CreateRawTransactionInput]>(&utxos)?, - serde_json::to_value(outputs)?, - serde_json::to_value(0i64)?, /* locktime - default 0: see https://developer.bitcoin.org/reference/rpc/createrawtransaction.html */ - serde_json::to_value(true)?, // BIP125-replaceable, aka Replace By Fee (RBF) - ]; - let raw_tx: String = self.rpc.call("createrawtransaction", &args)?; - - let funding_opts = FundRawTransactionOptions { - fee_rate: None, - add_inputs: Some(false), - subtract_fee_from_outputs: Some(vec![0]), - ..Default::default() - }; - let funded_raw_tx = self.rpc.fund_raw_transaction(raw_tx, Some(&funding_opts), None)?; - - let signed_funded_raw_tx = - self.rpc - .sign_raw_transaction_with_wallet(&funded_raw_tx.transaction()?, None, None)?; - - if signed_funded_raw_tx.errors.is_some() { - log::warn!( - "Received bitcoin funding errors (complete={}): {:?}", - signed_funded_raw_tx.complete, - signed_funded_raw_tx.errors - ); - return Err(Error::TransactionSigningError); - } - - let transaction = signed_funded_raw_tx.transaction()?; - self.rpc.send_raw_transaction(&transaction).map_err(Into::into) - } - /// Create or load a wallet on Bitcoin Core. async fn create_or_load_wallet(&self) -> Result<(), Error> { let wallet_name = if let Some(ref wallet_name) = self.wallet_name { @@ -1197,6 +1118,24 @@ impl BitcoinCoreApi for BitcoinCore { &[serde_json::to_value(raw_tx)?, serde_json::to_value(raw_merkle_proof)?], )?; } + // TODO: remove this migration after the next runtime upgrade + // filter to remove spent funds, the previous wallet migration caused + // signing failures for pruned nodes because they tried to double spend + let confirmed_payments_from = all_transactions.iter().filter(|tx| { + if let Some(status) = &tx.status { + if !status.confirmed { + return false; + } + }; + tx.vin + .iter() + .filter_map(|input| input.prevout.clone()) + .any(|output| matches!(&output.scriptpubkey_address, Some(addr) if addr == &address)) + }); + for transaction in confirmed_payments_from { + self.rpc + .call("removeprunedfunds", &[serde_json::to_value(transaction.txid)?])?; + } } Ok(()) } diff --git a/bitcoin/src/light/mod.rs b/bitcoin/src/light/mod.rs index bb5324f85..66f20231e 100644 --- a/bitcoin/src/light/mod.rs +++ b/bitcoin/src/light/mod.rs @@ -168,14 +168,6 @@ impl BitcoinCoreApi for BitcoinLight { Ok(self.get_change_address()?) } - async fn get_new_sweep_address(&self) -> Result { - Ok(self.get_change_address()?) - } - - async fn get_last_sweep_height(&self) -> Result, BitcoinError> { - Ok(None) - } - async fn get_new_public_key(&self) -> Result { Ok(self.private_key.public_key(&self.secp_ctx)) } @@ -339,10 +331,6 @@ impl BitcoinCoreApi for BitcoinLight { .await?) } - async fn sweep_funds(&self, _address: Address) -> Result { - Ok(Txid::all_zeros()) - } - async fn create_or_load_wallet(&self) -> Result<(), BitcoinError> { // nothing to do Ok(()) diff --git a/bitcoin/tests/integration_test.rs b/bitcoin/tests/integration_test.rs index 6a36f2355..938f6d63b 100644 --- a/bitcoin/tests/integration_test.rs +++ b/bitcoin/tests/integration_test.rs @@ -1,10 +1,6 @@ #![cfg(feature = "uses-bitcoind")] -use bitcoin::{ - Amount, Auth, BitcoinCore, BitcoinCoreApi, BitcoinCoreBuilder, Error, Network, PrivateKey, PublicKey, RpcApi, -}; -use bitcoincore_rpc::json; -use rand::{distributions::Alphanumeric, Rng}; +use bitcoin::{Auth, BitcoinCore, BitcoinCoreApi, BitcoinCoreBuilder, Error, Network, PrivateKey, PublicKey}; use regex::Regex; use std::env::var; @@ -93,52 +89,3 @@ async fn should_add_new_deposit_key() -> Result<(), Error> { Ok(()) } - -fn rand_wallet_name() -> String { - rand::thread_rng() - .sample_iter(&Alphanumeric) - .take(7) - .map(char::from) - .collect() -} - -#[tokio::test] -async fn should_sweep_funds() -> Result<(), Error> { - let btc_rpc1 = new_bitcoin_core(Some(rand_wallet_name()))?; - btc_rpc1.create_or_load_wallet().await?; - - let btc_rpc2 = new_bitcoin_core(Some(rand_wallet_name()))?; - btc_rpc2.create_or_load_wallet().await?; - - let btc_rpc3 = new_bitcoin_core(Some(rand_wallet_name()))?; - btc_rpc3.create_or_load_wallet().await?; - - // blocks must have 100 confirmations for reward to be spent - let address1 = btc_rpc1.get_new_address().await?; - btc_rpc1.mine_blocks(101, Some(address1)); - - let address2 = btc_rpc2.get_new_address().await?; - let txid = btc_rpc1.rpc.send_to_address( - &address2, - Amount::from_sat(100000), - None, - None, - None, - None, - None, - Some(json::EstimateMode::Economical), - )?; - btc_rpc1.mine_blocks(1, None); - - assert_eq!(btc_rpc2.get_balance(None)?.to_sat(), 100000); - - let address3 = btc_rpc3.get_new_address().await?; - let _txid = btc_rpc2.sweep_funds(address3).await?; - btc_rpc1.mine_blocks(1, None); - - assert_eq!(btc_rpc2.get_balance(None)?.to_sat(), 0); - // balance before minus fees - assert_eq!(btc_rpc3.get_balance(None)?.to_sat(), 97800); - - Ok(()) -} diff --git a/runtime/src/integration/bitcoin_simulator.rs b/runtime/src/integration/bitcoin_simulator.rs index 36891a013..96838c98f 100644 --- a/runtime/src/integration/bitcoin_simulator.rs +++ b/runtime/src/integration/bitcoin_simulator.rs @@ -405,15 +405,6 @@ impl BitcoinCoreApi for MockBitcoinCore { let address = BtcAddress::P2PKH(H160::from(bytes)); Ok(address.to_address(Network::Regtest)?) } - - async fn get_new_sweep_address(&self) -> Result { - self.get_new_address().await - } - - async fn get_last_sweep_height(&self) -> Result, BitcoinError> { - Ok(None) - } - async fn get_new_public_key(&self) -> Result { let secp = Secp256k1::new(); let raw_secret_key: [u8; SECRET_KEY_SIZE] = thread_rng().gen(); @@ -523,9 +514,6 @@ impl BitcoinCoreApi for MockBitcoinCore { .unwrap(); Ok(metadata) } - async fn sweep_funds(&self, _address: Address) -> Result { - Ok(Txid::all_zeros()) - } async fn create_or_load_wallet(&self) -> Result<(), BitcoinError> { Ok(()) } @@ -536,7 +524,6 @@ impl BitcoinCoreApi for MockBitcoinCore { async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError> { Ok(()) } - fn get_utxo_count(&self) -> Result { Ok(0) } diff --git a/vault/src/connection_manager.rs b/vault/src/connection_manager.rs index 10fb679ce..070edc9c8 100644 --- a/vault/src/connection_manager.rs +++ b/vault/src/connection_manager.rs @@ -28,7 +28,6 @@ pub trait Service { btc_parachain: BtcParachain, bitcoin_core_master: DynBitcoinCoreApi, bitcoin_core_shared: DynBitcoinCoreApi, - bitcoin_core_shared_v2: DynBitcoinCoreApi, config: Config, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -108,9 +107,6 @@ impl ConnectionManager { let bitcoin_core_shared = config_copy.new_client_with_network(Some(format!("{prefix}-shared")), network_copy)?; bitcoin_core_shared.create_or_load_wallet().await?; - let bitcoin_core_shared_v2 = - config_copy.new_client_with_network(Some(format!("{prefix}-shared-v2")), network_copy)?; - bitcoin_core_shared_v2.create_or_load_wallet().await?; let constructor = move |vault_id: VaultId| { let collateral_currency: CurrencyId = vault_id.collateral_currency(); @@ -132,7 +128,6 @@ impl ConnectionManager { btc_parachain, bitcoin_core_master, bitcoin_core_shared, - bitcoin_core_shared_v2, config, self.monitoring_config.clone(), shutdown_tx.clone(), diff --git a/vault/src/execution.rs b/vault/src/execution.rs index 26bdb2128..f774defb8 100644 --- a/vault/src/execution.rs +++ b/vault/src/execution.rs @@ -809,8 +809,6 @@ mod tests { async fn get_block_hash(&self, height: u32) -> Result; async fn get_pruned_height(&self) -> Result; async fn get_new_address(&self) -> Result; - async fn get_new_sweep_address(&self) -> Result; - async fn get_last_sweep_height(&self) -> Result, BitcoinError>; async fn get_new_public_key(&self) -> Result; fn dump_private_key(&self, address: &Address) -> Result; fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError>; @@ -822,7 +820,6 @@ mod tests { async fn wait_for_transaction_metadata(&self, txid: Txid, num_confirmations: u32, block_hash: Option, is_wallet: bool) -> Result; async fn create_and_send_transaction(&self, address: Address, sat: u64, fee_rate: SatPerVbyte, request_id: Option) -> Result; async fn send_to_address(&self, address: Address, sat: u64, request_id: Option, fee_rate: SatPerVbyte, num_confirmations: u32) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/issue.rs b/vault/src/issue.rs index 6f135ee10..9bc8dea54 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -166,7 +166,6 @@ pub async fn add_keys_from_past_issue_request( // privkey let btc_end_height = bitcoin_core.get_block_count().await? as usize; let btc_pruned_start_height = bitcoin_core.get_pruned_height().await? as usize; - let btc_max_sweep_height = bitcoin_core.get_last_sweep_height().await?; let issues = issue_requests.clone().into_iter().map(|(_key, issue)| issue).collect(); scanning_status.update(issues, btc_end_height); @@ -183,10 +182,7 @@ pub async fn add_keys_from_past_issue_request( issue_requests .into_iter() .filter_map(|(_, request)| { - // only import if address is AFTER last sweep height and BEFORE current pruning height - if btc_max_sweep_height.is_some_and(|sweep_height| request.btc_height > sweep_height) - && (request.btc_height as usize) < btc_pruned_start_height - { + if (request.btc_height as usize) < btc_pruned_start_height { Some(request.btc_address.to_address(bitcoin_core.network()).ok()?) } else { None diff --git a/vault/src/lib.rs b/vault/src/lib.rs index a93685643..8dac97932 100644 --- a/vault/src/lib.rs +++ b/vault/src/lib.rs @@ -1,6 +1,5 @@ #![recursion_limit = "256"] #![feature(array_zip)] -#![feature(is_some_and)] mod cancellation; mod cli; diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index 9a37b7003..d98bd533a 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -782,8 +782,6 @@ mod tests { async fn get_block_hash(&self, height: u32) -> Result; async fn get_pruned_height(&self) -> Result; async fn get_new_address(&self) -> Result; - async fn get_new_sweep_address(&self) -> Result; - async fn get_last_sweep_height(&self) -> Result, BitcoinError>; async fn get_new_public_key(&self) -> Result; fn dump_private_key(&self, address: &Address) -> Result; fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError>; @@ -795,7 +793,6 @@ mod tests { async fn wait_for_transaction_metadata(&self, txid: Txid, num_confirmations: u32, block_hash: Option, is_wallet: bool) -> Result; async fn create_and_send_transaction(&self, address: Address, sat: u64, fee_rate: SatPerVbyte, request_id: Option) -> Result; async fn send_to_address(&self, address: Address, sat: u64, request_id: Option, fee_rate: SatPerVbyte, num_confirmations: u32) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/replace.rs b/vault/src/replace.rs index 37d61d2a0..36b31669f 100644 --- a/vault/src/replace.rs +++ b/vault/src/replace.rs @@ -285,7 +285,6 @@ mod tests { fee_rate: SatPerVbyte, num_confirmations: u32, ) -> Result; - async fn sweep_funds(&self, address: Address) -> Result; async fn create_or_load_wallet(&self) -> Result<(), BitcoinError>; async fn rescan_blockchain(&self, start_height: usize, end_height: usize) -> Result<(), BitcoinError>; async fn rescan_electrs_for_addresses(&self, addresses: Vec
) -> Result<(), BitcoinError>; diff --git a/vault/src/system.rs b/vault/src/system.rs index 79d766219..892a733e0 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -219,7 +219,6 @@ pub struct VaultIdManager { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, pub(crate) btc_rpc_shared_wallet: DynBitcoinCoreApi, - pub(crate) btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, // TODO: remove this #[allow(clippy::type_complexity)] constructor: Arc Result + Send + Sync>>, @@ -231,7 +230,6 @@ impl VaultIdManager { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, btc_rpc_shared_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, constructor: impl Fn(VaultId) -> Result + Send + Sync + 'static, db_path: String, ) -> Self { @@ -240,7 +238,6 @@ impl VaultIdManager { constructor: Arc::new(Box::new(constructor)), btc_rpc_master_wallet, btc_rpc_shared_wallet, - btc_rpc_shared_wallet_v2, btc_parachain, db: DatabaseConfig { path: db_path }, } @@ -251,7 +248,6 @@ impl VaultIdManager { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, btc_rpc_shared_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, map: HashMap, db_path: &str, ) -> Self { @@ -273,7 +269,6 @@ impl VaultIdManager { constructor: Arc::new(Box::new(|_| unimplemented!())), btc_rpc_master_wallet, btc_rpc_shared_wallet, - btc_rpc_shared_wallet_v2, btc_parachain, db: DatabaseConfig { path: db_path.to_string(), @@ -317,9 +312,10 @@ impl VaultIdManager { } tracing::info!("Merging wallet for {:?}", vault_id); + let all_addresses = btc_rpc.list_addresses()?; // issue keys should be imported separately but we need to iterate // through currency specific wallets to get change addresses - for address in btc_rpc.list_addresses()? { + for address in &all_addresses { tracing::info!("Found {:?}", address); // get private key from currency specific wallet let private_key = btc_rpc.dump_private_key(&address)?; @@ -327,11 +323,17 @@ impl VaultIdManager { btc_rpc_shared.import_private_key(&private_key, false)?; } + if btc_rpc_shared.get_pruned_height().await? != 0 { + // rescan via electrs to import or remove change utxos + // this is required because pruned nodes cannot rescan themselves + btc_rpc_shared.rescan_electrs_for_addresses(all_addresses).await?; + } + tracing::info!("Initializing metrics..."); let metrics = PerCurrencyMetrics::new(&vault_id); let data = VaultData { vault_id: vault_id.clone(), - btc_rpc: self.btc_rpc_shared_wallet_v2.clone(), + btc_rpc: btc_rpc_shared, metrics: metrics.clone(), }; PerCurrencyMetrics::initialize_values(self.btc_parachain.clone(), &data).await; @@ -366,23 +368,6 @@ impl VaultIdManager { Ok(()) } - async fn sweep_shared_wallet(&self) -> Result<(), Error> { - if self.btc_rpc_shared_wallet.get_pruned_height().await? == 0 { - // no need to sweep, full node can rescan - return Ok(()); - } else if self.btc_rpc_shared_wallet_v2.get_last_sweep_height().await?.is_some() { - // already has sweep tx - return Ok(()); - } - - // sweep funds from shared wallet to shared-v2 - let shared_v2_wallet_address = self.btc_rpc_shared_wallet_v2.get_new_sweep_address().await?; - let txid = self.btc_rpc_shared_wallet.sweep_funds(shared_v2_wallet_address).await?; - tracing::info!("Sent sweep tx: {txid}"); - - Ok(()) - } - pub async fn listen_for_vault_id_registrations(self) -> Result<(), Error> { Ok(self .btc_parachain @@ -455,7 +440,6 @@ impl Service for VaultService { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, btc_rpc_shared_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -466,7 +450,6 @@ impl Service for VaultService { btc_parachain, btc_rpc_master_wallet, btc_rpc_shared_wallet, - btc_rpc_shared_wallet_v2, config, monitoring_config, shutdown, @@ -541,7 +524,6 @@ impl VaultService { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, btc_rpc_shared_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet_v2: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -559,7 +541,6 @@ impl VaultService { btc_parachain, btc_rpc_master_wallet, btc_rpc_shared_wallet, - btc_rpc_shared_wallet_v2, constructor, db_path, ), From 501b687693deaef6c4f49cb8360ce46b4a77c100 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:46:00 +0200 Subject: [PATCH 13/22] Revert "fix: migration for pruned nodes" This reverts commit 5870ce0c6db59e2184e2bbafbf72b364142faf53. --- bitcoin/src/electrs/types.rs | 4 ++-- bitcoin/src/lib.rs | 25 +------------------------ vault/src/system.rs | 9 +-------- 3 files changed, 4 insertions(+), 34 deletions(-) diff --git a/bitcoin/src/electrs/types.rs b/bitcoin/src/electrs/types.rs index 83586a20d..5320a3c61 100644 --- a/bitcoin/src/electrs/types.rs +++ b/bitcoin/src/electrs/types.rs @@ -15,7 +15,7 @@ pub struct TransactionStatus { } // https://github.com/Blockstream/electrs/blob/adedee15f1fe460398a7045b292604df2161adc0/src/rest.rs#L167-L189 -#[derive(Deserialize, Clone)] +#[derive(Deserialize)] pub struct TxInValue { pub txid: Txid, pub vout: u32, @@ -33,7 +33,7 @@ pub struct TxInValue { } // https://github.com/Blockstream/electrs/blob/adedee15f1fe460398a7045b292604df2161adc0/src/rest.rs#L239-L270 -#[derive(Deserialize, Clone)] +#[derive(Deserialize)] pub struct TxOutValue { pub scriptpubkey: ScriptBuf, pub scriptpubkey_asm: String, diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 0ef0c1b6b..fe5148c30 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -469,11 +469,6 @@ impl BitcoinCore { // Make sure signing is successful if signed_funded_raw_tx.errors.is_some() { - log::warn!( - "Received bitcoin funding errors (complete={}): {:?}", - signed_funded_raw_tx.complete, - signed_funded_raw_tx.errors - ); return Err(Error::TransactionSigningError); } @@ -1097,7 +1092,7 @@ impl BitcoinCoreApi for BitcoinCore { // filter to only import // a) payments in the blockchain (not in mempool), and // b) payments TO the address (as bitcoin core will already know about transactions spending FROM it) - let confirmed_payments_to = all_transactions.iter().filter(|tx| { + let confirmed_payments_to = all_transactions.into_iter().filter(|tx| { if let Some(status) = &tx.status { if !status.confirmed { return false; @@ -1118,24 +1113,6 @@ impl BitcoinCoreApi for BitcoinCore { &[serde_json::to_value(raw_tx)?, serde_json::to_value(raw_merkle_proof)?], )?; } - // TODO: remove this migration after the next runtime upgrade - // filter to remove spent funds, the previous wallet migration caused - // signing failures for pruned nodes because they tried to double spend - let confirmed_payments_from = all_transactions.iter().filter(|tx| { - if let Some(status) = &tx.status { - if !status.confirmed { - return false; - } - }; - tx.vin - .iter() - .filter_map(|input| input.prevout.clone()) - .any(|output| matches!(&output.scriptpubkey_address, Some(addr) if addr == &address)) - }); - for transaction in confirmed_payments_from { - self.rpc - .call("removeprunedfunds", &[serde_json::to_value(transaction.txid)?])?; - } } Ok(()) } diff --git a/vault/src/system.rs b/vault/src/system.rs index 892a733e0..6a0cd3688 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -312,10 +312,9 @@ impl VaultIdManager { } tracing::info!("Merging wallet for {:?}", vault_id); - let all_addresses = btc_rpc.list_addresses()?; // issue keys should be imported separately but we need to iterate // through currency specific wallets to get change addresses - for address in &all_addresses { + for address in btc_rpc.list_addresses()? { tracing::info!("Found {:?}", address); // get private key from currency specific wallet let private_key = btc_rpc.dump_private_key(&address)?; @@ -323,12 +322,6 @@ impl VaultIdManager { btc_rpc_shared.import_private_key(&private_key, false)?; } - if btc_rpc_shared.get_pruned_height().await? != 0 { - // rescan via electrs to import or remove change utxos - // this is required because pruned nodes cannot rescan themselves - btc_rpc_shared.rescan_electrs_for_addresses(all_addresses).await?; - } - tracing::info!("Initializing metrics..."); let metrics = PerCurrencyMetrics::new(&vault_id); let data = VaultData { From 39d1c4d42adb89565f1a2839616494422430d17d Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:47:25 +0200 Subject: [PATCH 14/22] Revert "fix: create shared wallet" This reverts commit 48775ffcb95f403c2b3e448ec4210273b2aa832b. --- vault/src/connection_manager.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/vault/src/connection_manager.rs b/vault/src/connection_manager.rs index 070edc9c8..6ff71e9fd 100644 --- a/vault/src/connection_manager.rs +++ b/vault/src/connection_manager.rs @@ -106,7 +106,6 @@ impl ConnectionManager { // master wallet if we switch to descriptor wallets let bitcoin_core_shared = config_copy.new_client_with_network(Some(format!("{prefix}-shared")), network_copy)?; - bitcoin_core_shared.create_or_load_wallet().await?; let constructor = move |vault_id: VaultId| { let collateral_currency: CurrencyId = vault_id.collateral_currency(); From 2ebf990287cfe00e568e22e300c0770067d2b003 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:47:56 +0200 Subject: [PATCH 15/22] Revert "chore: fix vaultid import" This reverts commit f8b2f8d9b02502bb36eeec06c089e9064227e5d1. --- vault/src/issue.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/vault/src/issue.rs b/vault/src/issue.rs index 9bc8dea54..e2b51b268 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -464,7 +464,6 @@ mod tests { subxt::utils::Static, CurrencyId::Token, TokenSymbol::{DOT, IBTC, INTR}, - VaultId, }; fn dummy_issues(heights: Vec<(u32, usize)>) -> Vec { From d0869dc0043de909b75c970c0e6010a15adf8655 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:49:45 +0200 Subject: [PATCH 16/22] Revert "refactor: import all issue keys" This reverts commit 189f7e2db9736315246332c253733f427a7b3807. --- vault/src/issue.rs | 39 +++++++++++++++++-------------- vault/src/system.rs | 56 +++++++++++++++++++++------------------------ 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/vault/src/issue.rs b/vault/src/issue.rs index e2b51b268..da3f0f11c 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -5,9 +5,9 @@ use crate::{ use bitcoin::{BlockHash, Error as BitcoinError, Hash, PublicKey, Transaction, TransactionExt}; use futures::{channel::mpsc::Sender, future, SinkExt, StreamExt, TryFutureExt}; use runtime::{ - AccountId, BtcAddress, BtcPublicKey, BtcRelayPallet, CancelIssueEvent, ExecuteIssueEvent, H256Le, - InterBtcIssueRequest, InterBtcParachain, IssuePallet, IssueRequestStatus, PartialAddress, PrettyPrint, - RequestIssueEvent, UtilFuncs, H256, + BtcAddress, BtcPublicKey, BtcRelayPallet, CancelIssueEvent, ExecuteIssueEvent, H256Le, InterBtcIssueRequest, + InterBtcParachain, IssuePallet, IssueRequestStatus, PartialAddress, PrettyPrint, RequestIssueEvent, UtilFuncs, + VaultId, H256, }; use sha2::{Digest, Sha256}; use std::{ @@ -82,11 +82,10 @@ struct RescanStatus { newest_issue_height: u32, queued_rescan_range: Option<(usize, usize)>, // start, end(including) } - impl RescanStatus { // there was a bug pre-v2 that set rescanning status to an invalid range. // by changing the keyname we effectively force a reset - const KEY: &str = "rescan-status-v3"; + const KEY: &str = "rescan-status-v2"; fn update(&mut self, mut issues: Vec, current_bitcoin_height: usize) { // Only look at issues that haven't been processed yet issues.retain(|issue| issue.opentime > self.newest_issue_height); @@ -134,12 +133,11 @@ impl RescanStatus { Some((start, chunk_end)) } - fn get(account_id: &AccountId, db: &DatabaseConfig) -> Result { - Ok(db.get(account_id, Self::KEY)?.unwrap_or_default()) + fn get(vault_id: &VaultId, db: &crate::system::DatabaseConfig) -> Result { + Ok(db.get(vault_id, Self::KEY)?.unwrap_or_default()) } - - fn store(&self, account_id: &AccountId, db: &DatabaseConfig) -> Result<(), Error> { - db.put(account_id, Self::KEY, self)?; + fn store(&self, vault_id: &VaultId, db: &crate::system::DatabaseConfig) -> Result<(), Error> { + db.put(vault_id, Self::KEY, self)?; Ok(()) } } @@ -147,13 +145,19 @@ impl RescanStatus { pub async fn add_keys_from_past_issue_request( bitcoin_core: &DynBitcoinCoreApi, btc_parachain: &InterBtcParachain, - db: &DatabaseConfig, + vault_id: &VaultId, + db: &crate::system::DatabaseConfig, ) -> Result<(), Error> { - let account_id = btc_parachain.get_account_id(); - let mut scanning_status = RescanStatus::get(&account_id, db)?; - tracing::info!("Scanning: {scanning_status:?}"); + let mut scanning_status = RescanStatus::get(vault_id, db)?; + tracing::info!("initial status: = {scanning_status:?}"); - let issue_requests = btc_parachain.get_vault_issue_requests(account_id.clone()).await?; + // TODO: remove filter since we use a shared wallet + let issue_requests: Vec<_> = btc_parachain + .get_vault_issue_requests(btc_parachain.get_account_id().clone()) + .await? + .into_iter() + .filter(|(_, issue)| &issue.vault == vault_id) + .collect(); for (issue_id, request) in issue_requests.clone().into_iter() { if let Err(e) = add_new_deposit_key(bitcoin_core, issue_id, request.btc_public_key).await { @@ -194,7 +198,7 @@ pub async fn add_keys_from_past_issue_request( } // save progress s.t. we don't rescan pruned range again if we crash now - scanning_status.store(account_id, db)?; + scanning_status.store(vault_id, db)?; let mut chunk_size = 1; // rescan the blockchain in chunks, so that we can save progress. The code below @@ -214,7 +218,7 @@ pub async fn add_keys_from_past_issue_request( chunk_size = (chunk_size.checked_div(2).ok_or(Error::ArithmeticUnderflow)?).max(1); } - scanning_status.store(account_id, db)?; + scanning_status.store(vault_id, db)?; } Ok(()) @@ -462,6 +466,7 @@ mod tests { use super::*; use runtime::{ subxt::utils::Static, + AccountId, CurrencyId::Token, TokenSymbol::{DOT, IBTC, INTR}, }; diff --git a/vault/src/system.rs b/vault/src/system.rs index 6a0cd3688..79b9822da 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -19,9 +19,9 @@ use futures::{ use git_version::git_version; use runtime::{ cli::{parse_duration_minutes, parse_duration_ms}, - AccountId, BtcRelayPallet, CollateralBalancesPallet, CurrencyId, Error as RuntimeError, InterBtcParachain, - PrettyPrint, RegisterVaultEvent, StoreMainChainHeaderEvent, TryFromSymbol, UpdateActiveBlockEvent, UtilFuncs, - VaultCurrencyPair, VaultId, VaultRegistryPallet, + BtcRelayPallet, CollateralBalancesPallet, CurrencyId, Error as RuntimeError, InterBtcParachain, PrettyPrint, + RegisterVaultEvent, RuntimeCurrencyInfo, StoreMainChainHeaderEvent, TryFromSymbol, UpdateActiveBlockEvent, + UtilFuncs, VaultCurrencyPair, VaultId, VaultRegistryPallet, }; use std::{collections::HashMap, pin::Pin, sync::Arc, time::Duration}; use tokio::{sync::RwLock, time::sleep}; @@ -182,25 +182,35 @@ pub struct DatabaseConfig { } impl DatabaseConfig { - fn prefixed_key(account_id: &AccountId, key: &str) -> Result { + fn prefixed_key(vault_id: &VaultId, key: &str) -> Result { Ok(format!( - "{}-{}", - account_id.pretty_print(), /* technically not needed since each client should have their own - * db, but doesn't hurt to be safe */ + "{}-{}-{}-{}", + vault_id.account_id.pretty_print(), /* technically not needed since each client should have their own + * db, but doesn't hurt to be safe */ + vault_id + .currencies + .collateral + .symbol() + .map_err(|_| BitcoinError::FailedToConstructWalletName)?, + vault_id + .currencies + .wrapped + .symbol() + .map_err(|_| BitcoinError::FailedToConstructWalletName)?, key )) } - pub fn put(&self, account_id: &AccountId, key: &str, value: &V) -> Result<(), Error> { + pub fn put(&self, vault_id: &VaultId, key: &str, value: &V) -> Result<(), Error> { let db = rocksdb::DB::open_default(self.path.clone())?; - let key = Self::prefixed_key(account_id, key)?; + let key = Self::prefixed_key(vault_id, key)?; db.put(key, serde_json::to_vec(value)?)?; Ok(()) } - pub fn get(&self, account_id: &AccountId, key: &str) -> Result, Error> { + pub fn get(&self, vault_id: &VaultId, key: &str) -> Result, Error> { let db = rocksdb::DB::open_default(self.path.clone())?; - let key = Self::prefixed_key(account_id, key)?; + let key = Self::prefixed_key(vault_id, key)?; let value = match db.get(key)? { None => return Ok(None), @@ -311,7 +321,6 @@ impl VaultIdManager { } } - tracing::info!("Merging wallet for {:?}", vault_id); // issue keys should be imported separately but we need to iterate // through currency specific wallets to get change addresses for address in btc_rpc.list_addresses()? { @@ -322,6 +331,9 @@ impl VaultIdManager { btc_rpc_shared.import_private_key(&private_key, false)?; } + tracing::info!("Adding keys from past issues..."); + issue::add_keys_from_past_issue_request(&btc_rpc_shared, &self.btc_parachain, &vault_id, &self.db).await?; + tracing::info!("Initializing metrics..."); let metrics = PerCurrencyMetrics::new(&vault_id); let data = VaultData { @@ -342,9 +354,8 @@ impl VaultIdManager { .get_vaults_by_account_id(self.btc_parachain.get_account_id()) .await? { - match (only_migrate, is_vault_registered(&self.btc_parachain, &vault_id).await) { - // TODO: import keys for liquidated vaults? - (false, Err(Error::RuntimeError(RuntimeError::VaultLiquidated))) => { + match is_vault_registered(&self.btc_parachain, &vault_id).await { + Err(Error::RuntimeError(RuntimeError::VaultLiquidated)) => { tracing::error!( "[{}] Vault is liquidated -- not going to process events for this vault.", vault_id.pretty_print() @@ -417,7 +428,6 @@ impl VaultIdManager { pub struct VaultService { btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -526,7 +536,6 @@ impl VaultService { Self { btc_parachain: btc_parachain.clone(), btc_rpc_master_wallet: btc_rpc_master_wallet.clone(), - btc_rpc_shared_wallet: btc_rpc_shared_wallet.clone(), config, monitoring_config, shutdown, @@ -648,19 +657,6 @@ impl VaultService { // purposefully _after_ maybe_register_vault and _before_ other calls self.vault_id_manager.fetch_vault_ids().await?; - tracing::info!("Adding keys from past issues..."); - issue::add_keys_from_past_issue_request( - &self.btc_rpc_shared_wallet, - &self.btc_parachain, - &self.vault_id_manager.db, - ) - .await?; - - if self.config.only_migrate { - tracing::info!("Only migrating - quitting now."); - return Err(BackoffError::Permanent(Error::ClientShutdown)); - } - let startup_height = self.await_parachain_block().await?; let open_request_executor = execute_open_requests( From 0147a37a406db40194a9b4c9a1cd28a9f4502758 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 14:49:55 +0200 Subject: [PATCH 17/22] Revert "chore: duplicate rpc in test setup" This reverts commit c3925ecff23cf8fae08c410b420d886a31d0e56c. --- vault/tests/vault_integration_tests.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/vault/tests/vault_integration_tests.rs b/vault/tests/vault_integration_tests.rs index 1c249c3e8..bfb420fff 100644 --- a/vault/tests/vault_integration_tests.rs +++ b/vault/tests/vault_integration_tests.rs @@ -95,7 +95,6 @@ async fn test_redeem_succeeds() { let btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( vault_provider.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_redeem_succeeds", @@ -165,7 +164,6 @@ async fn test_replace_succeeds() { let new_btc_rpc_master_wallet = btc_rpc.clone(); let _vault_id_manager = VaultIdManager::from_map( new_vault_provider.clone(), - new_btc_rpc_master_wallet.clone(), new_btc_rpc_master_wallet, btc_rpcs, "test_replace_succeeds1", @@ -179,7 +177,6 @@ async fn test_replace_succeeds() { let old_btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( old_vault_provider.clone(), - old_btc_rpc_master_wallet.clone(), old_btc_rpc_master_wallet, btc_rpcs, "test_replace_succeeds2", @@ -354,7 +351,6 @@ async fn test_cancellation_succeeds() { let new_btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( new_vault_provider.clone(), - new_btc_rpc_master_wallet.clone(), new_btc_rpc_master_wallet, btc_rpcs, "test_cancellation_succeeds", @@ -623,7 +619,6 @@ async fn test_automatic_issue_execution_succeeds() { let btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( vault2_provider.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_automatic_issue_execution_succeeds", @@ -723,7 +718,6 @@ async fn test_automatic_issue_execution_succeeds_with_big_transaction() { let btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( vault2_provider.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_automatic_issue_execution_succeeds_with_big_transaction", @@ -812,7 +806,6 @@ async fn test_execute_open_requests_succeeds() { let btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( vault_provider.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_execute_open_requests_succeeds", @@ -1213,7 +1206,6 @@ mod test_with_bitcoind { let btc_rpc_master_wallet = btc_rpc.clone(); let vault_id_manager = VaultIdManager::from_map( vault_provider.clone(), - btc_rpc_master_wallet.clone(), btc_rpc_master_wallet, btc_rpcs, "test_automatic_rbf_succeeds", From a596fca33b1710e7c770167d994652d51ea2a8c5 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 15:10:14 +0200 Subject: [PATCH 18/22] Revert "refactor: merge duplicate bitcoin metrics" This reverts commit 1ea7cd621d61c00458706f9ddff0fefb8648bb0e. --- vault/src/metrics.rs | 134 +++++++++++++++++++++++++------------------ vault/src/system.rs | 2 +- 2 files changed, 80 insertions(+), 56 deletions(-) diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index d98bd533a..9d9ab8fb1 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -31,7 +31,7 @@ const SLEEP_DURATION: Duration = Duration::from_secs(5 * 60); const SECONDS_PER_HOUR: f64 = 3600.0; const CURRENCY_LABEL: &str = "currency"; -const EXPECTED_BTC_BALANCE_TYPE_LABEL: &str = "type"; +const BTC_BALANCE_TYPE_LABEL: &str = "type"; const REQUEST_STATUS_LABEL: &str = "status"; const TASK_NAME: &str = "task"; const TOKIO_POLLING_INTERVAL_MS: u64 = 10000; @@ -45,10 +45,9 @@ lazy_static! { &[CURRENCY_LABEL] ) .expect("Failed to create prometheus metric"); - pub static ref AVERAGE_BTC_FEE: StatefulGauge = StatefulGauge { - gauge: Gauge::new("avg_btc_fee", "Average Bitcoin Fee").expect("Failed to create prometheus metric"), - data: Arc::new(RwLock::new(AverageTracker { total: 0, count: 0 })), - }; + pub static ref AVERAGE_BTC_FEE: GaugeVec = + GaugeVec::new(Opts::new("avg_btc_fee", "Average Bitcoin Fee"), &[CURRENCY_LABEL]) + .expect("Failed to create prometheus metric"); pub static ref LOCKED_COLLATERAL: GaugeVec = GaugeVec::new(Opts::new("locked_collateral", "Locked Collateral"), &[CURRENCY_LABEL]) .expect("Failed to create prometheus metric"); @@ -71,13 +70,14 @@ lazy_static! { &[TASK_NAME] ) .expect("Failed to create prometheus metric"); - pub static ref UTXO_COUNT: IntGauge = - IntGauge::new("utxo_count", "Number of Unspent Bitcoin Outputs",).expect("Failed to create prometheus metric"); - pub static ref ACTUAL_BTC_BALANCE: Gauge = - Gauge::new("actual_btc_balance", "Actual Bitcoin Balance",).expect("Failed to create prometheus metric"); - pub static ref EXPECTED_BTC_BALANCE: GaugeVec = GaugeVec::new( - Opts::new("expected_btc_balance", "Expected Bitcoin Balance"), - &[CURRENCY_LABEL, EXPECTED_BTC_BALANCE_TYPE_LABEL] + pub static ref UTXO_COUNT: IntGaugeVec = IntGaugeVec::new( + Opts::new("utxo_count", "Number of Unspent Bitcoin Outputs"), + &[CURRENCY_LABEL] + ) + .expect("Failed to create prometheus metric"); + pub static ref BTC_BALANCE: GaugeVec = GaugeVec::new( + Opts::new("btc_balance", "Bitcoin Balance"), + &[CURRENCY_LABEL, BTC_BALANCE_TYPE_LABEL] ) .expect("Failed to create prometheus metric"); pub static ref ISSUES: GaugeVec = GaugeVec::new( @@ -92,22 +92,21 @@ lazy_static! { .expect("Failed to create prometheus metric"); pub static ref NATIVE_CURRENCY_BALANCE: Gauge = Gauge::new("native_currency_balance", "Native Currency Balance").expect("Failed to create prometheus metric"); - pub static ref FEE_BUDGET_SURPLUS: StatefulGauge = StatefulGauge { - gauge: Gauge::new("fee_budget_surplus", "Fee Budget Surplus").expect("Failed to create prometheus metric"), - data: Arc::new(RwLock::new(0)), - }; + pub static ref FEE_BUDGET_SURPLUS: GaugeVec = + GaugeVec::new(Opts::new("fee_budget_surplus", "Fee Budget Surplus"), &[CURRENCY_LABEL]) + .expect("Failed to create prometheus metric"); pub static ref RESTART_COUNT: IntCounter = IntCounter::new("restart_count", "Number of service restarts").expect("Failed to create prometheus metric"); } #[derive(Clone, Debug)] -pub struct AverageTracker { +struct AverageTracker { total: u64, count: u64, } #[derive(Clone, Debug)] -pub struct StatefulGauge { +struct StatefulGauge { gauge: Gauge, data: Arc>, } @@ -116,6 +115,7 @@ pub struct StatefulGauge { struct BtcBalance { upperbound: Gauge, lowerbound: Gauge, + actual: Gauge, } #[derive(Clone, Debug)] @@ -134,6 +134,9 @@ pub struct PerCurrencyMetrics { btc_balance: BtcBalance, issues: RequestCounter, redeems: RequestCounter, + average_btc_fee: StatefulGauge, + fee_budget_surplus: StatefulGauge, + utxo_count: IntGauge, } #[async_trait] @@ -169,10 +172,9 @@ impl PerCurrencyMetrics { fn new_with_label(label: &str) -> Self { let labels = HashMap::from([(CURRENCY_LABEL, label)]); - let expected_btc_balance_gauge = |balance_type: &'static str| { - let labels = - HashMap::<&str, &str>::from([(CURRENCY_LABEL, label), (EXPECTED_BTC_BALANCE_TYPE_LABEL, balance_type)]); - EXPECTED_BTC_BALANCE.with(&labels) + let btc_balance_gauge = |balance_type: &'static str| { + let labels = HashMap::<&str, &str>::from([(CURRENCY_LABEL, label), (BTC_BALANCE_TYPE_LABEL, balance_type)]); + BTC_BALANCE.with(&labels) }; let request_type_label = |balance_type: &'static str| { HashMap::<&str, &str>::from([(CURRENCY_LABEL, label), (REQUEST_STATUS_LABEL, balance_type)]) @@ -183,9 +185,19 @@ impl PerCurrencyMetrics { collateralization: COLLATERALIZATION.with(&labels), required_collateral: REQUIRED_COLLATERAL.with(&labels), remaining_time_to_redeem_hours: REMAINING_TIME_TO_REDEEM_HOURS.with(&labels), + utxo_count: UTXO_COUNT.with(&labels), + fee_budget_surplus: StatefulGauge { + gauge: FEE_BUDGET_SURPLUS.with(&labels), + data: Arc::new(RwLock::new(0)), + }, + average_btc_fee: StatefulGauge { + gauge: AVERAGE_BTC_FEE.with(&labels), + data: Arc::new(RwLock::new(AverageTracker { total: 0, count: 0 })), + }, btc_balance: BtcBalance { - upperbound: expected_btc_balance_gauge("required_upperbound"), - lowerbound: expected_btc_balance_gauge("required_lowerbound"), + upperbound: btc_balance_gauge("required_upperbound"), + lowerbound: btc_balance_gauge("required_lowerbound"), + actual: btc_balance_gauge("actual"), }, issues: RequestCounter { open_count: ISSUES.with(&request_type_label("open")), @@ -236,7 +248,7 @@ impl PerCurrencyMetrics { .fold(0i64, |acc, x| async move { acc.saturating_add(x) }) .await; - *FEE_BUDGET_SURPLUS.data.write().await = fee_budget_surplus; + *vault.metrics.fee_budget_surplus.data.write().await = fee_budget_surplus; publish_fee_budget_surplus(vault).await?; } Ok(()) @@ -256,14 +268,14 @@ impl PerCurrencyMetrics { .iter() .filter_map(|tx| tx.detail.fee.map(|amount| amount.to_sat().unsigned_abs())) .fold((0, 0), |(total, count), x| (total + x, count + 1)); - *AVERAGE_BTC_FEE.data.write().await = AverageTracker { total, count }; + *vault.metrics.average_btc_fee.data.write().await = AverageTracker { total, count }; - publish_utxo_count(&vault.btc_rpc); - publish_bitcoin_balance(&vault.btc_rpc); + publish_utxo_count(vault); + publish_bitcoin_balance(vault); let _ = tokio::join!( Self::initialize_fee_budget_surplus(vault, parachain_rpc.clone(), bitcoin_transactions), - publish_average_bitcoin_fee(), + publish_average_bitcoin_fee(vault), publish_expected_bitcoin_balance(vault, parachain_rpc.clone()), publish_locked_collateral(vault, ¶chain_rpc), publish_required_collateral(vault, ¶chain_rpc), @@ -273,13 +285,12 @@ impl PerCurrencyMetrics { } pub fn register_custom_metrics() -> Result<(), RuntimeError> { - REGISTRY.register(Box::new(AVERAGE_BTC_FEE.gauge.clone()))?; + REGISTRY.register(Box::new(AVERAGE_BTC_FEE.clone()))?; REGISTRY.register(Box::new(LOCKED_COLLATERAL.clone()))?; REGISTRY.register(Box::new(COLLATERALIZATION.clone()))?; REGISTRY.register(Box::new(REQUIRED_COLLATERAL.clone()))?; - REGISTRY.register(Box::new(FEE_BUDGET_SURPLUS.gauge.clone()))?; - REGISTRY.register(Box::new(ACTUAL_BTC_BALANCE.clone()))?; - REGISTRY.register(Box::new(EXPECTED_BTC_BALANCE.clone()))?; + REGISTRY.register(Box::new(FEE_BUDGET_SURPLUS.clone()))?; + REGISTRY.register(Box::new(BTC_BALANCE.clone()))?; REGISTRY.register(Box::new(NATIVE_CURRENCY_BALANCE.clone()))?; REGISTRY.register(Box::new(ISSUES.clone()))?; REGISTRY.register(Box::new(REDEEMS.clone()))?; @@ -366,45 +377,47 @@ pub async fn update_bitcoin_metrics( // update the average fee if let Some(amount) = new_fee_entry { { - let mut tmp = AVERAGE_BTC_FEE.data.write().await; + let mut tmp = vault.metrics.average_btc_fee.data.write().await; *tmp = AverageTracker { total: tmp.total.saturating_add(amount.to_sat().unsigned_abs()), count: tmp.count.saturating_add(1), }; } - publish_average_bitcoin_fee().await; + publish_average_bitcoin_fee(vault).await; if let Ok(budget) = TryInto::::try_into(fee_budget.unwrap_or(0)) { let surplus = budget.saturating_sub(amount.to_sat().abs()); - let mut tmp = FEE_BUDGET_SURPLUS.data.write().await; + let mut tmp = vault.metrics.fee_budget_surplus.data.write().await; *tmp = tmp.saturating_add(surplus); } publish_fee_budget_surplus(vault).await?; } - publish_bitcoin_balance(&vault.btc_rpc); + publish_bitcoin_balance(vault); Ok(()) } -async fn publish_fee_budget_surplus(vault: &VaultData) -> Result<(), Error> { - let surplus = *FEE_BUDGET_SURPLUS.data.read().await; - FEE_BUDGET_SURPLUS +async fn publish_fee_budget_surplus(vault: &VaultData) -> Result<(), ServiceError> { + let surplus = *vault.metrics.fee_budget_surplus.data.read().await; + vault + .metrics + .fee_budget_surplus .gauge .set(surplus as f64 / vault.vault_id.wrapped_currency().inner()?.one() as f64); Ok(()) } -async fn publish_average_bitcoin_fee() { - let average = match AVERAGE_BTC_FEE.data.read().await { +async fn publish_average_bitcoin_fee(vault: &VaultData) { + let average = match vault.metrics.average_btc_fee.data.read().await { x if x.count > 0 => x.total as f64 / x.count as f64, _ => Default::default(), }; - AVERAGE_BTC_FEE.gauge.set(average); + vault.metrics.average_btc_fee.gauge.set(average); } -fn publish_bitcoin_balance(btc_rpc: &DynBitcoinCoreApi) { - match btc_rpc.get_balance(None) { - Ok(bitcoin_balance) => ACTUAL_BTC_BALANCE.set(bitcoin_balance.to_btc()), +fn publish_bitcoin_balance(vault: &VaultData) { + match vault.btc_rpc.get_balance(None) { + Ok(bitcoin_balance) => vault.metrics.btc_balance.actual.set(bitcoin_balance.to_btc()), Err(e) => { // unexpected error, but not critical so just continue tracing::warn!("Failed to get Bitcoin balance: {}", e); @@ -423,10 +436,10 @@ async fn publish_native_currency_balance>>, btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - pub(crate) btc_rpc_shared_wallet: DynBitcoinCoreApi, + btc_rpc_shared_wallet: DynBitcoinCoreApi, // TODO: remove this #[allow(clippy::type_complexity)] constructor: Arc Result + Send + Sync>>, From 4793dcc14b784d49cfd586dde7f23ebf6bf1f49b Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 15:12:18 +0200 Subject: [PATCH 19/22] Revert "refactor: merge currency specific bitcoin wallets" This reverts commit d011b3d726c59c804af774272707777f72878d74. --- bitcoin/src/iter.rs | 5 +- bitcoin/src/lib.rs | 42 ++++------------ bitcoin/src/light/mod.rs | 18 ++----- runtime/src/integration/bitcoin_simulator.rs | 7 +-- vault/src/connection_manager.rs | 17 ++----- vault/src/execution.rs | 8 ++- vault/src/issue.rs | 3 +- vault/src/metrics.rs | 29 +++++------ vault/src/replace.rs | 5 +- vault/src/system.rs | 52 ++++++-------------- 10 files changed, 53 insertions(+), 133 deletions(-) diff --git a/bitcoin/src/iter.rs b/bitcoin/src/iter.rs index 81dba865e..e5d9ed130 100644 --- a/bitcoin/src/iter.rs +++ b/bitcoin/src/iter.rs @@ -207,7 +207,6 @@ mod tests { async fn wait_for_block(&self, height: u32, num_confirmations: u32) -> Result; fn get_balance(&self, min_confirmations: Option) -> Result; fn list_transactions(&self, max_count: Option) -> Result, Error>; - fn list_addresses(&self) -> Result, Error>; async fn get_block_count(&self) -> Result; async fn get_raw_tx(&self, txid: &Txid, block_hash: &BlockHash) -> Result, Error>; async fn get_transaction(&self, txid: &Txid, block_hash: Option) -> Result; @@ -215,8 +214,8 @@ mod tests { async fn get_block_hash(&self, height: u32) -> Result; async fn get_new_address(&self) -> Result; async fn get_new_public_key(&self) -> Result; - fn dump_private_key(&self, address: &Address) -> Result; - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), Error>; + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result; + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), Error>; async fn add_new_deposit_key( &self, public_key: PublicKey, diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index fe5148c30..eedfad98d 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -148,8 +148,6 @@ pub trait BitcoinCoreApi { fn list_transactions(&self, max_count: Option) -> Result, Error>; - fn list_addresses(&self) -> Result, Error>; - async fn get_raw_tx(&self, txid: &Txid, block_hash: &BlockHash) -> Result, Error>; async fn get_transaction(&self, txid: &Txid, block_hash: Option) -> Result; @@ -162,9 +160,9 @@ pub trait BitcoinCoreApi { async fn get_new_public_key(&self) -> Result; - fn dump_private_key(&self, address: &Address) -> Result; + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result; - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), Error>; + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), Error>; async fn add_new_deposit_key(&self, public_key: PublicKey, secret_key: Vec) -> Result<(), Error>; @@ -711,27 +709,6 @@ impl BitcoinCoreApi for BitcoinCore { .list_transactions(None, max_count.or(Some(DEFAULT_MAX_TX_COUNT)), None, None)?) } - // TODO: remove this once the wallet migration has completed - fn list_addresses(&self) -> Result, Error> { - // Lists groups of addresses which have had their common ownership - // made public by common use as inputs or as the resulting change - // in past transactions - let groupings: Vec>> = self.rpc.call("listaddressgroupings", &[])?; - let addresses = groupings - .into_iter() - .flatten() - .filter_map(|group| { - group - .get(0) - .and_then(|v| v.as_str()) - .map(Address::from_str)? - .and_then(|x| x.require_network(self.network)) - .ok() - }) - .collect::>(); - Ok(addresses) - } - /// Get the raw transaction identified by `Txid` and stored /// in the specified block. /// @@ -797,16 +774,15 @@ impl BitcoinCoreApi for BitcoinCore { Ok(public_key) } - fn dump_private_key(&self, address: &Address) -> Result { - Ok(self.rpc.dump_private_key(address)?) + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result { + let address = Address::p2wpkh(public_key, self.network).map_err(ConversionError::from)?; + Ok(self.rpc.dump_private_key(&address)?) } - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), Error> { - Ok(self.rpc.import_private_key( - private_key, - is_derivation_key.then_some(DERIVATION_KEY_LABEL), - Some(false), - )?) + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), Error> { + Ok(self + .rpc + .import_private_key(private_key, Some(DERIVATION_KEY_LABEL), Some(false))?) } /// Derive and import the private key for the master public key and public secret diff --git a/bitcoin/src/light/mod.rs b/bitcoin/src/light/mod.rs index 66f20231e..1c6ba6cf7 100644 --- a/bitcoin/src/light/mod.rs +++ b/bitcoin/src/light/mod.rs @@ -137,12 +137,6 @@ impl BitcoinCoreApi for BitcoinLight { Ok(Default::default()) } - // TODO: remove this later - fn list_addresses(&self) -> Result, BitcoinError> { - // don't need to migrate keys - Ok(Default::default()) - } - async fn get_raw_tx(&self, txid: &Txid, _block_hash: &BlockHash) -> Result, BitcoinError> { Ok(self.electrs.get_raw_tx(txid).await?) } @@ -172,17 +166,11 @@ impl BitcoinCoreApi for BitcoinLight { Ok(self.private_key.public_key(&self.secp_ctx)) } - fn dump_private_key(&self, address: &Address) -> Result { - self.wallet - .key_store - .read() - .map_err(Into::::into)? - .get(address) - .ok_or(Error::NoPrivateKey.into()) - .cloned() + fn dump_derivation_key(&self, _public_key: &PublicKey) -> Result { + Ok(self.private_key) } - fn import_private_key(&self, _private_key: &PrivateKey, _is_derivation_key: bool) -> Result<(), BitcoinError> { + fn import_derivation_key(&self, _private_key: &PrivateKey) -> Result<(), BitcoinError> { // nothing to do Ok(()) } diff --git a/runtime/src/integration/bitcoin_simulator.rs b/runtime/src/integration/bitcoin_simulator.rs index 96838c98f..8a81a2859 100644 --- a/runtime/src/integration/bitcoin_simulator.rs +++ b/runtime/src/integration/bitcoin_simulator.rs @@ -341,9 +341,6 @@ impl BitcoinCoreApi for MockBitcoinCore { fn list_transactions(&self, max_count: Option) -> Result, BitcoinError> { Ok(vec![]) } - fn list_addresses(&self) -> Result, BitcoinError> { - Ok(vec![]) - } async fn get_block_count(&self) -> Result { Ok((self.blocks.read().await.len() - 1).try_into().unwrap()) } @@ -412,10 +409,10 @@ impl BitcoinCoreApi for MockBitcoinCore { let public_key = secp256k1::PublicKey::from_secret_key(&secp, &secret_key); Ok(PublicKey::new(public_key)) } - fn dump_private_key(&self, address: &Address) -> Result { + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result { todo!() } - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError> { + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError> { todo!() } async fn add_new_deposit_key(&self, _public_key: PublicKey, _secret_key: Vec) -> Result<(), BitcoinError> { diff --git a/vault/src/connection_manager.rs b/vault/src/connection_manager.rs index 6ff71e9fd..63eeffb66 100644 --- a/vault/src/connection_manager.rs +++ b/vault/src/connection_manager.rs @@ -26,8 +26,7 @@ pub trait Service { fn new_service( btc_parachain: BtcParachain, - bitcoin_core_master: DynBitcoinCoreApi, - bitcoin_core_shared: DynBitcoinCoreApi, + bitcoin_core: DynBitcoinCoreApi, config: Config, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -84,7 +83,7 @@ impl ConnectionManager { let shutdown_tx = ShutdownSender::new(); let prefix = self.wallet_name.clone().unwrap_or_else(|| "vault".to_string()); - let bitcoin_core_master = self.bitcoin_config.new_client(Some(format!("{prefix}-master"))).await?; + let bitcoin_core = self.bitcoin_config.new_client(Some(format!("{prefix}-master"))).await?; // only open connection to parachain after bitcoind sync to prevent timeout let signer = self.signer.clone(); @@ -99,14 +98,7 @@ impl ConnectionManager { .await?; let config_copy = self.bitcoin_config.clone(); - let network_copy = bitcoin_core_master.network(); - - // use a separate wallet for all bitcoin transactions - // to make exporting the private key easier from the - // master wallet if we switch to descriptor wallets - let bitcoin_core_shared = - config_copy.new_client_with_network(Some(format!("{prefix}-shared")), network_copy)?; - + let network_copy = bitcoin_core.network(); let constructor = move |vault_id: VaultId| { let collateral_currency: CurrencyId = vault_id.collateral_currency(); let wrapped_currency: CurrencyId = vault_id.wrapped_currency(); @@ -125,8 +117,7 @@ impl ConnectionManager { let service = S::new_service( btc_parachain, - bitcoin_core_master, - bitcoin_core_shared, + bitcoin_core, config, self.monitoring_config.clone(), shutdown_tx.clone(), diff --git a/vault/src/execution.rs b/vault/src/execution.rs index f774defb8..e607b6df2 100644 --- a/vault/src/execution.rs +++ b/vault/src/execution.rs @@ -664,7 +664,7 @@ fn get_request_for_btc_tx(tx: &Transaction, hash_map: &HashMap) - } } -#[cfg(all(test, feature = "parachain-metadata-kintsugi"))] +#[cfg(all(test, feature = "parachain-metadata-kintsugi-testnet"))] mod tests { use super::*; use crate::metrics::PerCurrencyMetrics; @@ -708,7 +708,6 @@ mod tests { async fn get_foreign_asset_metadata(&self, id: u32) -> Result; async fn get_lend_tokens(&self) -> Result, RuntimeError>; } - #[async_trait] pub trait VaultRegistryPallet { async fn get_vault(&self, vault_id: &VaultId) -> Result; @@ -801,7 +800,6 @@ mod tests { async fn wait_for_block(&self, height: u32, num_confirmations: u32) -> Result; fn get_balance(&self, min_confirmations: Option) -> Result; fn list_transactions(&self, max_count: Option) -> Result, BitcoinError>; - fn list_addresses(&self) -> Result, BitcoinError>; async fn get_block_count(&self) -> Result; async fn get_raw_tx(&self, txid: &Txid, block_hash: &BlockHash) -> Result, BitcoinError>; async fn get_transaction(&self, txid: &Txid, block_hash: Option) -> Result; @@ -810,8 +808,8 @@ mod tests { async fn get_pruned_height(&self) -> Result; async fn get_new_address(&self) -> Result; async fn get_new_public_key(&self) -> Result; - fn dump_private_key(&self, address: &Address) -> Result; - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError>; + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result; + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError>; async fn add_new_deposit_key(&self, public_key: PublicKey, secret_key: Vec) -> Result<(), BitcoinError>; async fn get_best_block_hash(&self) -> Result; async fn get_block(&self, hash: &BlockHash) -> Result; diff --git a/vault/src/issue.rs b/vault/src/issue.rs index da3f0f11c..b2a39ca8b 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -151,7 +151,6 @@ pub async fn add_keys_from_past_issue_request( let mut scanning_status = RescanStatus::get(vault_id, db)?; tracing::info!("initial status: = {scanning_status:?}"); - // TODO: remove filter since we use a shared wallet let issue_requests: Vec<_> = btc_parachain .get_vault_issue_requests(btc_parachain.get_account_id().clone()) .await? @@ -165,7 +164,7 @@ pub async fn add_keys_from_past_issue_request( } } - // read height only _after_ the last add_new_deposit_key. If a new block arrives + // read height only _after_ the last add_new_deposit_key.If a new block arrives // while we rescan, bitcoin core will correctly recognize addressed associated with the // privkey let btc_end_height = bitcoin_core.get_block_count().await? as usize; diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index 9d9ab8fb1..9b416c4a0 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -277,9 +277,9 @@ impl PerCurrencyMetrics { Self::initialize_fee_budget_surplus(vault, parachain_rpc.clone(), bitcoin_transactions), publish_average_bitcoin_fee(vault), publish_expected_bitcoin_balance(vault, parachain_rpc.clone()), - publish_locked_collateral(vault, ¶chain_rpc), - publish_required_collateral(vault, ¶chain_rpc), - publish_collateralization(vault, ¶chain_rpc), + publish_locked_collateral(vault, parachain_rpc.clone()), + publish_required_collateral(vault, parachain_rpc.clone()), + publish_collateralization(vault, parachain_rpc.clone()), ); } } @@ -358,8 +358,8 @@ pub async fn publish_required_collateral( Ok(()) } -pub async fn publish_collateralization(vault: &VaultData, parachain_rpc: &P) { - // if the collateralization is infinite, return 0 rather than logging an error so +pub async fn publish_collateralization(vault: &VaultData, parachain_rpc: P) { + // if the collateralization is infinite, return 0 rather than logging an error, so // the metrics do change in case of a replacement let collateralization = parachain_rpc .get_collateralization_from_vault(vault.vault_id.clone(), false) @@ -585,11 +585,9 @@ pub async fn monitor_bridge_metrics( .iter() .filter(|vault| &vault.vault_id.collateral_currency() == currency_id) { - let _ = tokio::join!( - publish_locked_collateral(vault, parachain_rpc), - publish_required_collateral(vault, parachain_rpc), - publish_collateralization(vault, parachain_rpc), - ); + let _ = publish_locked_collateral(vault, parachain_rpc.clone()).await; + let _ = publish_required_collateral(vault, parachain_rpc.clone()).await; + publish_collateralization(vault, parachain_rpc.clone()).await; } } }, @@ -791,7 +789,6 @@ mod tests { async fn wait_for_block(&self, height: u32, num_confirmations: u32) -> Result; fn get_balance(&self, min_confirmations: Option) -> Result; fn list_transactions(&self, max_count: Option) -> Result, BitcoinError>; - fn list_addresses(&self) -> Result, BitcoinError>; async fn get_block_count(&self) -> Result; async fn get_raw_tx(&self, txid: &Txid, block_hash: &BlockHash) -> Result, BitcoinError>; async fn get_transaction(&self, txid: &Txid, block_hash: Option) -> Result; @@ -800,8 +797,8 @@ mod tests { async fn get_pruned_height(&self) -> Result; async fn get_new_address(&self) -> Result; async fn get_new_public_key(&self) -> Result; - fn dump_private_key(&self, address: &Address) -> Result; - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError>; + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result; + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError>; async fn add_new_deposit_key(&self, public_key: PublicKey, secret_key: Vec) -> Result<(), BitcoinError>; async fn get_best_block_hash(&self) -> Result; async fn get_block(&self, hash: &BlockHash) -> Result; @@ -1080,7 +1077,7 @@ mod tests { metrics: PerCurrencyMetrics::dummy(), }; - publish_locked_collateral(&vault_data, ¶chain_rpc).await.unwrap(); + publish_locked_collateral(&vault_data, parachain_rpc).await.unwrap(); let total_collateral = vault_data.metrics.locked_collateral.get(); assert_eq!(total_collateral, 0.0000000075); @@ -1133,7 +1130,7 @@ mod tests { metrics: PerCurrencyMetrics::dummy(), }; - publish_collateralization(&vault_data, ¶chain_rpc).await; + publish_collateralization(&vault_data, parachain_rpc).await; let collateralization_metrics = vault_data.metrics.collateralization.get(); assert_eq!( @@ -1162,7 +1159,7 @@ mod tests { metrics: PerCurrencyMetrics::dummy(), }; - publish_required_collateral(&vault_data, ¶chain_rpc).await.unwrap(); + publish_required_collateral(&vault_data, parachain_rpc).await.unwrap(); let required_collateral = vault_data.metrics.required_collateral.get(); assert_eq!(required_collateral, 0.000000005); diff --git a/vault/src/replace.rs b/vault/src/replace.rs index 36b31669f..6f72bb44d 100644 --- a/vault/src/replace.rs +++ b/vault/src/replace.rs @@ -241,7 +241,6 @@ mod tests { async fn wait_for_block(&self, height: u32, num_confirmations: u32) -> Result; fn get_balance(&self, min_confirmations: Option) -> Result; fn list_transactions(&self, max_count: Option) -> Result, BitcoinError>; - fn list_addresses(&self) -> Result, BitcoinError>; async fn get_block_count(&self) -> Result; async fn get_raw_tx(&self, txid: &Txid, block_hash: &BlockHash) -> Result, BitcoinError>; async fn get_transaction(&self, txid: &Txid, block_hash: Option) -> Result; @@ -250,8 +249,8 @@ mod tests { async fn get_pruned_height(&self) -> Result; async fn get_new_address(&self) -> Result; async fn get_new_public_key(&self) -> Result; - fn dump_private_key(&self, address: &Address) -> Result; - fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError>; + fn dump_derivation_key(&self, public_key: &PublicKey) -> Result; + fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError>; async fn add_new_deposit_key( &self, public_key: PublicKey, diff --git a/vault/src/system.rs b/vault/src/system.rs index 5d6166ef9..e97518c4b 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -228,8 +228,7 @@ pub struct VaultIdManager { vault_data: Arc>>, btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, - // TODO: remove this + // TODO: refactor this #[allow(clippy::type_complexity)] constructor: Arc Result + Send + Sync>>, db: DatabaseConfig, @@ -239,7 +238,6 @@ impl VaultIdManager { pub fn new( btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, constructor: impl Fn(VaultId) -> Result + Send + Sync + 'static, db_path: String, ) -> Self { @@ -247,7 +245,6 @@ impl VaultIdManager { vault_data: Arc::new(RwLock::new(HashMap::new())), constructor: Arc::new(Box::new(constructor)), btc_rpc_master_wallet, - btc_rpc_shared_wallet, btc_parachain, db: DatabaseConfig { path: db_path }, } @@ -257,7 +254,6 @@ impl VaultIdManager { pub fn from_map( btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, map: HashMap, db_path: &str, ) -> Self { @@ -278,7 +274,6 @@ impl VaultIdManager { vault_data: Arc::new(RwLock::new(vault_data)), constructor: Arc::new(Box::new(|_| unimplemented!())), btc_rpc_master_wallet, - btc_rpc_shared_wallet, btc_parachain, db: DatabaseConfig { path: db_path.to_string(), @@ -295,25 +290,25 @@ impl VaultIdManager { .await .map_err(Error::WalletInitializationFailure)?; - let btc_rpc_master = &self.btc_rpc_master_wallet; - let btc_rpc_shared = self.btc_rpc_shared_wallet.clone(); - tracing::info!("Adding derivation key..."); let derivation_key = self .btc_parachain .get_public_key() .await? .ok_or(BitcoinError::MissingPublicKey)?; + + // migration to the new shared public key setup: copy the public key from the + // currency-specific wallet to the master wallet. This can be removed once all + // vaults have migrated let public_key = PublicKey::from_slice(&derivation_key.0).map_err(BitcoinError::KeyError)?; - let address = Address::p2wpkh(&public_key, btc_rpc_master.network()) - .map_err(ConversionError::from) - .map_err(BitcoinError::ConversionError)?; + if let Ok(private_key) = btc_rpc.dump_derivation_key(&public_key) { + self.btc_rpc_master_wallet.import_derivation_key(&private_key)?; + } // Copy the derivation key from the master wallet to use currency-specific wallet - match btc_rpc_master.dump_private_key(&address) { + match self.btc_rpc_master_wallet.dump_derivation_key(&public_key) { Ok(private_key) => { - // TODO: remove this after the migration is complete - btc_rpc_shared.import_private_key(&private_key, true)?; + btc_rpc.import_derivation_key(&private_key)?; } Err(err) => { tracing::error!("Could not find the derivation key in the bitcoin wallet"); @@ -321,24 +316,15 @@ impl VaultIdManager { } } - // issue keys should be imported separately but we need to iterate - // through currency specific wallets to get change addresses - for address in btc_rpc.list_addresses()? { - tracing::info!("Found {:?}", address); - // get private key from currency specific wallet - let private_key = btc_rpc.dump_private_key(&address)?; - // import key into main wallet - btc_rpc_shared.import_private_key(&private_key, false)?; - } - tracing::info!("Adding keys from past issues..."); - issue::add_keys_from_past_issue_request(&btc_rpc_shared, &self.btc_parachain, &vault_id, &self.db).await?; + + issue::add_keys_from_past_issue_request(&btc_rpc, &self.btc_parachain, &vault_id, &self.db).await?; tracing::info!("Initializing metrics..."); let metrics = PerCurrencyMetrics::new(&vault_id); let data = VaultData { vault_id: vault_id.clone(), - btc_rpc: btc_rpc_shared, + btc_rpc: btc_rpc.clone(), metrics: metrics.clone(), }; PerCurrencyMetrics::initialize_values(self.btc_parachain.clone(), &data).await; @@ -388,7 +374,6 @@ impl VaultIdManager { .await?) } - // TODO: we can refactor this since we only use one wallet pub async fn get_bitcoin_rpc(&self, vault_id: &VaultId) -> Option { self.vault_data.read().await.get(vault_id).map(|x| x.btc_rpc.clone()) } @@ -442,7 +427,6 @@ impl Service for VaultService { fn new_service( btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -452,7 +436,6 @@ impl Service for VaultService { VaultService::new( btc_parachain, btc_rpc_master_wallet, - btc_rpc_shared_wallet, config, monitoring_config, shutdown, @@ -526,7 +509,6 @@ impl VaultService { fn new( btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -539,13 +521,7 @@ impl VaultService { config, monitoring_config, shutdown, - vault_id_manager: VaultIdManager::new( - btc_parachain, - btc_rpc_master_wallet, - btc_rpc_shared_wallet, - constructor, - db_path, - ), + vault_id_manager: VaultIdManager::new(btc_parachain, btc_rpc_master_wallet, constructor, db_path), } } From 863f75aa9ec30ef8dc02a615d7e1284c39cb1b17 Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 13 Sep 2023 15:29:49 +0200 Subject: [PATCH 20/22] fix: fix incorrect merge conflict resolutions --- bitcoin/src/lib.rs | 1 - bitcoin/tests/integration_test.rs | 16 ---------------- vault/src/issue.rs | 4 ++-- vault/src/metrics.rs | 14 +++++--------- vault/src/system.rs | 2 +- 5 files changed, 8 insertions(+), 29 deletions(-) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index eedfad98d..f6ff6e19c 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -53,7 +53,6 @@ pub use sp_core::H256; use std::{ convert::TryInto, future::Future, - str::FromStr, sync::Arc, time::{Duration, Instant}, }; diff --git a/bitcoin/tests/integration_test.rs b/bitcoin/tests/integration_test.rs index 938f6d63b..865a1c734 100644 --- a/bitcoin/tests/integration_test.rs +++ b/bitcoin/tests/integration_test.rs @@ -26,22 +26,6 @@ async fn should_get_new_address() -> Result<(), Error> { Ok(()) } -#[tokio::test] -async fn should_list_addresses() -> Result<(), Error> { - let btc_rpc = new_bitcoin_core(Some("Alice".to_string()))?; - btc_rpc.create_or_load_wallet().await?; - - let address = btc_rpc.get_new_address().await?; - btc_rpc.mine_blocks(101, Some(address.clone())); - - assert!( - btc_rpc.list_addresses()?.contains(&address), - "Address not found in groupings" - ); - - Ok(()) -} - #[tokio::test] async fn should_get_new_public_key() -> Result<(), Error> { let btc_rpc = new_bitcoin_core(Some("Bob".to_string()))?; diff --git a/vault/src/issue.rs b/vault/src/issue.rs index b2a39ca8b..9586f5294 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -1,6 +1,6 @@ use crate::{ - delay::RandomDelay, metrics::publish_expected_bitcoin_balance, service::DynBitcoinCoreApi, system::DatabaseConfig, - Error, Event, IssueRequests, VaultIdManager, + delay::RandomDelay, metrics::publish_expected_bitcoin_balance, service::DynBitcoinCoreApi, Error, Event, + IssueRequests, VaultIdManager, }; use bitcoin::{BlockHash, Error as BitcoinError, Hash, PublicKey, Transaction, TransactionExt}; use futures::{channel::mpsc::Sender, future, SinkExt, StreamExt, TryFutureExt}; diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index 9b416c4a0..f0bccce45 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -2,10 +2,7 @@ use std::{collections::HashMap, convert::TryInto, sync::Arc}; use crate::{ execution::parachain_blocks_to_bitcoin_blocks_rounded_up, - service::{ - warp::{Rejection, Reply}, - DynBitcoinCoreApi, - }, + service::warp::{Rejection, Reply}, system::{VaultData, VaultIdManager}, Error, }; @@ -335,7 +332,7 @@ fn raw_value_as_currency(value: u128, currency: CurrencyId) -> Result( vault: &VaultData, - parachain_rpc: &P, + parachain_rpc: P, ) -> Result<(), Error> { if let Ok(actual_collateral) = parachain_rpc.get_vault_total_collateral(vault.vault_id.clone()).await { let actual_collateral = raw_value_as_currency(actual_collateral, vault.vault_id.collateral_currency())?; @@ -346,7 +343,7 @@ pub async fn publish_locked_collateral( pub async fn publish_required_collateral( vault: &VaultData, - parachain_rpc: &P, + parachain_rpc: P, ) -> Result<(), Error> { if let Ok(required_collateral) = parachain_rpc .get_required_collateral_for_vault(vault.vault_id.clone()) @@ -397,7 +394,7 @@ pub async fn update_bitcoin_metrics( Ok(()) } -async fn publish_fee_budget_surplus(vault: &VaultData) -> Result<(), ServiceError> { +async fn publish_fee_budget_surplus(vault: &VaultData) -> Result<(), Error> { let surplus = *vault.metrics.fee_budget_surplus.data.read().await; vault .metrics @@ -671,6 +668,7 @@ pub async fn publish_tokio_metrics( #[cfg(all(test, feature = "parachain-metadata-kintsugi"))] mod tests { use super::*; + use crate::connection_manager::DynBitcoinCoreApi; use async_trait::async_trait; use bitcoin::{ json, Address, Amount, BitcoinCoreApi, Block, BlockHash, BlockHeader, Error as BitcoinError, Network, @@ -685,8 +683,6 @@ mod tests { Error as RuntimeError, InterBtcIssueRequest, InterBtcRedeemRequest, InterBtcReplaceRequest, InterBtcVault, LendingAssets, RequestIssueEvent, Token, VaultId, VaultStatus, DOT, H256, IBTC, INTR, }; - use service::DynBitcoinCoreApi; - use std::collections::BTreeSet; mockall::mock! { Provider {} diff --git a/vault/src/system.rs b/vault/src/system.rs index e97518c4b..f4a5bb657 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -9,7 +9,7 @@ use crate::{ }; use async_trait::async_trait; use backoff::Error as BackoffError; -use bitcoin::{Address, ConversionError, Error as BitcoinError, Network, PublicKey}; +use bitcoin::{Error as BitcoinError, Network, PublicKey}; use clap::Parser; use futures::{ channel::{mpsc, mpsc::Sender}, From 0e80c01dc59864752a6b40bb37ec768044742b2f Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 20 Sep 2023 11:34:49 +0200 Subject: [PATCH 21/22] fix: fix some reverts --- bitcoin/src/lib.rs | 6 ++++++ vault/src/execution.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index f6ff6e19c..a47dbf8d6 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -466,6 +466,12 @@ impl BitcoinCore { // Make sure signing is successful if signed_funded_raw_tx.errors.is_some() { + log::warn!( + "Received bitcoin funding errors (complete={}): {:?}", + signed_funded_raw_tx.complete, + signed_funded_raw_tx.errors + ); + return Err(Error::TransactionSigningError); } diff --git a/vault/src/execution.rs b/vault/src/execution.rs index e607b6df2..7f42d81d6 100644 --- a/vault/src/execution.rs +++ b/vault/src/execution.rs @@ -664,7 +664,7 @@ fn get_request_for_btc_tx(tx: &Transaction, hash_map: &HashMap) - } } -#[cfg(all(test, feature = "parachain-metadata-kintsugi-testnet"))] +#[cfg(all(test, feature = "parachain-metadata-kintsugi"))] mod tests { use super::*; use crate::metrics::PerCurrencyMetrics; From 2c2b92cfbd9822afd2dd2f09ac01e4fa9ee8df4f Mon Sep 17 00:00:00 2001 From: Sander Bosma Date: Wed, 20 Sep 2023 11:41:21 +0200 Subject: [PATCH 22/22] Revert "fix: option for migration in liquidated vaults" This reverts commit 14719de51bb6cbf9a730ebc6d868ff9bd0bf3597. --- vault/src/system.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/vault/src/system.rs b/vault/src/system.rs index f4a5bb657..24f79745a 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -125,10 +125,6 @@ pub struct VaultServiceConfig { /// the path is generated from the --keyname argument #[clap(long)] pub db_path: Option, - - /// run the wallet migration, but don't start regular vault services - #[clap(long)] - pub only_migrate: bool, } async fn active_block_listener( @@ -334,7 +330,7 @@ impl VaultIdManager { Ok(()) } - pub async fn fetch_vault_ids(&self, only_migrate: bool) -> Result<(), Error> { + pub async fn fetch_vault_ids(&self) -> Result<(), Error> { for vault_id in self .btc_parachain .get_vaults_by_account_id(self.btc_parachain.get_account_id()) @@ -347,10 +343,10 @@ impl VaultIdManager { vault_id.pretty_print() ); } - (_, Ok(_)) | (true, Err(Error::RuntimeError(RuntimeError::VaultLiquidated))) => { + Ok(_) => { self.add_vault_id(vault_id.clone()).await?; } - (_, Err(x)) => { + Err(x) => { return Err(x); } }