Skip to content

Commit

Permalink
feat: use conservative default sBTC limits (#1254)
Browse files Browse the repository at this point in the history
* feat: use conservative default sBTC limits

* chore: restore trait import

* chore: use max money as minimum

* chore: remove default in favor of explicit methods
  • Loading branch information
matteojug authored Jan 22, 2025
1 parent f53b9bf commit 84541ed
Show file tree
Hide file tree
Showing 11 changed files with 494 additions and 38 deletions.
34 changes: 17 additions & 17 deletions signer/src/bitcoin/utxo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1672,7 +1672,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 2,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};
let keypair = Keypair::new_global(&mut OsRng);
Expand Down Expand Up @@ -1786,7 +1786,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -1876,7 +1876,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -1985,7 +1985,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2071,7 +2071,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2116,7 +2116,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2167,7 +2167,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2215,7 +2215,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 8,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2275,7 +2275,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 8,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2375,7 +2375,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 8,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2441,7 +2441,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 8,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2521,7 +2521,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 8,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};
let mut transactions = requests.construct_transactions().unwrap();
Expand Down Expand Up @@ -2558,7 +2558,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 0,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2616,7 +2616,7 @@ mod tests {
},
num_signers: 10,
accept_threshold: 8,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2919,7 +2919,7 @@ mod tests {
},
num_signers: 11,
accept_threshold: 6,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -2966,7 +2966,7 @@ mod tests {
},
accept_threshold: 127,
num_signers: 128,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -3022,7 +3022,7 @@ mod tests {
},
accept_threshold: 10,
num_signers: 14,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down
32 changes: 30 additions & 2 deletions signer/src/context/signer_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::keys::PublicKey;
/// A struct for holding internal signer state. This struct is served by
/// the [`SignerContext`] and can be used to cache global state instead of
/// fetching it via I/O for frequently accessed information.
#[derive(Debug, Default)]
#[derive(Debug)]
pub struct SignerState {
current_signer_set: SignerSet,
current_limits: RwLock<SbtcLimits>,
Expand Down Expand Up @@ -77,8 +77,20 @@ impl SignerState {
}
}

impl Default for SignerState {
fn default() -> Self {
Self {
current_signer_set: Default::default(),
current_limits: RwLock::new(SbtcLimits::zero()),
sbtc_contracts_deployed: Default::default(),
sbtc_bitcoin_start_height: Default::default(),
is_sbtc_bitcoin_start_height_set: Default::default(),
}
}
}

/// Represents the current sBTC limits.
#[derive(Debug, Clone, Default, PartialEq)]
#[derive(Debug, Clone, PartialEq)]
pub struct SbtcLimits {
/// Represents the total cap for all pegged-in BTC/sBTC.
total_cap: Option<Amount>,
Expand Down Expand Up @@ -120,6 +132,22 @@ impl SbtcLimits {
}
}

/// Create a new `SbtcLimits` object without any limits
pub fn unlimited() -> Self {
Self::new(None, None, None, None, None)
}

/// Create a new `SbtcLimits` object with limits set to zero (fully constraining)
pub fn zero() -> Self {
Self::new(
Some(Amount::ZERO),
Some(Amount::MAX_MONEY),
Some(Amount::ZERO),
Some(Amount::ZERO),
Some(Amount::ZERO),
)
}

/// Get the total cap for all pegged-in BTC/sBTC.
pub fn total_cap(&self) -> Amount {
self.total_cap.unwrap_or(Amount::MAX_MONEY)
Expand Down
2 changes: 1 addition & 1 deletion signer/src/testing/block_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ impl EmilyInteract for TestHarness {
}

async fn get_limits(&self) -> Result<SbtcLimits, Error> {
Ok(SbtcLimits::default())
Ok(SbtcLimits::unlimited())
}
}

