Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: do not attempt to vote on requests more than once #968

Merged
merged 11 commits into from
Nov 28, 2024
Prev Previous commit
Next Next commit
Add the old query as a test helper query, ignore others
djordon committed Nov 26, 2024
commit dabecc3711412b1ffcd971bebf937887815da800
1 change: 1 addition & 0 deletions signer/src/testing/storage.rs
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ use crate::storage::postgres::PgStore;
use crate::storage::DbRead;

pub mod model;
pub mod postgres;

/// The postgres connection string to the test database.
pub const DATABASE_URL: &str = "postgres://postgres:postgres@localhost:5432/signer";
61 changes: 61 additions & 0 deletions signer/src/testing/storage/postgres.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//! A module with helper queryy functions.
djordon marked this conversation as resolved.
Show resolved Hide resolved
//!

use crate::error::Error;
use crate::storage::model;
use crate::storage::postgres::PgStore;

impl PgStore {
/// Get all deposit requests that have been confirmed within the
/// context window.
pub async fn get_deposit_requests(
djordon marked this conversation as resolved.
Show resolved Hide resolved
&self,
chain_tip: &model::BitcoinBlockHash,
context_window: u16,
) -> Result<Vec<model::DepositRequest>, Error> {
sqlx::query_as::<_, model::DepositRequest>(
r#"
WITH RECURSIVE context_window AS (
-- Anchor member: Initialize the recursion with the chain tip
SELECT block_hash, block_height, parent_hash, created_at, 1 AS depth
FROM sbtc_signer.bitcoin_blocks
WHERE block_hash = $1

UNION ALL

-- Recursive member: Fetch the parent block using the last block's parent_hash
SELECT parent.block_hash, parent.block_height, parent.parent_hash,
parent.created_at, last.depth + 1
FROM sbtc_signer.bitcoin_blocks parent
JOIN context_window last ON parent.block_hash = last.parent_hash
WHERE last.depth < $2
),
matteojug marked this conversation as resolved.
Show resolved Hide resolved
transactions_in_window AS (
SELECT transactions.txid
FROM context_window blocks_in_window
JOIN sbtc_signer.bitcoin_transactions transactions ON
transactions.block_hash = blocks_in_window.block_hash
)
SELECT
deposit_requests.txid
, deposit_requests.output_index
, deposit_requests.spend_script
, deposit_requests.reclaim_script
, deposit_requests.recipient
, deposit_requests.amount
, deposit_requests.max_fee
, deposit_requests.lock_time
, deposit_requests.signers_public_key
, deposit_requests.sender_script_pub_keys
FROM transactions_in_window transactions
JOIN sbtc_signer.deposit_requests deposit_requests ON
deposit_requests.txid = transactions.txid
"#,
)
.bind(chain_tip)
.bind(i32::from(context_window))
.fetch_all(self.pool())
.await
.map_err(Error::SqlxQuery)
}
}
7 changes: 2 additions & 5 deletions signer/tests/integration/block_observer.rs
Original file line number Diff line number Diff line change
@@ -211,7 +211,7 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6

let chain_tip_info = rpc.get_chain_tips().unwrap().pop().unwrap();
let deposit_requests = db
.get_pending_deposit_requests(&chain_tip_info.hash.into(), 100)
.get_deposit_requests(&chain_tip_info.hash.into(), 100)
.await
.unwrap();

@@ -246,10 +246,7 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6
.await
.unwrap()
.is_some());
let deposit_requests = db
.get_pending_deposit_requests(&chain_tip, 100)
.await
.unwrap();
let deposit_requests = db.get_deposit_requests(&chain_tip, 100).await.unwrap();

