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: db stacks transactions validated incorrectly #599

Merged
merged 3 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion signer/src/block_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,8 @@ where
&mut self,
blocks: &[nakamoto::NakamotoBlock],
) -> Result<(), Error> {
let txs = storage::postgres::extract_relevant_transactions(blocks);
let deployer = &self.context.config().signer.deployer;
let txs = storage::postgres::extract_relevant_transactions(blocks, deployer);
let headers = blocks
.iter()
.map(model::StacksBlock::try_from)
Expand Down
45 changes: 40 additions & 5 deletions signer/src/storage/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use blockstack_lib::codec::StacksMessageCodec;
use blockstack_lib::types::chainstate::StacksBlockId;
use futures::StreamExt as _;
use sqlx::PgExecutor;
use stacks_common::types::chainstate::StacksAddress;

use crate::bitcoin::utxo::SignerUtxo;
use crate::error::Error;
Expand Down Expand Up @@ -64,14 +65,20 @@ fn contract_transaction_kinds() -> &'static HashMap<&'static str, TransactionTyp

/// This function extracts the signer relevant sBTC related transactions
/// from the given blocks.
pub fn extract_relevant_transactions(blocks: &[NakamotoBlock]) -> Vec<model::Transaction> {
///
/// Here the deployer is the address that deployed the sBTC smart
/// contracts.
pub fn extract_relevant_transactions(
blocks: &[NakamotoBlock],
deployer: &StacksAddress,
) -> Vec<model::Transaction> {
let transaction_kinds = contract_transaction_kinds();
blocks
.iter()
.flat_map(|block| block.txs.iter().map(|tx| (tx, block.block_id())))
.filter_map(|(tx, block_id)| match &tx.payload {
TransactionPayload::ContractCall(x)
if CONTRACT_NAMES.contains(&x.contract_name.as_str()) =>
if CONTRACT_NAMES.contains(&x.contract_name.as_str()) && &x.address == deployer =>
{
Some(model::Transaction {
txid: tx.txid().into_bytes(),
Expand Down Expand Up @@ -1730,22 +1737,50 @@ mod tests {
blocks.push(NakamotoBlock::consensus_deserialize(bytes).unwrap());
}

let txs = extract_relevant_transactions(&blocks);
let deployer = StacksAddress::burn_address(false);
let txs = extract_relevant_transactions(&blocks, &deployer);
assert!(txs.is_empty());

let last_block = blocks.last_mut().unwrap();
let mut tx = last_block.txs.last().unwrap().clone();

let contract_call = TransactionContractCall {
address: StacksAddress::new(2, Hash160([0u8; 20])),
address: deployer,
contract_name: ContractName::from(contract_name),
function_name: ClarityName::from(function_name),
function_args: Vec::new(),
};
tx.payload = TransactionPayload::ContractCall(contract_call);
last_block.txs.push(tx);

let txs = extract_relevant_transactions(&blocks);
let txs = extract_relevant_transactions(&blocks, &deployer);
assert_eq!(txs.len(), 1);

// We've just seen that if the deployer supplied here matches the
// address in the transaction, then we will consider it a relevant
// transaction. Now what if someone tries to pull a fast one by
// deploying their own modified version of the sBTC smart contracts
// and creating contract calls against that? We'll the address of
// these contract calls won't match the ones that we are interested
// in and we will filter them out. We test that now,
let contract_call = TransactionContractCall {
// This is the address of the poser that deployed their own
// versions of the sBTC smart contracts.
address: StacksAddress::new(2, Hash160([1; 20])),
contract_name: ContractName::from(contract_name),
function_name: ClarityName::from(function_name),
function_args: Vec::new(),
};
// The last transaction in the last nakamoto block is a legit
// transaction. Let's remove it and replace it with a non-legit
// one.
let last_block = blocks.last_mut().unwrap();
let mut tx = last_block.txs.pop().unwrap();
tx.payload = TransactionPayload::ContractCall(contract_call);
last_block.txs.push(tx);

// Now there aren't any relevant transactions in the block
let txs = extract_relevant_transactions(&blocks, &deployer);
assert!(txs.is_empty());
}
}
3 changes: 2 additions & 1 deletion signer/src/testing/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ use crate::stacks::contracts::StacksTxPostConditions;
use crate::stacks::wallet::SignerWallet;

/// A static for a test 2-3 multi-sig wallet. This wallet is loaded with
/// funds in the local devnet environment.
/// funds in the local devnet environment. It matches the signer.deployer
/// address in the default config file.
pub static WALLET: LazyLock<(SignerWallet, [Keypair; 3], u16)> = LazyLock::new(generate_wallet);

/// Helper function for generating a test 2-3 multi-sig wallet
Expand Down
2 changes: 1 addition & 1 deletion signer/tests/integration/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ async fn complete_deposit_wrapper_tx_accepted<T: AsContractCall>(contract: Contr
.await
.unwrap();

let transactions = postgres::extract_relevant_transactions(&blocks);
let transactions = postgres::extract_relevant_transactions(&blocks, &signer.wallet.address());
assert!(!transactions.is_empty());

let txids = transactions
Expand Down
13 changes: 9 additions & 4 deletions signer/tests/integration/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,16 @@ async fn should_be_able_to_query_bitcoin_blocks() {
signer::testing::storage::drop_db(store).await;
}

struct InitiateWithdrawalRequest;
struct InitiateWithdrawalRequest {
deployer: StacksAddress,
}

impl AsContractCall for InitiateWithdrawalRequest {
const CONTRACT_NAME: &'static str = "sbtc-withdrawal";
cylewitruk marked this conversation as resolved.
Show resolved Hide resolved
const FUNCTION_NAME: &'static str = "initiate-withdrawal-request";
/// The stacks address that deployed the contract.
fn deployer_address(&self) -> StacksAddress {
StacksAddress::burn_address(false)
self.deployer
}
/// The arguments to the clarity function.
fn as_contract_args(&self) -> Vec<ClarityValue> {
Expand All @@ -122,7 +124,9 @@ impl AsContractCall for InitiateWithdrawalRequest {
/// do, which is store all stacks blocks and store the transactions that we
/// care about, which, naturally, are sBTC related transactions.
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[test_case(ContractCallWrapper(InitiateWithdrawalRequest); "initiate-withdrawal")]
#[test_case(ContractCallWrapper(InitiateWithdrawalRequest {
deployer: testing::wallet::WALLET.0.address(),
}); "initiate-withdrawal")]
#[test_case(ContractCallWrapper(CompleteDepositV1 {
outpoint: bitcoin::OutPoint::null(),
amount: 123654,
Expand Down Expand Up @@ -188,7 +192,8 @@ async fn writing_stacks_blocks_works<T: AsContractCall>(contract: ContractCallWr

// Okay now to save these blocks. We check that all of these blocks are
// saved and that the transaction that we care about is saved as well.
let txs = storage::postgres::extract_relevant_transactions(&blocks);
let settings = Settings::new_from_default_config().unwrap();
let txs = storage::postgres::extract_relevant_transactions(&blocks, &settings.signer.deployer);
let headers = blocks
.iter()
.map(model::StacksBlock::try_from)
Expand Down
Loading