Skip to content

feat: use longest common prefix for determining tx replay set #6353

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 6 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to the versioning scheme outlined in the [README.md](README.md).

## Unreleased

### Added

- When determining a global transaction replay set, the state evaluator now uses a longest-common-prefix algorithm to find a replay set in the case where a single replay set has less than 70% of signer weight.

## [3.2.0.0.1]
### Added

Expand Down
289 changes: 289 additions & 0 deletions libsigner/src/tests/signer_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,117 @@ use crate::v0::messages::{
};
use crate::v0::signer_state::{GlobalStateEvaluator, ReplayTransactionSet, SignerStateMachine};

/// Test setup helper struct containing common test data
struct SignerStateTest {
global_eval: GlobalStateEvaluator,
addresses: Vec<StacksAddress>,
burn_block: ConsensusHash,
burn_block_height: u64,
current_miner: StateMachineUpdateMinerState,
local_supported_signer_protocol_version: u64,
active_signer_protocol_version: u64,
tx_a: StacksTransaction,
tx_b: StacksTransaction,
tx_c: StacksTransaction,
tx_d: StacksTransaction,
}

impl SignerStateTest {
fn new(num_signers: u32) -> Self {
let global_eval = generate_global_state_evaluator(num_signers);
let addresses: Vec<_> = global_eval.address_weights.keys().cloned().collect();
let local_address = addresses[0].clone();

let burn_block = ConsensusHash([20u8; 20]);
let burn_block_height = 100;
let current_miner = StateMachineUpdateMinerState::ActiveMiner {
current_miner_pkh: Hash160([0xab; 20]),
tenure_id: ConsensusHash([0x44; 20]),
parent_tenure_id: ConsensusHash([0x22; 20]),
parent_tenure_last_block: StacksBlockId([0x33; 32]),
parent_tenure_last_block_height: 1,
};

let local_supported_signer_protocol_version = 1;
let active_signer_protocol_version = 1;

// Create test transactions with different memos for uniqueness
let pk1 = StacksPrivateKey::random();
let pk2 = StacksPrivateKey::random();
let pk3 = StacksPrivateKey::random();
let pk4 = StacksPrivateKey::random();

let make_tx = |pk: &StacksPrivateKey, memo: [u8; 34]| StacksTransaction {
version: TransactionVersion::Testnet,
chain_id: 0x80000000,
auth: TransactionAuth::from_p2pkh(pk).unwrap(),
anchor_mode: TransactionAnchorMode::Any,
post_condition_mode: TransactionPostConditionMode::Allow,
post_conditions: vec![],
payload: TransactionPayload::TokenTransfer(
local_address.clone().into(),
100,
TokenTransferMemo(memo),
),
};

let tx_a = make_tx(&pk1, [1u8; 34]);
let tx_b = make_tx(&pk2, [2u8; 34]);
let tx_c = make_tx(&pk3, [3u8; 34]);
let tx_d = make_tx(&pk4, [4u8; 34]);

Self {
global_eval,
addresses,
burn_block,
burn_block_height,
current_miner,
local_supported_signer_protocol_version,
active_signer_protocol_version,
tx_a,
tx_b,
tx_c,
tx_d,
}
}

/// Create a replay transaction update message
fn create_replay_update(
&self,
transactions: Vec<StacksTransaction>,
) -> StateMachineUpdateMessage {
StateMachineUpdateMessage::new(
self.active_signer_protocol_version,
self.local_supported_signer_protocol_version,
StateMachineUpdateContent::V1 {
burn_block: self.burn_block,
burn_block_height: self.burn_block_height,
current_miner: self.current_miner.clone(),
replay_transactions: transactions,
},
)
.unwrap()
}

/// Update multiple signers with the same replay transaction set
fn update_signers(&mut self, signer_indices: &[usize], transactions: Vec<StacksTransaction>) {
let update = self.create_replay_update(transactions);
for &index in signer_indices {
self.global_eval
.insert_update(self.addresses[index].clone(), update.clone());
}
}

/// Get the global state replay set
fn get_global_replay_set(&mut self) -> Vec<StacksTransaction> {
self.global_eval
.determine_global_state()
.unwrap()
.tx_replay_set
.unwrap_or_default()
}
}

fn generate_global_state_evaluator(num_addresses: u32) -> GlobalStateEvaluator {
let address_weights = generate_random_address_with_equal_weights(num_addresses);
let active_protocol_version = 0;
Expand Down Expand Up @@ -417,3 +528,181 @@ fn determine_global_states_with_tx_replay_set() {
tx_replay_state_machine
);
}

#[test]
/// Case: One signer has [A,B,C], another has [A,B] - should find common prefix [A,B]
fn test_replay_set_common_prefix_coalescing() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test seems a duplicate respect to test_replay_set_common_prefix_coalescing_demo(). Could we consider to remove one of them?

about test naming: test_replay_set_common_prefix_coalescing would be my preference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, my bad, I didn't mean to have them both. I've removed the _demo version.

let mut state_test = SignerStateTest::new(5);

// Signers 0, 1: [A,B,C] (40% weight)
state_test.update_signers(
&[0, 1],
vec![
state_test.tx_a.clone(),
state_test.tx_b.clone(),
state_test.tx_c.clone(),
],
);

// Signers 2, 3, 4: [A,B] (60% weight - should win)
state_test.update_signers(
&[2, 3, 4],
vec![state_test.tx_a.clone(), state_test.tx_b.clone()],
);

let transactions = state_test.get_global_replay_set();