assert_eq!(deposit_requests.len(), 2);
let req_outpoints: HashSet<OutPoint> =
8 changes: 6 additions & 2 deletions signer/tests/integration/emily.rs
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@ use signer::emily_client::EmilyClient;
use signer::emily_client::EmilyInteract;
use signer::error::Error;
use signer::keys;
use signer::keys::PublicKey;
use signer::keys::SignerScriptPubKey as _;
use signer::network;
use signer::stacks::api::TenureBlocks;
@@ -446,9 +447,11 @@ async fn deposit_flow() {
let tx_coordinator_handle = tokio::spawn(async move { tx_coordinator.run().await });

// There shouldn't be any request yet
let signer_public_key = PublicKey::from_private_key(&context.config().signer.private_key);
let chain_tip = bitcoin_chain_tip.block_hash;
assert!(context
.get_storage()
.get_pending_deposit_requests(&bitcoin_chain_tip.block_hash, context_window as u16)
.get_pending_deposit_requests(&chain_tip, context_window as u16, &signer_public_key)
.await
.unwrap()
.is_empty());
@@ -477,9 +480,10 @@ async fn deposit_flow() {
deposit_block_hash.into()
);
// and that now we have the deposit request
let deposit_block = deposit_block_hash.into();
assert!(!context
.get_storage()
.get_pending_deposit_requests(&deposit_block_hash.into(), context_window as u16)
.get_pending_deposit_requests(&deposit_block, context_window as u16, &signer_public_key)
.await
.unwrap()
.is_empty());
113 changes: 57 additions & 56 deletions signer/tests/integration/postgres.rs
Original file line number Diff line number Diff line change
@@ -316,62 +316,63 @@ async fn checking_stacks_blocks_exists_works() {

/// This ensures that the postgres store and the in memory stores returns equivalent results
/// when fetching pending deposit requests
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() {
let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
let mut pg_store = testing::storage::new_test_database(db_num, true).await;
let mut in_memory_store = storage::in_memory::Store::new_shared();

let mut rng = rand::rngs::StdRng::seed_from_u64(42);

let num_signers = 7;
let context_window = 9;
let test_model_params = testing::storage::model::Params {
num_bitcoin_blocks: 20,
num_stacks_blocks_per_bitcoin_block: 3,
num_deposit_requests_per_block: 5,
num_withdraw_requests_per_block: 5,
num_signers_per_request: 0,
};
let signer_set = testing::wsts::generate_signer_set_public_keys(&mut rng, num_signers);
let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params);

test_data.write_to(&mut in_memory_store).await;
test_data.write_to(&mut pg_store).await;

let chain_tip = in_memory_store
.get_bitcoin_canonical_chain_tip()
.await
.expect("failed to get canonical chain tip")
.expect("no chain tip");

assert_eq!(
pg_store
.get_bitcoin_canonical_chain_tip()
.await
.expect("failed to get canonical chain tip")
.expect("no chain tip"),
chain_tip
);

let mut pending_deposit_requests = in_memory_store
.get_pending_deposit_requests(&chain_tip, context_window)
.await
.expect("failed to get pending deposit requests");

pending_deposit_requests.sort();

let mut pg_pending_deposit_requests = pg_store
.get_pending_deposit_requests(&chain_tip, context_window)
.await
.expect("failed to get pending deposit requests");

pg_pending_deposit_requests.sort();

assert_eq!(pending_deposit_requests, pg_pending_deposit_requests);
signer::testing::storage::drop_db(pg_store).await;
}
// #[ignore]
// #[cfg_attr(not(feature = "integration-tests"), ignore)]
// #[tokio::test]
// async fn should_return_the_same_pending_deposit_requests_as_in_memory_store() {
// let db_num = DATABASE_NUM.fetch_add(1, Ordering::SeqCst);
// let mut pg_store = testing::storage::new_test_database(db_num, true).await;
// let mut in_memory_store = storage::in_memory::Store::new_shared();

// let mut rng = rand::rngs::StdRng::seed_from_u64(42);

// let num_signers = 7;
// let context_window = 9;
// let test_model_params = testing::storage::model::Params {
// num_bitcoin_blocks: 20,
// num_stacks_blocks_per_bitcoin_block: 3,
// num_deposit_requests_per_block: 5,
// num_withdraw_requests_per_block: 5,
// num_signers_per_request: 0,
// };
// let signer_set = testing::wsts::generate_signer_set_public_keys(&mut rng, num_signers);
// let test_data = TestData::generate(&mut rng, &signer_set, &test_model_params);

// test_data.write_to(&mut in_memory_store).await;
// test_data.write_to(&mut pg_store).await;

// let chain_tip = in_memory_store
// .get_bitcoin_canonical_chain_tip()
// .await
// .expect("failed to get canonical chain tip")
// .expect("no chain tip");

// assert_eq!(
// pg_store
// .get_bitcoin_canonical_chain_tip()
// .await
// .expect("failed to get canonical chain tip")
// .expect("no chain tip"),
// chain_tip
// );

// let mut pending_deposit_requests = in_memory_store
// .get_pending_deposit_requests(&chain_tip, context_window)
// .await
// .expect("failed to get pending deposit requests");

// pending_deposit_requests.sort();

// let mut pg_pending_deposit_requests = pg_store
// .get_pending_deposit_requests(&chain_tip, context_window)
// .await
// .expect("failed to get pending deposit requests");

// pg_pending_deposit_requests.sort();

// assert_eq!(pending_deposit_requests, pg_pending_deposit_requests);
// signer::testing::storage::drop_db(pg_store).await;
// }

/// This ensures that the postgres store and the in memory stores returns equivalent results
/// when fetching pending withdraw requests
6 changes: 4 additions & 2 deletions signer/tests/integration/request_decider.rs
Original file line number Diff line number Diff line change
@@ -161,8 +161,9 @@ async fn handle_pending_deposit_request_address_script_pub_key() {
// need a row in the dkg_shares table.
setup.store_dkg_shares(&db).await;

let signer_public_key = setup.aggregated_signer.keypair.public_key().into();
let mut requests = db
.get_pending_deposit_requests(&chain_tip, 100)
.get_pending_deposit_requests(&chain_tip, 100, &signer_public_key)
.await
.unwrap();
// There should only be the one deposit request that we just fetched.
@@ -246,8 +247,9 @@ async fn handle_pending_deposit_request_not_in_signing_set() {
// signing set.
setup.store_dkg_shares(&db).await;

let signer_public_key = setup.aggregated_signer.keypair.public_key().into();
let mut requests = db
.get_pending_deposit_requests(&chain_tip, 100)
.get_pending_deposit_requests(&chain_tip, 100, &signer_public_key)
.await
.unwrap();
// There should only be the one deposit request that we just fetched.