Expand Down
9 changes: 7 additions & 2 deletions signer/tests/integration/bitcoin_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ async fn one_tx_per_request_set() {
.with_mocked_stacks_client()
.with_mocked_emily_client()
.build();
ctx.state().update_current_limits(SbtcLimits::unlimited());

let signers = TestSignerSet::new(&mut rng);
let amounts = [DepositAmounts {
Expand Down Expand Up @@ -195,6 +196,7 @@ async fn one_invalid_deposit_invalidates_tx() {
.with_mocked_stacks_client()
.with_mocked_emily_client()
.build();
ctx.state().update_current_limits(SbtcLimits::unlimited());

let signers = TestSignerSet::new(&mut rng);
let amounts = [
Expand Down Expand Up @@ -382,6 +384,7 @@ async fn cannot_sign_deposit_is_ok() {
.with_mocked_stacks_client()
.with_mocked_emily_client()
.build();
ctx.state().update_current_limits(SbtcLimits::unlimited());

let amounts = [
DepositAmounts {
Expand Down Expand Up @@ -515,7 +518,7 @@ async fn cannot_sign_deposit_is_ok() {
signer_state: signer_btc_state(&ctx, &request, &btc_ctx).await,
accept_threshold: 2,
num_signers: 3,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: ctx.config().signer.max_deposits_per_bitcoin_tx.get(),
};
let txs = sbtc_requests.construct_transactions().unwrap();
Expand Down Expand Up @@ -545,6 +548,7 @@ async fn sighashes_match_from_sbtc_requests_object() {
.with_mocked_stacks_client()
.with_mocked_emily_client()
.build();
ctx.state().update_current_limits(SbtcLimits::unlimited());

let signers = TestSignerSet::new(&mut rng);
let amounts = [
Expand Down Expand Up @@ -647,7 +651,7 @@ async fn sighashes_match_from_sbtc_requests_object() {
signer_state: signer_btc_state(&ctx, &request, &btc_ctx).await,
accept_threshold: 2,
num_signers: 3,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: ctx.config().signer.max_deposits_per_bitcoin_tx.get(),
};
let txs = sbtc_requests.construct_transactions().unwrap();
Expand Down Expand Up @@ -677,6 +681,7 @@ async fn outcome_is_independent_of_input_order() {
.with_mocked_stacks_client()
.with_mocked_emily_client()
.build();
ctx.state().update_current_limits(SbtcLimits::unlimited());

let signers = TestSignerSet::new(&mut rng);
let amounts = [
Expand Down
13 changes: 5 additions & 8 deletions signer/tests/integration/block_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use signer::context::SbtcLimits;
use signer::emily_client::EmilyClient;
use signer::error::Error;
use signer::keys::SignerScriptPubKey as _;
use signer::logging::setup_logging;
use signer::stacks::api::TenureBlocks;
use signer::storage::model;
use signer::storage::model::BitcoinBlockHash;
Expand Down Expand Up @@ -83,6 +82,7 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6
.with_mocked_emily_client()
.with_mocked_stacks_client()
.build();
ctx.state().update_current_limits(SbtcLimits::unlimited());

// We're going to create two confirmed deposits. This also generates
// sweep transactions, but this information is not in our database, so
Expand All @@ -104,7 +104,7 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6
client
.expect_get_limits()
.times(1..)
.returning(|| Box::pin(async { Ok(SbtcLimits::default()) }));
.returning(|| Box::pin(async { Ok(SbtcLimits::unlimited()) }));
})
.await;

Expand Down Expand Up @@ -246,8 +246,6 @@ async fn load_latest_deposit_requests_persists_requests_from_past(blocks_ago: u6
#[ignore = "This is an integration test that requires devenv running"]
#[tokio::test]
async fn link_blocks() {
setup_logging("info", true);

let db = testing::storage::new_test_database().await;

let stacks_client = StacksClient::new(Url::parse("http://localhost:20443").unwrap()).unwrap();
Expand Down Expand Up @@ -379,7 +377,6 @@ async fn fetch_input(db: &PgStore, output_type: TxPrevoutType) -> Vec<TxPrevout>
async fn block_observer_stores_donation_and_sbtc_utxos() {
let mut rng = rand::rngs::StdRng::seed_from_u64(51);
let (rpc, faucet) = regtest::initialize_blockchain();
// signer::logging::setup_logging("info,signer=debug", false);

// We need to populate our databases, so let's fetch the data.
let emily_client =
Expand Down Expand Up @@ -578,7 +575,7 @@ async fn block_observer_stores_donation_and_sbtc_utxos() {
},
accept_threshold: 4,
num_signers: 7,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: ctx.config().signer.max_deposits_per_bitcoin_tx.get(),
};

Expand Down Expand Up @@ -659,9 +656,9 @@ async fn block_observer_stores_donation_and_sbtc_utxos() {
}

#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[test_case::test_case(false, SbtcLimits::default(); "no contracts, default limits")]
#[test_case::test_case(false, SbtcLimits::unlimited(); "no contracts, default limits")]
#[test_case::test_case(false, SbtcLimits::new(Some(bitcoin::Amount::from_sat(1_000)), None, None, None, None); "no contracts, total cap limit")]
#[test_case::test_case(true, SbtcLimits::default(); "deployed contracts, default limits")]
#[test_case::test_case(true, SbtcLimits::unlimited(); "deployed contracts, default limits")]
#[test_case::test_case(true, SbtcLimits::new(Some(bitcoin::Amount::from_sat(1_000)), None, None, None, None); "deployed contracts, total cap limit")]
#[tokio::test]
async fn block_observer_handles_update_limits(deployed: bool, sbtc_limits: SbtcLimits) {
Expand Down
1 change: 0 additions & 1 deletion signer/tests/integration/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ async fn complete_deposit_wrapper_tx_accepted<T: AsContractCall>(contract: Contr
#[ignore = "This is an integration test that requires a stacks-node to work"]
#[tokio::test]
async fn estimate_tx_fees() {
signer::logging::setup_logging("info", false);
let client = stacks_client();

let payload = SmartContract::SbtcRegistry;
Expand Down
2 changes: 1 addition & 1 deletion signer/tests/integration/rbf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub fn transaction_with_rbf(
},
accept_threshold: failure_threshold,
num_signers: 2 * failure_threshold,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down
4 changes: 2 additions & 2 deletions signer/tests/integration/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl TestSweepSetup {
},
accept_threshold: 4,
num_signers: 7,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down Expand Up @@ -766,7 +766,7 @@ impl TestSweepSetup2 {
},
accept_threshold: 4,
num_signers: 7,
sbtc_limits: SbtcLimits::default(),
sbtc_limits: SbtcLimits::unlimited(),
max_deposits_per_bitcoin_tx: DEFAULT_MAX_DEPOSITS_PER_BITCOIN_TX,
};

Expand Down
Loading

0 comments on commit 84541ed

Please sign in to comment.