// Should find common prefix [A,B] since it's the longest prefix with majority support
assert_eq!(transactions.len(), 2);
assert_eq!(transactions[0], state_test.tx_a); // Order matters!
assert_eq!(transactions[1], state_test.tx_b);
assert!(!transactions.contains(&state_test.tx_c));
Comment on lines +556 to +559
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: last assertion assert!(!transactions.contains(&state_test.tx_c)) appears to be redundant. Is the intention to enforce the concept?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it is redundant, but I guess it doesn't hurt to keep it 🤷🏼

}

#[test]
/// Case: One sequence has clear majority - should use that sequence
fn test_replay_set_majority_prefix_selection() {
let mut state_test = SignerStateTest::new(5);

// Signer 0: [A] (20% weight)
state_test.update_signers(&[0], vec![state_test.tx_a.clone()]);

// Signers 1, 2, 3, 4: [C] (80% weight - above threshold)
state_test.update_signers(&[1, 2, 3, 4], vec![state_test.tx_c.clone()]);

let transactions = state_test.get_global_replay_set();

// Should use [C] since it has majority support (80% > 70%)
assert_eq!(transactions.len(), 1);
assert_eq!(transactions[0], state_test.tx_c);
}

#[test]
/// Case: Exact agreement should be prioritized over subset coalescing
fn test_replay_set_exact_agreement_prioritized() {
let mut state_test = SignerStateTest::new(5);

// 4 signers agree on [A,B] exactly (80% - above threshold)
state_test.update_signers(
&[0, 1, 2, 3],
vec![state_test.tx_a.clone(), state_test.tx_b.clone()],
);

// 1 signer has just [A] (20%)
state_test.update_signers(&[4], vec![state_test.tx_a.clone()]);

let transactions = state_test.get_global_replay_set();

// Should use exact agreement [A,B] rather than common prefix [A]
assert_eq!(transactions.len(), 2);
assert_eq!(transactions[0], state_test.tx_a); // Order matters!
assert_eq!(transactions[1], state_test.tx_b);
}

#[test]
/// Case: Complete disagreement - no overlap and no majority
fn test_replay_set_no_agreement_returns_empty() {
let mut state_test = SignerStateTest::new(5);

// Signer 0: [A] (20% weight)
state_test.update_signers(&[0], vec![state_test.tx_a.clone()]);

// Signer 1: [B] (20% weight)
state_test.update_signers(&[1], vec![state_test.tx_b.clone()]);

// Signer 2: [C] (20% weight)
state_test.update_signers(&[2], vec![state_test.tx_c.clone()]);

// Signers 3, 4: empty sets (40% weight)
state_test.update_signers(&[3, 4], vec![]);

let transactions = state_test.get_global_replay_set();

// Should return empty set to prioritize liveness when no agreement
assert_eq!(transactions.len(), 0);
}

#[test]
/// Case: Same transactions in different order have no common prefix
fn test_replay_set_order_matters_no_common_prefix() {
let mut state_test = SignerStateTest::new(4);

// Signers 0, 1: [A,B] (50% weight)
state_test.update_signers(
&[0, 1],
vec![state_test.tx_a.clone(), state_test.tx_b.clone()],
);

// Signers 2, 3: [B,A] (50% weight)
state_test.update_signers(
&[2, 3],
vec![state_test.tx_b.clone(), state_test.tx_a.clone()],
);

let transactions = state_test.get_global_replay_set();

// Should return empty set since [A,B] and [B,A] have no common prefix
// Even though both contain the same transactions, order matters for replay
assert_eq!(transactions.len(), 0);
}

#[test]
/// Case: [A,B,C] vs [A,B,D] should find common prefix [A,B]
fn test_replay_set_partial_prefix_match() {
let mut state_test = SignerStateTest::new(5);

// Signer 0, 1: [A,B,C] (40% weight)
state_test.update_signers(
&[0, 1],
vec![
state_test.tx_a.clone(),
state_test.tx_b.clone(),
state_test.tx_c.clone(),
],
);

// Signers 2, 3, 4: [A,B,D] (60% weight)
state_test.update_signers(
&[2, 3, 4],
vec![
state_test.tx_a.clone(),
state_test.tx_b.clone(),
state_test.tx_d.clone(),
],
);

let transactions = state_test.get_global_replay_set();

// Should find [A,B] as the longest common prefix with majority support
assert_eq!(transactions.len(), 2);
assert_eq!(transactions[0], state_test.tx_a);
assert_eq!(transactions[1], state_test.tx_b);
}

#[test]
/// Edge case: Equal-weight competing prefixes should find common prefix
fn test_replay_set_equal_weight_competing_prefixes() {
let mut state_test = SignerStateTest::new(6);

// Signers 0, 1, 2: [A,B] (50% weight - not enough alone)
state_test.update_signers(
&[0, 1, 2],
vec![state_test.tx_a.clone(), state_test.tx_b.clone()],
);

// Signers 3, 4, 5: [A,C] (50% weight - not enough alone)
state_test.update_signers(
&[3, 4, 5],
vec![state_test.tx_a.clone(), state_test.tx_c.clone()],
);

let transactions = state_test.get_global_replay_set();

// Should find common prefix [A] since both [A,B] and [A,C] start with [A]
// and [A] has 100% support (above the 70% threshold)
assert_eq!(transactions.len(), 1, "Should find common prefix [A]");
assert_eq!(
transactions[0], state_test.tx_a,
"Should contain transaction A"
);
}
Loading
Loading