Skip to content

Add SIGNER_APPROVAL_THRESHOLD env var for lowering threshold on testnet #6375

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion libsigner/src/tests/signer_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn generate_global_state_evaluator(num_addresses: u32) -> GlobalStateEvaluator {
for address in address_weights.keys() {
address_updates.insert(address.clone(), update.clone());
}
GlobalStateEvaluator::new(address_updates, address_weights)
GlobalStateEvaluator::new(address_updates, address_weights, false)
}

fn generate_random_address_with_equal_weights(num_addresses: u32) -> HashMap<StacksAddress, u32> {
Expand Down
28 changes: 22 additions & 6 deletions libsigner/src/v0/signer_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::hash::{Hash, Hasher};

use blockstack_lib::chainstate::stacks::StacksTransaction;
use clarity::types::chainstate::StacksAddress;
use clarity::util::compute_voting_weight_threshold;
use serde::{Deserialize, Serialize};
use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId};
use stacks_common::util::hash::Hash160;
Expand All @@ -33,21 +34,30 @@ pub struct GlobalStateEvaluator {
pub address_updates: HashMap<StacksAddress, StateMachineUpdate>,
/// The total weight of all signers
pub total_weight: u32,
/// The threshold weight required to reach rejection
pub rejection_weight: u32,
/// The threshold weight required to reach approval
pub approval_weight: u32,
}

