From e184cfdb2935a2db670ed962eef665a026f009d5 Mon Sep 17 00:00:00 2001 From: Matteo Almanza Date: Tue, 1 Oct 2024 00:15:55 +0200 Subject: [PATCH] chore: CR nits --- signer/src/storage/in_memory.rs | 10 ++-- signer/src/storage/mod.rs | 42 +---------------- signer/src/storage/postgres.rs | 36 ++++++++++----- signer/src/storage/util.rs | 46 +++++++++++++++++++ signer/src/testing/transaction_coordinator.rs | 22 +++++++-- signer/src/transaction_coordinator.rs | 7 ++- 6 files changed, 103 insertions(+), 60 deletions(-) create mode 100644 signer/src/storage/util.rs diff --git a/signer/src/storage/in_memory.rs b/signer/src/storage/in_memory.rs index fbe7942d..8c5e8898 100644 --- a/signer/src/storage/in_memory.rs +++ b/signer/src/storage/in_memory.rs @@ -1,10 +1,10 @@ //! In-memory store implementation - useful for tests -use bitcoin::consensus::Decodable; +use bitcoin::consensus::Decodable as _; use bitcoin::OutPoint; use blockstack_lib::types::chainstate::StacksBlockId; -use futures::StreamExt; -use futures::TryStreamExt; +use futures::StreamExt as _; +use futures::TryStreamExt as _; use std::collections::HashMap; use std::sync::Arc; use tokio::sync::Mutex; @@ -19,7 +19,7 @@ use crate::stacks::events::WithdrawalCreateEvent; use crate::stacks::events::WithdrawalRejectEvent; use crate::storage::model; -use super::get_utxo; +use super::util::get_utxo; /// A store wrapped in an Arc> for interior mutability pub type SharedStore = Arc>; @@ -426,6 +426,7 @@ impl super::DbRead for SharedStore { &self, chain_tip: &model::BitcoinBlockHash, aggregate_key: &PublicKey, + context_window: u16, ) -> Result, Error> { let script_pubkey = aggregate_key.signers_script_pubkey(); let store = self.lock().await; @@ -434,6 +435,7 @@ impl super::DbRead for SharedStore { // Traverse the canonical chain backwards and find the first block containing relevant sbtc tx(s) let sbtc_txs = std::iter::successors(first, |block| bitcoin_blocks.get(&block.parent_hash)) + .take(context_window as usize) .filter_map(|block| { let txs = store.bitcoin_block_to_transactions.get(&block.block_hash)?; diff --git a/signer/src/storage/mod.rs b/signer/src/storage/mod.rs index a6481099..ce771139 100644 --- a/signer/src/storage/mod.rs +++ b/signer/src/storage/mod.rs @@ -10,8 +10,8 @@ pub mod in_memory; pub mod model; pub mod postgres; pub mod sqlx; +pub mod util; -use std::collections::HashSet; use std::future::Future; use blockstack_lib::types::chainstate::StacksBlockId; @@ -19,7 +19,6 @@ use blockstack_lib::types::chainstate::StacksBlockId; use crate::bitcoin::utxo::SignerUtxo; use crate::error::Error; use crate::keys::PublicKey; -use crate::keys::SignerScriptPubKey as _; use crate::stacks::events::CompletedDepositEvent; use crate::stacks::events::WithdrawalAcceptEvent; use crate::stacks::events::WithdrawalCreateEvent; @@ -149,6 +148,7 @@ pub trait DbRead { &self, chain_tip: &model::BitcoinBlockHash, aggregate_key: &crate::keys::PublicKey, + context_window: u16, ) -> impl Future, Error>> + Send; /// For the given outpoint and aggregate key, get the list all signer @@ -302,41 +302,3 @@ pub trait DbWrite { event: &CompletedDepositEvent, ) -> impl Future> + Send; } - -/// Given the sbtc txs in a block, returns the `aggregate_key` utxo (if there's exactly one) -pub fn get_utxo( - aggregate_key: &PublicKey, - sbtc_txs: Vec, -) -> Result, Error> { - let script_pubkey = aggregate_key.signers_script_pubkey(); - - let spent: HashSet = sbtc_txs - .iter() - .flat_map(|tx| tx.input.iter().map(|txin| txin.previous_output)) - .collect(); - - let utxos = sbtc_txs - .iter() - .flat_map(|tx| { - if let Some(tx_out) = tx.output.first() { - let outpoint = bitcoin::OutPoint::new(tx.compute_txid(), 0); - if tx_out.script_pubkey == *script_pubkey && !spent.contains(&outpoint) { - return Some(SignerUtxo { - outpoint, - amount: tx_out.value.to_sat(), - // Txs are filtered based on the `aggregate_key` script pubkey - public_key: bitcoin::XOnlyPublicKey::from(aggregate_key), - }); - } - } - - None - }) - .collect::>(); - - match utxos[..] { - [] => Ok(None), - [utxo] => Ok(Some(utxo)), - _ => Err(Error::TooManySignerUtxos), - } -} diff --git a/signer/src/storage/postgres.rs b/signer/src/storage/postgres.rs index a352ace7..04734a66 100644 --- a/signer/src/storage/postgres.rs +++ b/signer/src/storage/postgres.rs @@ -3,13 +3,13 @@ use std::collections::HashMap; use std::sync::OnceLock; -use bitcoin::consensus::Decodable; +use bitcoin::consensus::Decodable as _; use bitcoin::hashes::Hash as _; use blockstack_lib::chainstate::nakamoto::NakamotoBlock; use blockstack_lib::chainstate::stacks::TransactionPayload; use blockstack_lib::codec::StacksMessageCodec; use blockstack_lib::types::chainstate::StacksBlockId; -use futures::StreamExt; +use futures::StreamExt as _; use sqlx::PgExecutor; use crate::bitcoin::utxo::SignerUtxo; @@ -23,7 +23,7 @@ use crate::stacks::events::WithdrawalRejectEvent; use crate::storage::model; use crate::storage::model::TransactionType; -use super::get_utxo; +use super::util::get_utxo; /// All migration scripts from the `signer/migrations` directory. static PGSQL_MIGRATIONS: include_dir::Dir = @@ -953,35 +953,43 @@ impl super::DbRead for PgStore { &self, chain_tip: &model::BitcoinBlockHash, aggregate_key: &PublicKey, + context_window: u16, ) -> Result, Error> { let script_pubkey = aggregate_key.signers_script_pubkey(); - let mut txs = sqlx::query_as::<_, model::Transaction>( r#" WITH RECURSIVE tx_block_chain AS ( - SELECT + SELECT block_hash , parent_hash + , 1 AS depth FROM sbtc_signer.bitcoin_blocks WHERE block_hash = $1 - + UNION ALL - + SELECT parent.block_hash , parent.parent_hash + , child.depth + 1 FROM sbtc_signer.bitcoin_blocks AS parent - JOIN tx_block_chain AS child - ON child.parent_hash = parent.block_hash + JOIN tx_block_chain AS child ON child.parent_hash = parent.block_hash + WHERE child.depth < $2 ) - SELECT txs.txid, txs.tx, txs.tx_type, tbc.block_hash + SELECT + txs.txid + , txs.tx + , txs.tx_type + , tbc.block_hash FROM tx_block_chain AS tbc JOIN sbtc_signer.bitcoin_transactions AS bt ON tbc.block_hash = bt.block_hash JOIN sbtc_signer.transactions AS txs USING (txid) - WHERE txs.tx_type = 'sbtc_transaction'; + WHERE txs.tx_type = 'sbtc_transaction' + ORDER BY tbc.depth ASC; "#, ) .bind(chain_tip) + .bind(context_window as i32) .fetch(&self.0); let mut utxo_block = None; @@ -1007,7 +1015,11 @@ impl super::DbRead for PgStore { // Fetch all the sbtc txs in the same block let sbtc_txs = sqlx::query_as::<_, model::Transaction>( r#" - SELECT txs.txid, txs.tx, txs.tx_type, bt.block_hash + SELECT + txs.txid + , txs.tx + , txs.tx_type + , bt.block_hash FROM sbtc_signer.transactions AS txs JOIN sbtc_signer.bitcoin_transactions AS bt USING (txid) WHERE txs.tx_type = 'sbtc_transaction' AND bt.block_hash = $1; diff --git a/signer/src/storage/util.rs b/signer/src/storage/util.rs new file mode 100644 index 00000000..f440bb73 --- /dev/null +++ b/signer/src/storage/util.rs @@ -0,0 +1,46 @@ +//! General utilities for the storage. + +use std::collections::HashSet; + +use crate::bitcoin::utxo::SignerUtxo; +use crate::error::Error; +use crate::keys::PublicKey; +use crate::keys::SignerScriptPubKey as _; + +/// Given the sbtc txs in a block, returns the `aggregate_key` utxo (if there's exactly one) +pub fn get_utxo( + aggregate_key: &PublicKey, + sbtc_txs: Vec, +) -> Result, Error> { + let script_pubkey = aggregate_key.signers_script_pubkey(); + + let spent: HashSet = sbtc_txs + .iter() + .flat_map(|tx| tx.input.iter().map(|txin| txin.previous_output)) + .collect(); + + let utxos = sbtc_txs + .iter() + .flat_map(|tx| { + if let Some(tx_out) = tx.output.first() { + let outpoint = bitcoin::OutPoint::new(tx.compute_txid(), 0); + if tx_out.script_pubkey == *script_pubkey && !spent.contains(&outpoint) { + return Some(SignerUtxo { + outpoint, + amount: tx_out.value.to_sat(), + // Txs are filtered based on the `aggregate_key` script pubkey + public_key: bitcoin::XOnlyPublicKey::from(aggregate_key), + }); + } + } + + None + }) + .collect::>(); + + match utxos[..] { + [] => Ok(None), + [utxo] => Ok(Some(utxo)), + _ => Err(Error::TooManySignerUtxos), + } +} diff --git a/signer/src/testing/transaction_coordinator.rs b/signer/src/testing/transaction_coordinator.rs index dfa5f9b2..b3d9a692 100644 --- a/signer/src/testing/transaction_coordinator.rs +++ b/signer/src/testing/transaction_coordinator.rs @@ -283,7 +283,7 @@ where assert_eq!(chain_tip, block_ref.block_hash); let signer_utxo = storage - .get_signer_utxo(&chain_tip, &aggregate_key) + .get_signer_utxo(&chain_tip, &aggregate_key, self.context_window as u16) .await .unwrap() .expect("no signer utxo"); @@ -380,12 +380,28 @@ where public_key: bitcoin::XOnlyPublicKey::from(aggregate_key), }; let signer_utxo = storage - .get_signer_utxo(&chain_tip.block_hash, &aggregate_key) + .get_signer_utxo( + &chain_tip.block_hash, + &aggregate_key, + self.context_window as u16, + ) .await .unwrap() .expect("no signer utxo"); assert_eq!(signer_utxo, expected); } + + // Check context window + assert!(storage + .get_signer_utxo(&block_c2.block_hash, &aggregate_key, 1) + .await + .unwrap() + .is_none()); + assert!(storage + .get_signer_utxo(&block_c2.block_hash, &aggregate_key, 2) + .await + .unwrap() + .is_some()); } /// Assert we get the correct UTXO with a spending chain in a block @@ -469,7 +485,7 @@ where assert_eq!(chain_tip, block_ref.block_hash); let signer_utxo = storage - .get_signer_utxo(&chain_tip, &aggregate_key) + .get_signer_utxo(&chain_tip, &aggregate_key, self.context_window as u16) .await .unwrap() .expect("no signer utxo"); diff --git a/signer/src/transaction_coordinator.rs b/signer/src/transaction_coordinator.rs index 78f6fcc1..103baf33 100644 --- a/signer/src/transaction_coordinator.rs +++ b/signer/src/transaction_coordinator.rs @@ -405,9 +405,14 @@ where let Some(chain_tip) = self.storage.get_bitcoin_canonical_chain_tip().await? else { return Err(Error::NoChainTip); }; + + let context_window = self + .context_window + .try_into() + .map_err(|_| Error::TypeConversion)?; let utxo = self .storage - .get_signer_utxo(&chain_tip, aggregate_key) + .get_signer_utxo(&chain_tip, aggregate_key, context_window) .await? .ok_or(Error::MissingSignerUtxo)?; let last_fees = self.bitcoin_client.get_last_fee(utxo.outpoint).await?;