From d011b3d726c59c804af774272707777f72878d74 Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Tue, 18 Jul 2023 16:31:30 +0000 Subject: [PATCH 1/6] refactor: merge currency specific bitcoin wallets Signed-off-by: Gregory Hill --- bitcoin/src/iter.rs | 5 +- bitcoin/src/lib.rs | 38 ++++++++++---- bitcoin/src/light/mod.rs | 12 +++-- runtime/src/integration/bitcoin_simulator.rs | 7 ++- service/src/lib.rs | 17 +++++-- vault/src/execution.rs | 8 +-- vault/src/issue.rs | 3 +- vault/src/metrics.rs | 33 ++++++------ vault/src/replace.rs | 5 +- vault/src/system.rs | 53 ++++++++++++++------ 10 files changed, 125 insertions(+), 56 deletions(-) diff --git a/bitcoin/src/iter.rs b/bitcoin/src/iter.rs index 311fa5c92..0e7a5c090 100644 --- a/bitcoin/src/iter.rs +++ b/bitcoin/src/iter.rs @@ -207,6 +207,7 @@ 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; @@ -214,8 +215,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_derivation_key(&self, public_key: &PublicKey) -> Result; - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), Error>; + fn dump_private_key(&self, address: &Address) -> Result; + fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> 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 eb84783b9..0112406eb 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -139,6 +139,8 @@ 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; @@ -151,9 +153,9 @@ pub trait BitcoinCoreApi { async fn get_new_public_key(&self) -> Result; - fn dump_derivation_key(&self, public_key: &PublicKey) -> Result; + fn dump_private_key(&self, address: &Address) -> Result; - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), Error>; + fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), Error>; async fn add_new_deposit_key(&self, public_key: PublicKey, secret_key: Vec) -> Result<(), Error>; @@ -692,6 +694,23 @@ 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> { + #[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize)] + pub struct ListAddressGroupingResult { + pub address: Address, + #[serde(with = "bitcoincore_rpc::bitcoin::util::amount::serde::as_btc")] + pub amount: SignedAmount, + pub label: Option, + } + + // 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", &[])?; + Ok(groupings.into_iter().flatten().map(|group| group.address).collect()) + } + /// Get the raw transaction identified by `Txid` and stored /// in the specified block. /// @@ -753,15 +772,16 @@ impl BitcoinCoreApi for BitcoinCore { Ok(public_key) } - 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 dump_private_key(&self, address: &Address) -> Result { + Ok(self.rpc.dump_private_key(address)?) } - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), Error> { - Ok(self - .rpc - .import_private_key(private_key, Some(DERIVATION_KEY_LABEL), Some(false))?) + 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), + )?) } /// 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 e1e29e4f9..69f5abe68 100644 --- a/bitcoin/src/light/mod.rs +++ b/bitcoin/src/light/mod.rs @@ -133,6 +133,12 @@ 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?) } @@ -162,11 +168,11 @@ impl BitcoinCoreApi for BitcoinLight { Ok(self.private_key.public_key(&self.secp_ctx)) } - fn dump_derivation_key(&self, _public_key: &PublicKey) -> Result { - Ok(self.private_key) + fn dump_private_key(&self, _: &Address) -> Result { + Err(Error::InvalidAddress.into()) } - fn import_derivation_key(&self, _private_key: &PrivateKey) -> Result<(), BitcoinError> { + fn import_private_key(&self, _private_key: &PrivateKey, _is_derivation_key: bool) -> Result<(), BitcoinError> { // nothing to do Ok(()) } diff --git a/runtime/src/integration/bitcoin_simulator.rs b/runtime/src/integration/bitcoin_simulator.rs index 105d6c703..3363b0eab 100644 --- a/runtime/src/integration/bitcoin_simulator.rs +++ b/runtime/src/integration/bitcoin_simulator.rs @@ -326,6 +326,9 @@ 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()) } @@ -385,10 +388,10 @@ impl BitcoinCoreApi for MockBitcoinCore { let public_key = secp256k1::PublicKey::from_secret_key(&secp, &secret_key); Ok(PublicKey::new(public_key)) } - fn dump_derivation_key(&self, public_key: &PublicKey) -> Result { + fn dump_private_key(&self, address: &Address) -> Result { todo!() } - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError> { + fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> Result<(), BitcoinError> { todo!() } async fn add_new_deposit_key(&self, _public_key: PublicKey, _secret_key: Vec) -> Result<(), BitcoinError> { diff --git a/service/src/lib.rs b/service/src/lib.rs index cf9b831f1..aef294e69 100644 --- a/service/src/lib.rs +++ b/service/src/lib.rs @@ -28,7 +28,8 @@ pub trait Service { fn new_service( btc_parachain: BtcParachain, - bitcoin_core: DynBitcoinCoreApi, + bitcoin_core_master: DynBitcoinCoreApi, + bitcoin_core_shared: DynBitcoinCoreApi, config: Config, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -87,7 +88,7 @@ impl ConnectionManager { let shutdown_tx = ShutdownSender::new(); let prefix = self.wallet_name.clone().unwrap_or_else(|| "vault".to_string()); - let bitcoin_core = self.bitcoin_config.new_client(Some(format!("{prefix}-master"))).await?; + let bitcoin_core_master = 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(); @@ -102,7 +103,14 @@ impl ConnectionManager { .await?; let config_copy = self.bitcoin_config.clone(); - let network_copy = bitcoin_core.network(); + 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 constructor = move |vault_id: VaultId| { let collateral_currency: CurrencyId = vault_id.collateral_currency(); let wrapped_currency: CurrencyId = vault_id.wrapped_currency(); @@ -121,7 +129,8 @@ impl ConnectionManager { let service = S::new_service( btc_parachain, - bitcoin_core, + bitcoin_core_master, + bitcoin_core_shared, config, self.monitoring_config.clone(), shutdown_tx.clone(), diff --git a/vault/src/execution.rs b/vault/src/execution.rs index 95e5d0f14..efe98fdf5 100644 --- a/vault/src/execution.rs +++ b/vault/src/execution.rs @@ -655,7 +655,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; @@ -699,6 +699,7 @@ 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; @@ -793,6 +794,7 @@ 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; @@ -801,8 +803,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_derivation_key(&self, public_key: &PublicKey) -> Result; - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError>; + fn dump_private_key(&self, address: &Address) -> Result; + fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> 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 15f5c04de..cd6bc55b7 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -151,6 +151,7 @@ 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? @@ -164,7 +165,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 30a469989..eb0d2ec83 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, parachain_rpc.clone()), - publish_required_collateral(vault, parachain_rpc.clone()), - publish_collateralization(vault, parachain_rpc.clone()), + publish_locked_collateral(vault, ¶chain_rpc), + publish_required_collateral(vault, ¶chain_rpc), + publish_collateralization(vault, ¶chain_rpc), ); } } @@ -335,7 +335,7 @@ fn raw_value_as_currency(value: u128, currency: CurrencyId) -> Result( vault: &VaultData, - parachain_rpc: P, + parachain_rpc: &P, ) -> Result<(), ServiceError> { 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 +346,7 @@ pub async fn publish_locked_collateral( pub async fn publish_required_collateral( vault: &VaultData, - parachain_rpc: P, + parachain_rpc: &P, ) -> Result<(), ServiceError> { if let Ok(required_collateral) = parachain_rpc .get_required_collateral_for_vault(vault.vault_id.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,9 +585,11 @@ pub async fn monitor_bridge_metrics( .iter() .filter(|vault| &vault.vault_id.collateral_currency() == currency_id) { - 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; + let _ = tokio::join!( + publish_locked_collateral(vault, parachain_rpc), + publish_required_collateral(vault, parachain_rpc), + publish_collateralization(vault, parachain_rpc), + ); } } }, @@ -794,6 +796,7 @@ 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; @@ -802,8 +805,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_derivation_key(&self, public_key: &PublicKey) -> Result; - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError>; + fn dump_private_key(&self, address: &Address) -> Result; + fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> 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; @@ -1082,7 +1085,7 @@ mod tests { metrics: PerCurrencyMetrics::dummy(), }; - publish_locked_collateral(&vault_data, parachain_rpc).await.unwrap(); + publish_locked_collateral(&vault_data, ¶chain_rpc).await.unwrap(); let total_collateral = vault_data.metrics.locked_collateral.get(); assert_eq!(total_collateral, 0.0000000075); @@ -1135,7 +1138,7 @@ mod tests { metrics: PerCurrencyMetrics::dummy(), }; - publish_collateralization(&vault_data, parachain_rpc).await; + publish_collateralization(&vault_data, ¶chain_rpc).await; let collateralization_metrics = vault_data.metrics.collateralization.get(); assert_eq!( @@ -1164,7 +1167,7 @@ mod tests { metrics: PerCurrencyMetrics::dummy(), }; - publish_required_collateral(&vault_data, parachain_rpc).await.unwrap(); + publish_required_collateral(&vault_data, ¶chain_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 c3f9ef465..07f6ebb8f 100644 --- a/vault/src/replace.rs +++ b/vault/src/replace.rs @@ -238,6 +238,7 @@ 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; @@ -246,8 +247,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_derivation_key(&self, public_key: &PublicKey) -> Result; - fn import_derivation_key(&self, private_key: &PrivateKey) -> Result<(), BitcoinError>; + fn dump_private_key(&self, address: &Address) -> Result; + fn import_private_key(&self, private_key: &PrivateKey, is_derivation_key: bool) -> 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 8941117d0..6edc6b3b9 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -8,7 +8,7 @@ use crate::{ Event, IssueRequests, CHAIN_HEIGHT_POLLING_INTERVAL, }; use async_trait::async_trait; -use bitcoin::{Error as BitcoinError, Network, PublicKey}; +use bitcoin::{Address, ConversionError, Error as BitcoinError, Network, PublicKey}; use clap::Parser; use futures::{ channel::{mpsc, mpsc::Sender}, @@ -224,7 +224,8 @@ pub struct VaultIdManager { vault_data: Arc>>, btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - // TODO: refactor this + btc_rpc_shared_wallet: DynBitcoinCoreApi, + // TODO: remove this #[allow(clippy::type_complexity)] constructor: Arc Result + Send + Sync>>, db: DatabaseConfig, @@ -234,6 +235,7 @@ 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 { @@ -241,6 +243,7 @@ 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 }, } @@ -250,6 +253,7 @@ 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 { @@ -270,6 +274,7 @@ 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(), @@ -286,25 +291,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)?; - if let Ok(private_key) = btc_rpc.dump_derivation_key(&public_key) { - self.btc_rpc_master_wallet.import_derivation_key(&private_key)?; - } + let address = Address::p2wpkh(&public_key, btc_rpc_master.network()) + .map_err(ConversionError::from) + .map_err(BitcoinError::ConversionError)?; // Copy the derivation key from the master wallet to use currency-specific wallet - match self.btc_rpc_master_wallet.dump_derivation_key(&public_key) { + match btc_rpc_master.dump_private_key(&address) { Ok(private_key) => { - btc_rpc.import_derivation_key(&private_key)?; + // TODO: remove this after the migration is complete + btc_rpc_shared.import_private_key(&private_key, true)?; } Err(err) => { tracing::error!("Could not find the derivation key in the bitcoin wallet"); @@ -312,15 +317,23 @@ impl VaultIdManager { } } - tracing::info!("Adding keys from past issues..."); + // 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()? { + // 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)?; + } - issue::add_keys_from_past_issue_request(&btc_rpc, &self.btc_parachain, &vault_id, &self.db).await?; + 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 { vault_id: vault_id.clone(), - btc_rpc: btc_rpc.clone(), + btc_rpc: btc_rpc_shared, metrics: metrics.clone(), }; PerCurrencyMetrics::initialize_values(self.btc_parachain.clone(), &data).await; @@ -370,6 +383,7 @@ 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()) } @@ -423,6 +437,7 @@ 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, @@ -432,6 +447,7 @@ impl Service for VaultService { VaultService::new( btc_parachain, btc_rpc_master_wallet, + btc_rpc_shared_wallet, config, monitoring_config, shutdown, @@ -508,6 +524,7 @@ impl VaultService { fn new( btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, + btc_rpc_shared_wallet: DynBitcoinCoreApi, config: VaultServiceConfig, monitoring_config: MonitoringConfig, shutdown: ShutdownSender, @@ -520,7 +537,13 @@ impl VaultService { config, monitoring_config, shutdown, - vault_id_manager: VaultIdManager::new(btc_parachain, btc_rpc_master_wallet, constructor, db_path), + vault_id_manager: VaultIdManager::new( + btc_parachain, + btc_rpc_master_wallet, + btc_rpc_shared_wallet, + constructor, + db_path, + ), } } From 1ea7cd621d61c00458706f9ddff0fefb8648bb0e Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Tue, 18 Jul 2023 17:23:44 +0000 Subject: [PATCH 2/6] refactor: merge duplicate bitcoin metrics Signed-off-by: Gregory Hill --- vault/src/metrics.rs | 133 ++++++++++++++++++------------------------- vault/src/system.rs | 2 +- 2 files changed, 56 insertions(+), 79 deletions(-) diff --git a/vault/src/metrics.rs b/vault/src/metrics.rs index eb0d2ec83..dcb0b095d 100644 --- a/vault/src/metrics.rs +++ b/vault/src/metrics.rs @@ -21,7 +21,7 @@ use runtime::{ }; use service::{ warp::{Rejection, Reply}, - Error as ServiceError, + DynBitcoinCoreApi, Error as ServiceError, }; use std::time::Duration; use tokio::{sync::RwLock, time::sleep}; @@ -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 BTC_BALANCE_TYPE_LABEL: &str = "type"; +const EXPECTED_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,9 +45,10 @@ lazy_static! { &[CURRENCY_LABEL] ) .expect("Failed to create prometheus metric"); - 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 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 LOCKED_COLLATERAL: GaugeVec = GaugeVec::new(Opts::new("locked_collateral", "Locked Collateral"), &[CURRENCY_LABEL]) .expect("Failed to create prometheus metric"); @@ -70,14 +71,13 @@ lazy_static! { &[TASK_NAME] ) .expect("Failed to create prometheus metric"); - 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] + 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] ) .expect("Failed to create prometheus metric"); pub static ref ISSUES: GaugeVec = GaugeVec::new( @@ -92,21 +92,22 @@ 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: GaugeVec = - GaugeVec::new(Opts::new("fee_budget_surplus", "Fee Budget Surplus"), &[CURRENCY_LABEL]) - .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 RESTART_COUNT: IntCounter = IntCounter::new("restart_count", "Number of service restarts").expect("Failed to create prometheus metric"); } #[derive(Clone, Debug)] -struct AverageTracker { +pub struct AverageTracker { total: u64, count: u64, } #[derive(Clone, Debug)] -struct StatefulGauge { +pub struct StatefulGauge { gauge: Gauge, data: Arc>, } @@ -115,7 +116,6 @@ struct StatefulGauge { struct BtcBalance { upperbound: Gauge, lowerbound: Gauge, - actual: Gauge, } #[derive(Clone, Debug)] @@ -134,9 +134,6 @@ pub struct PerCurrencyMetrics { btc_balance: BtcBalance, issues: RequestCounter, redeems: RequestCounter, - average_btc_fee: StatefulGauge, - fee_budget_surplus: StatefulGauge, - utxo_count: IntGauge, } #[async_trait] @@ -172,9 +169,10 @@ impl PerCurrencyMetrics { fn new_with_label(label: &str) -> Self { let labels = HashMap::from([(CURRENCY_LABEL, label)]); - 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 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 request_type_label = |balance_type: &'static str| { HashMap::<&str, &str>::from([(CURRENCY_LABEL, label), (REQUEST_STATUS_LABEL, balance_type)]) @@ -185,19 +183,9 @@ 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: btc_balance_gauge("required_upperbound"), - lowerbound: btc_balance_gauge("required_lowerbound"), - actual: btc_balance_gauge("actual"), + upperbound: expected_btc_balance_gauge("required_upperbound"), + lowerbound: expected_btc_balance_gauge("required_lowerbound"), }, issues: RequestCounter { open_count: ISSUES.with(&request_type_label("open")), @@ -248,7 +236,7 @@ impl PerCurrencyMetrics { .fold(0i64, |acc, x| async move { acc.saturating_add(x) }) .await; - *vault.metrics.fee_budget_surplus.data.write().await = fee_budget_surplus; + *FEE_BUDGET_SURPLUS.data.write().await = fee_budget_surplus; publish_fee_budget_surplus(vault).await?; } Ok(()) @@ -268,14 +256,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)); - *vault.metrics.average_btc_fee.data.write().await = AverageTracker { total, count }; + *AVERAGE_BTC_FEE.data.write().await = AverageTracker { total, count }; - publish_utxo_count(vault); - publish_bitcoin_balance(vault); + publish_utxo_count(&vault.btc_rpc); + publish_bitcoin_balance(&vault.btc_rpc); let _ = tokio::join!( Self::initialize_fee_budget_surplus(vault, parachain_rpc.clone(), bitcoin_transactions), - publish_average_bitcoin_fee(vault), + publish_average_bitcoin_fee(), publish_expected_bitcoin_balance(vault, parachain_rpc.clone()), publish_locked_collateral(vault, ¶chain_rpc), publish_required_collateral(vault, ¶chain_rpc), @@ -285,12 +273,13 @@ impl PerCurrencyMetrics { } pub fn register_custom_metrics() -> Result<(), RuntimeError> { - REGISTRY.register(Box::new(AVERAGE_BTC_FEE.clone()))?; + REGISTRY.register(Box::new(AVERAGE_BTC_FEE.gauge.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.clone()))?; - REGISTRY.register(Box::new(BTC_BALANCE.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(NATIVE_CURRENCY_BALANCE.clone()))?; REGISTRY.register(Box::new(ISSUES.clone()))?; REGISTRY.register(Box::new(REDEEMS.clone()))?; @@ -377,47 +366,45 @@ pub async fn update_bitcoin_metrics( // update the average fee if let Some(amount) = new_fee_entry { { - let mut tmp = vault.metrics.average_btc_fee.data.write().await; + let mut tmp = 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(vault).await; + publish_average_bitcoin_fee().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 = vault.metrics.fee_budget_surplus.data.write().await; + let mut tmp = FEE_BUDGET_SURPLUS.data.write().await; *tmp = tmp.saturating_add(surplus); } publish_fee_budget_surplus(vault).await?; } - publish_bitcoin_balance(vault); + publish_bitcoin_balance(&vault.btc_rpc); Ok(()) } 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 + let surplus = *FEE_BUDGET_SURPLUS.data.read().await; + FEE_BUDGET_SURPLUS .gauge .set(surplus as f64 / vault.vault_id.wrapped_currency().inner()?.one() as f64); Ok(()) } -async fn publish_average_bitcoin_fee(vault: &VaultData) { - let average = match vault.metrics.average_btc_fee.data.read().await { +async fn publish_average_bitcoin_fee() { + let average = match AVERAGE_BTC_FEE.data.read().await { x if x.count > 0 => x.total as f64 / x.count as f64, _ => Default::default(), }; - vault.metrics.average_btc_fee.gauge.set(average); + AVERAGE_BTC_FEE.gauge.set(average); } -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()), +fn publish_bitcoin_balance(btc_rpc: &DynBitcoinCoreApi) { + match btc_rpc.get_balance(None) { + Ok(bitcoin_balance) => ACTUAL_BTC_BALANCE.set(bitcoin_balance.to_btc()), Err(e) => { // unexpected error, but not critical so just continue tracing::warn!("Failed to get Bitcoin balance: {}", e); @@ -436,10 +423,10 @@ async fn publish_native_currency_balance>>, btc_parachain: InterBtcParachain, btc_rpc_master_wallet: DynBitcoinCoreApi, - btc_rpc_shared_wallet: DynBitcoinCoreApi, + pub(crate) btc_rpc_shared_wallet: DynBitcoinCoreApi, // TODO: remove this #[allow(clippy::type_complexity)] constructor: Arc Result + Send + Sync>>, From c3925ecff23cf8fae08c410b420d886a31d0e56c Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Tue, 18 Jul 2023 22:27:18 +0000 Subject: [PATCH 3/6] chore: duplicate rpc in test setup Signed-off-by: Gregory Hill --- vault/tests/vault_integration_tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/vault/tests/vault_integration_tests.rs b/vault/tests/vault_integration_tests.rs index 1ed336cd6..8fed6b781 100644 --- a/vault/tests/vault_integration_tests.rs +++ b/vault/tests/vault_integration_tests.rs @@ -94,6 +94,7 @@ 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", @@ -162,6 +163,7 @@ 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", @@ -175,6 +177,7 @@ 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", @@ -347,6 +350,7 @@ 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", @@ -613,6 +617,7 @@ 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", @@ -711,6 +716,7 @@ 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", @@ -798,6 +804,7 @@ 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", @@ -1200,6 +1207,7 @@ 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 189f7e2db9736315246332c253733f427a7b3807 Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Thu, 20 Jul 2023 20:55:45 +0000 Subject: [PATCH 4/6] refactor: import all issue keys Signed-off-by: Gregory Hill --- vault/src/issue.rs | 42 ++++++++++++++++++---------------------- vault/src/system.rs | 47 ++++++++++++++++++++++----------------------- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/vault/src/issue.rs b/vault/src/issue.rs index cd6bc55b7..d231c3fd9 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -1,12 +1,13 @@ use crate::{ - delay::RandomDelay, metrics::publish_expected_bitcoin_balance, Error, Event, IssueRequests, VaultIdManager, + delay::RandomDelay, metrics::publish_expected_bitcoin_balance, system::DatabaseConfig, Error, Event, IssueRequests, + VaultIdManager, }; use bitcoin::{BlockHash, Error as BitcoinError, PublicKey, Transaction, TransactionExt}; use futures::{channel::mpsc::Sender, future, SinkExt, StreamExt, TryFutureExt}; use runtime::{ - BtcAddress, BtcPublicKey, BtcRelayPallet, CancelIssueEvent, ExecuteIssueEvent, H256Le, InterBtcIssueRequest, - InterBtcParachain, IssuePallet, IssueRequestStatus, PartialAddress, PrettyPrint, RequestIssueEvent, UtilFuncs, - VaultId, H256, + AccountId, BtcAddress, BtcPublicKey, BtcRelayPallet, CancelIssueEvent, ExecuteIssueEvent, H256Le, + InterBtcIssueRequest, InterBtcParachain, IssuePallet, IssueRequestStatus, PartialAddress, PrettyPrint, + RequestIssueEvent, UtilFuncs, H256, }; use service::{DynBitcoinCoreApi, Error as ServiceError}; use sha2::{Digest, Sha256}; @@ -82,10 +83,11 @@ 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-v2"; + 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); @@ -133,11 +135,12 @@ impl RescanStatus { Some((start, chunk_end)) } - fn get(vault_id: &VaultId, db: &crate::system::DatabaseConfig) -> Result { - Ok(db.get(vault_id, Self::KEY)?.unwrap_or_default()) + fn get(account_id: &AccountId, db: &DatabaseConfig) -> Result { + Ok(db.get(account_id, Self::KEY)?.unwrap_or_default()) } - fn store(&self, vault_id: &VaultId, db: &crate::system::DatabaseConfig) -> Result<(), Error> { - db.put(vault_id, Self::KEY, self)?; + + fn store(&self, account_id: &AccountId, db: &DatabaseConfig) -> Result<(), Error> { + db.put(account_id, Self::KEY, self)?; Ok(()) } } @@ -145,19 +148,13 @@ impl RescanStatus { pub async fn add_keys_from_past_issue_request( bitcoin_core: &DynBitcoinCoreApi, btc_parachain: &InterBtcParachain, - vault_id: &VaultId, - db: &crate::system::DatabaseConfig, + db: &DatabaseConfig, ) -> Result<(), Error> { - let mut scanning_status = RescanStatus::get(vault_id, db)?; - tracing::info!("initial status: = {scanning_status:?}"); + let account_id = btc_parachain.get_account_id(); + let mut scanning_status = RescanStatus::get(&account_id, db)?; + tracing::info!("Scanning: {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? - .into_iter() - .filter(|(_, issue)| &issue.vault == vault_id) - .collect(); + 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 { @@ -198,7 +195,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(vault_id, db)?; + 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 @@ -218,7 +215,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(vault_id, db)?; + scanning_status.store(account_id, db)?; } Ok(()) @@ -466,7 +463,6 @@ 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 8f33fda39..430ede0fb 100644 --- a/vault/src/system.rs +++ b/vault/src/system.rs @@ -18,9 +18,9 @@ use futures::{ use git_version::git_version; use runtime::{ cli::{parse_duration_minutes, parse_duration_ms}, - BtcRelayPallet, CollateralBalancesPallet, CurrencyId, Error as RuntimeError, InterBtcParachain, PrettyPrint, - RegisterVaultEvent, RuntimeCurrencyInfo, StoreMainChainHeaderEvent, TryFromSymbol, UpdateActiveBlockEvent, - UtilFuncs, VaultCurrencyPair, VaultId, VaultRegistryPallet, + AccountId, BtcRelayPallet, CollateralBalancesPallet, CurrencyId, Error as RuntimeError, InterBtcParachain, + PrettyPrint, RegisterVaultEvent, StoreMainChainHeaderEvent, TryFromSymbol, UpdateActiveBlockEvent, UtilFuncs, + VaultCurrencyPair, VaultId, VaultRegistryPallet, }; use service::{wait_or_shutdown, DynBitcoinCoreApi, Error as ServiceError, MonitoringConfig, Service, ShutdownSender}; use std::{collections::HashMap, pin::Pin, sync::Arc, time::Duration}; @@ -178,35 +178,25 @@ pub struct DatabaseConfig { } impl DatabaseConfig { - fn prefixed_key(vault_id: &VaultId, key: &str) -> Result { + fn prefixed_key(account_id: &AccountId, key: &str) -> Result { Ok(format!( - "{}-{}-{}-{}", - 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)?, + "{}-{}", + account_id.pretty_print(), /* technically not needed since each client should have their own + * db, but doesn't hurt to be safe */ key )) } - pub fn put(&self, vault_id: &VaultId, key: &str, value: &V) -> Result<(), Error> { + pub fn put(&self, account_id: &AccountId, key: &str, value: &V) -> Result<(), Error> { let db = rocksdb::DB::open_default(self.path.clone())?; - let key = Self::prefixed_key(vault_id, key)?; + let key = Self::prefixed_key(account_id, key)?; db.put(key, serde_json::to_vec(value)?)?; Ok(()) } - pub fn get(&self, vault_id: &VaultId, key: &str) -> Result, Error> { + pub fn get(&self, account_id: &AccountId, key: &str) -> Result, Error> { let db = rocksdb::DB::open_default(self.path.clone())?; - let key = Self::prefixed_key(vault_id, key)?; + let key = Self::prefixed_key(account_id, key)?; let value = match db.get(key)? { None => return Ok(None), @@ -317,6 +307,7 @@ 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()? { @@ -326,9 +317,6 @@ 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 { @@ -350,6 +338,7 @@ impl VaultIdManager { .await? { match is_vault_registered(&self.btc_parachain, &vault_id).await { + // TODO: import keys for liquidated vaults? Err(Error::RuntimeError(RuntimeError::VaultLiquidated)) => { tracing::error!( "[{}] Vault is liquidated -- not going to process events for this vault.", @@ -423,6 +412,7 @@ 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, @@ -534,6 +524,7 @@ 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, @@ -645,6 +636,14 @@ 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?; + let startup_height = self.await_parachain_block().await?; let open_request_executor = execute_open_requests( From 94067759d7f4dbf06ec2aba8e76acca64ff2d9b1 Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Thu, 20 Jul 2023 23:09:18 +0000 Subject: [PATCH 5/6] fix: manually parse listaddressgroupings Signed-off-by: Gregory Hill --- bitcoin/Cargo.toml | 2 +- bitcoin/src/lib.rs | 31 ++++++++++++++------------ bitcoin/tests/integration_test.rs | 16 +++++++++++++ vault/tests/vault_integration_tests.rs | 8 +++---- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index ea1a7e0fd..dd203384a 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -8,7 +8,7 @@ edition = "2018" default = [] regtest-manual-mining = [] cli = ["clap"] -uses-bitcoind = [] +uses-bitcoind = ["regtest-manual-mining"] light-client = [] [dependencies] diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 0112406eb..698d99248 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -51,6 +51,7 @@ pub use sp_core::H256; use std::{ convert::TryInto, future::Future, + str::FromStr, sync::Arc, time::{Duration, Instant}, }; @@ -533,10 +534,15 @@ impl BitcoinCore { } #[cfg(feature = "regtest-manual-mining")] - pub fn mine_block(&self) -> Result { - Ok(self - .rpc - .generate_to_address(1, &self.rpc.get_new_address(None, Some(AddressType::Bech32))?)?[0]) + pub fn mine_blocks(&self, block_num: u64, maybe_address: Option
) -> BlockHash { + let address = + maybe_address.unwrap_or_else(|| self.rpc.get_new_address(None, Some(AddressType::Bech32)).unwrap()); + self.rpc + .generate_to_address(block_num, &address) + .unwrap() + .last() + .unwrap() + .clone() } async fn with_retry_on_timeout(&self, call: F) -> Result @@ -696,19 +702,16 @@ impl BitcoinCoreApi for BitcoinCore { // TODO: remove this once the wallet migration has completed fn list_addresses(&self) -> Result, Error> { - #[derive(Clone, PartialEq, Eq, Debug, serde::Deserialize)] - pub struct ListAddressGroupingResult { - pub address: Address, - #[serde(with = "bitcoincore_rpc::bitcoin::util::amount::serde::as_btc")] - pub amount: SignedAmount, - pub label: Option, - } - // 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", &[])?; - Ok(groupings.into_iter().flatten().map(|group| group.address).collect()) + 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)?.ok()) + .collect::>(); + Ok(addresses) } /// Get the raw transaction identified by `Txid` and stored diff --git a/bitcoin/tests/integration_test.rs b/bitcoin/tests/integration_test.rs index 865a1c734..938f6d63b 100644 --- a/bitcoin/tests/integration_test.rs +++ b/bitcoin/tests/integration_test.rs @@ -26,6 +26,22 @@ 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/tests/vault_integration_tests.rs b/vault/tests/vault_integration_tests.rs index 8fed6b781..446b5e9a2 100644 --- a/vault/tests/vault_integration_tests.rs +++ b/vault/tests/vault_integration_tests.rs @@ -988,9 +988,7 @@ mod test_with_bitcoind { ret.create_or_load_wallet().await.unwrap(); // fund the wallet by mining blocks - for _ in 0..102 { - ret.mine_block().unwrap(); - } + ret.mine_blocks(102, None); ret } @@ -1090,7 +1088,7 @@ mod test_with_bitcoind { })); tracing::trace!("Step 4: mine bitcoin block"); - let block_hash = btc_rpc.mine_block().unwrap(); + let block_hash = btc_rpc.mine_blocks(1, None); tracing::info!("Step 5: check that tx got included without changes"); btc_rpc @@ -1160,7 +1158,7 @@ mod test_with_bitcoind { assert!(btc_rpc.fee_rate(new_tx.txid()).await.unwrap().0 >= 10); tracing::trace!("Step 5: mine bitcoin block"); - let block_hash = btc_rpc.mine_block().unwrap(); + let block_hash = btc_rpc.mine_blocks(1, None); tracing::trace!("Step 6: check that only new tx got included"); btc_rpc.get_transaction(&new_tx.txid(), Some(block_hash)).await.unwrap(); From f8b2f8d9b02502bb36eeec06c089e9064227e5d1 Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Fri, 21 Jul 2023 15:50:22 +0000 Subject: [PATCH 6/6] chore: fix vaultid import Signed-off-by: Gregory Hill --- vault/src/issue.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vault/src/issue.rs b/vault/src/issue.rs index d231c3fd9..5c2720677 100644 --- a/vault/src/issue.rs +++ b/vault/src/issue.rs @@ -465,6 +465,7 @@ mod tests { subxt::utils::Static, CurrencyId::Token, TokenSymbol::{DOT, IBTC, INTR}, + VaultId, }; fn dummy_issues(heights: Vec<(u32, usize)>) -> Vec {