impl GlobalStateEvaluator {
/// Create a new state evaluator
pub fn new(
address_updates: HashMap<StacksAddress, StateMachineUpdate>,
address_weights: HashMap<StacksAddress, u32>,
mainnet: bool,
) -> Self {
let total_weight = address_weights
.values()
.fold(0u32, |acc, val| acc.saturating_add(*val));
let approval_weight = compute_voting_weight_threshold(total_weight, mainnet);
let rejection_weight = total_weight.saturating_sub(approval_weight);
Self {
address_weights,
address_updates,
total_weight,
approval_weight,
rejection_weight,
}
}

Expand All @@ -69,7 +79,7 @@ impl GlobalStateEvaluator {
let mut total_weight_support = 0;
for (version, weight_support) in protocol_versions.into_iter().rev() {
total_weight_support += weight_support;
if total_weight_support >= self.total_weight * 7 / 10 {
if self.reached_approval(total_weight_support) {
return Some(version);
}
}
Expand All @@ -89,7 +99,7 @@ impl GlobalStateEvaluator {
.entry((burn_block, burn_block_height))
.or_insert_with(|| 0);
*entry += weight;
if self.reached_agreement(*entry) {
if self.reached_approval(*entry) {
return Some((*burn_block, burn_block_height));
}
}
Expand Down Expand Up @@ -124,7 +134,7 @@ impl GlobalStateEvaluator {
let entry = state_views.entry(key).or_insert_with(|| 0);
*entry += weight;

if self.reached_agreement(*entry) {
if self.reached_approval(*entry) {
found_state_view = Some(state_machine);
}

Expand All @@ -133,7 +143,7 @@ impl GlobalStateEvaluator {
.or_insert_with(|| 0);
*replay_entry += weight;

if self.reached_agreement(*replay_entry) {
if self.reached_approval(*replay_entry) {
found_replay_set = Some(tx_replay_set);
}
if found_replay_set.is_some() && found_state_view.is_some() {
Expand All @@ -159,8 +169,14 @@ impl GlobalStateEvaluator {

/// Check if the supplied vote weight crosses the global agreement threshold.
/// Returns true if it has, false otherwise.
pub fn reached_agreement(&self, vote_weight: u32) -> bool {
vote_weight >= self.total_weight * 7 / 10
pub fn reached_approval(&self, vote_weight: u32) -> bool {
vote_weight >= self.approval_weight
}

/// Check if the supplied vote weight crosses the global disagreement threshold.
/// Returns true if it has, false otherwise.
pub fn reached_rejection(&self, vote_weight: u32) -> bool {
vote_weight >= self.rejection_weight
}

/// Get the global transaction replay set. Returns `None` if there
Expand Down
1 change: 1 addition & 0 deletions stacks-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ sha2 = { version = "0.10" }

[dev-dependencies]
proptest = "1.6.0"
serial_test = "3.2.0"

[target.'cfg(windows)'.dev-dependencies]
winapi = { version = "0.3", features = ["fileapi", "processenv", "winnt"] }
Expand Down
133 changes: 133 additions & 0 deletions stacks-common/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,136 @@ where
}
#[cfg(all(feature = "rusqlite", target_family = "wasm"))]
compile_error!("The `rusqlite` feature is not supported for wasm targets");

// The threshold of weighted votes to reach consensus in Nakamoto.
// This is out of 100, so 70 means "70%".
pub const NAKAMOTO_SIGNER_APPROVAL_THRESHOLD: u32 = 70;

/// Determines the signer approval threshold percentage for Nakamoto block approval.
///
/// By default, this uses the constant [`NAKAMOTO_SIGNER_APPROVAL_THRESHOLD`].
/// If the `SIGNER_APPROVAL_THRESHOLD` environment variable is set, its value (in percent)
/// will be used instead—unless `mainnet` is `true`, in which case overriding via the
/// environment variable is not allowed.
///
/// The environment variable value must be an integer between 1 and 100 (inclusive).
///
/// # Panics
/// - If `mainnet` is `true` and `SIGNER_APPROVAL_THRESHOLD` is set.
/// - If `SIGNER_APPROVAL_THRESHOLD` cannot be parsed as a `u32`.
/// - If the parsed threshold is not in the range `1..=100`.
///
/// # Parameters
/// - `mainnet`: Whether the network is Mainnet.
///
/// # Returns
/// The approval threshold percentage as a `u32`.
pub fn determine_signer_approval_threshold_percentage(mainnet: bool) -> u32 {
let mut threshold = NAKAMOTO_SIGNER_APPROVAL_THRESHOLD;
if let Ok(env_threshold) = std::env::var("SIGNER_APPROVAL_THRESHOLD") {
assert!(
!mainnet,
"Cannot use SIGNER_APPROVAL_THRESHOLD env variable with Mainnet."
);
match env_threshold.parse::<u32>() {
Ok(env_threshold) => {
assert!(
env_threshold > 0 && env_threshold <= 100,
"Invalid SIGNER_APPROVAL_THRESHOLD. Must be > 0 and <= 100"
);
threshold = env_threshold;
}
Err(e) => panic!("Failed to parse SIGNER_APPROVAL_THRESHOLD as a u32: {e}"),
}
}
threshold
}

/// Computes the minimum voting weight required to reach consensus.
///
/// The threshold is determined as a percentage of `total_weight`, using
/// [`determine_signer_approval_threshold_percentage`]. The percentage is
/// applied, and any remainder from integer division is rounded up by adding 1
/// if there is a non-zero remainder.
///
/// # Parameters
/// - `total_weight`: The total combined voting weight of all signers.
/// - `mainnet`: Whether the network is Mainnet (affects threshold calculation).
///
/// # Returns
/// The minimum voting weight (rounded up) required for consensus.
pub fn compute_voting_weight_threshold(total_weight: u32, mainnet: bool) -> u32 {
let threshold = determine_signer_approval_threshold_percentage(mainnet);
let ceil = if (total_weight * threshold) % 100 == 0 {
0
} else {
1
};
(total_weight * threshold / 100).saturating_add(ceil)
}

#[test]
pub fn test_compute_voting_weight_threshold_no_env() {
// We are purposefully testing ONLY compute_voting_weight_threshold without SIGNER_APPROVAL_THRESHOLD
// see following tests for env SIGNER_APPROVAL_THRESHOLD specific tests.
use crate::util::compute_voting_weight_threshold;
std::env::remove_var("SIGNER_APPROVAL_THRESHOLD");
assert_eq!(compute_voting_weight_threshold(100_u32, false), 70_u32,);

assert_eq!(compute_voting_weight_threshold(10_u32, false), 7_u32,);

assert_eq!(compute_voting_weight_threshold(3000_u32, false), 2100_u32,);

assert_eq!(compute_voting_weight_threshold(4000_u32, false), 2800_u32,);

// Round-up check
assert_eq!(compute_voting_weight_threshold(511_u32, false), 358_u32,);
}

#[test]
#[serial_test::serial]
fn returns_default_when_env_not_set() {
std::env::remove_var("SIGNER_APPROVAL_THRESHOLD");
let result = determine_signer_approval_threshold_percentage(false);
assert_eq!(result, 70);
}

#[test]
#[serial_test::serial]
fn uses_env_when_not_mainnet() {
std::env::set_var("SIGNER_APPROVAL_THRESHOLD", "75");
let result = determine_signer_approval_threshold_percentage(false);
assert_eq!(result, 75);
}

#[test]
#[serial_test::serial]
#[should_panic(expected = "Cannot use SIGNER_APPROVAL_THRESHOLD env variable with Mainnet")]
fn panics_if_env_set_on_mainnet() {
std::env::set_var("SIGNER_APPROVAL_THRESHOLD", "50");
let _ = determine_signer_approval_threshold_percentage(true);
}

#[test]
#[serial_test::serial]
#[should_panic(expected = "Failed to parse SIGNER_APPROVAL_THRESHOLD as a u32")]
fn panics_if_env_not_u32() {
std::env::set_var("SIGNER_APPROVAL_THRESHOLD", "not-a-number");
let _ = determine_signer_approval_threshold_percentage(false);
}

#[test]
#[serial_test::serial]
#[should_panic(expected = "Invalid SIGNER_APPROVAL_THRESHOLD. Must be > 0 and <= 100")]
fn panics_if_env_is_zero() {
std::env::set_var("SIGNER_APPROVAL_THRESHOLD", "0");
let _ = determine_signer_approval_threshold_percentage(false);
}

#[test]
#[serial_test::serial]
#[should_panic(expected = "Invalid SIGNER_APPROVAL_THRESHOLD. Must be > 0 and <= 100")]
fn panics_if_env_over_100() {
std::env::set_var("SIGNER_APPROVAL_THRESHOLD", "101");
let _ = determine_signer_approval_threshold_percentage(false);
}
14 changes: 4 additions & 10 deletions stacks-node/src/nakamoto_node/stackerdb_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,6 @@ impl StackerDBListener {
warn!("Replaced the miner/coordinator receiver of a prior thread. Prior thread may have crashed.");
}

let total_weight = reward_set.total_signing_weight().map_err(|e| {
warn!("Failed to calculate total weight for the reward set: {e:?}");
ChainstateError::NoRegisteredSigners(0)
})?;

let weight_threshold = NakamotoBlockHeader::compute_voting_weight_threshold(total_weight)?;

let reward_cycle_id = burnchain
.block_height_to_reward_cycle(burn_tip.block_height)
.expect("FATAL: tried to initialize coordinator before first burn block height");
Expand Down Expand Up @@ -191,7 +184,8 @@ impl StackerDBListener {
.get_latest_chunks(&slot_ids)
.inspect_err(|e| warn!("Unable to read the latest signer state from signer db: {e}."))
.unwrap_or_default();
let mut global_state_evaluator = GlobalStateEvaluator::new(HashMap::new(), address_weights);
let mut global_state_evaluator =
GlobalStateEvaluator::new(HashMap::new(), address_weights, config.is_mainnet());
for (chunk, slot_id) in chunks.into_iter().zip(slot_ids) {
let Some(chunk) = chunk else {
continue;
Expand All @@ -216,8 +210,8 @@ impl StackerDBListener {
node_keep_running,
keep_running,
signer_set,
total_weight,
weight_threshold,
total_weight: global_state_evaluator.total_weight,
weight_threshold: global_state_evaluator.approval_weight,
signer_entries,
blocks: Arc::new((Mutex::new(HashMap::new()), Condvar::new())),
signer_idle_timestamps: Arc::new(Mutex::new(HashMap::new())),
Expand Down
84 changes: 84 additions & 0 deletions stacks-node/src/tests/signer/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18462,3 +18462,87 @@ fn signer_loads_stackerdb_updates_on_startup() {
info!("------------------------- Shutdown -------------------------");
miners.shutdown();
}

#[test]
#[ignore]
#[serial_test::serial]
/// This test verifies that a a signer will send update messages to stackerdb when it updates its internal state
///
/// For a new bitcoin block arrival, the signers send a local state update message with this updated block and miner
/// For an inactive miner, the signer sends a local state update message indicating it is reverting to the prior miner
fn custom_signer_approval_threshold_testnet() {
if env::var("BITCOIND_TEST") != Ok("1".into()) {
return;
}

std::env::set_var("SIGNER_APPROVAL_THRESHOLD", "60");

tracing_subscriber::registry()
.with(fmt::layer())
.with(EnvFilter::from_default_env())
.init();

info!("------------------------- Test Setup -------------------------");
let num_signers = 3; // Let's only set 3 signers to that we can ensure if only 2 approve, we still hit global acceptance
let signer_test: SignerTest<SpawnedSigner> = SignerTest::new(num_signers, vec![]);
signer_test.boot_to_epoch_3();

let rejecting_signer =
StacksPublicKey::from_private(&signer_test.signer_stacks_private_keys[0]);
info!("------------------------- Make Signer {rejecting_signer:?} Reject all Proposals (33% reject)-------------------------");
let rejecting_signers = vec![rejecting_signer.clone()];
TEST_REJECT_ALL_BLOCK_PROPOSAL.set(rejecting_signers.clone());

test_observer::clear();

info!("------------------------- Ensure Chain Does Not Halt Even with 1 Rejection -------------------------");
signer_test.mine_nakamoto_block(Duration::from_secs(30), true);

let mined_block = test_observer::get_mined_nakamoto_blocks().pop().unwrap();
wait_for_block_rejections_from_signers(
30,
&mined_block.signer_signature_hash,
&rejecting_signers,
)
.expect("Expected signer to reject block proposal");

let second_rejecting_signer =
StacksPublicKey::from_private(&signer_test.signer_stacks_private_keys[1]);
info!("------------------------- Make Signer {second_rejecting_signer:?} Reject all Proposals (66% reject) -------------------------");
let rejecting_signers = vec![rejecting_signer.clone(), second_rejecting_signer.clone()];
TEST_REJECT_ALL_BLOCK_PROPOSAL.set(rejecting_signers.clone());

let peer_info = signer_test.get_peer_info();
let stacks_miner_pk = StacksPublicKey::from_private(
&signer_test
.running_nodes
.conf
.miner
.mining_key
.clone()
.unwrap(),
);
signer_test.mine_bitcoin_block();
let rejected_block =
wait_for_block_proposal(30, peer_info.stacks_tip_height + 1, &stacks_miner_pk)
.expect("Failed to get block proposal");

wait_for_block_rejections_from_signers(
30,
&rejected_block.header.signer_signature_hash(),
&rejecting_signers,
)
.expect("Expected signer to reject block proposal");

info!("------------------------- Verify Chain Halts -------------------------");
assert!(
wait_for(15, || {
Ok(signer_test.get_peer_info().stacks_tip_height > peer_info.stacks_tip_height)
})
.is_err(),
"Expected the chain to halt"
);

info!("------------------------- Shutdown -------------------------");
signer_test.shutdown();
}
2 changes: 1 addition & 1 deletion stacks-signer/src/chainstate/tests/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ fn check_sortition_timeout() {
let address = StacksAddress::p2pkh(false, &StacksPublicKey::new());
let mut address_weights = HashMap::new();
address_weights.insert(address.clone(), 10);
let eval = GlobalStateEvaluator::new(HashMap::new(), address_weights);
let eval = GlobalStateEvaluator::new(HashMap::new(), address_weights, false);
// We have not yet timed out
assert!(!SortitionState::is_timed_out(
&consensus_hash,
Expand Down
Loading
Loading