Skip to content

Commit c6c8199

Browse files
hstovehstove
committed
fix: use ReplayTransactionSet wrapper
1 parent a0d3189 commit c6c8199

File tree

5 files changed

+119
-21
lines changed

5 files changed

+119
-21
lines changed

‎libsigner/src/tests/signer_state.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use crate::v0::messages::{
2424
StateMachineUpdate as StateMachineUpdateMessage, StateMachineUpdateContent,
2525
StateMachineUpdateMinerState,
2626
};
27-
use crate::v0::signer_state::{GlobalStateEvaluator, SignerStateMachine};
27+
use crate::v0::signer_state::{GlobalStateEvaluator, ReplayTransactionSet, SignerStateMachine};
2828

2929
fn generate_global_state_evaluator(num_addresses: u32) -> GlobalStateEvaluator {
3030
let address_weights = generate_random_address_with_equal_weights(num_addresses);
@@ -237,7 +237,7 @@ fn determine_global_states() {
237237
burn_block_height,
238238
current_miner: (&current_miner).into(),
239239
active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
240-
tx_replay_set: None,
240+
tx_replay_set: ReplayTransactionSet::none(),
241241
};
242242

243243
global_eval.insert_update(local_address, local_update);
@@ -276,7 +276,7 @@ fn determine_global_states() {
276276
burn_block_height,
277277
current_miner: (&new_miner).into(),
278278
active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
279-
tx_replay_set: None,
279+
tx_replay_set: ReplayTransactionSet::none(),
280280
};
281281

282282
global_eval.insert_update(local_address, new_update);

‎libsigner/src/v0/signer_state.rs

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,12 @@ impl GlobalStateEvaluator {
132132
burn_block_height,
133133
current_miner,
134134
..
135-
} => (burn_block, burn_block_height, current_miner, None),
135+
} => (
136+
burn_block,
137+
burn_block_height,
138+
current_miner,
139+
ReplayTransactionSet::new(vec![]),
140+
),
136141
StateMachineUpdateContent::V1 {
137142
burn_block,
138143
burn_block_height,
@@ -142,7 +147,7 @@ impl GlobalStateEvaluator {
142147
burn_block,
143148
burn_block_height,
144149
current_miner,
145-
Some(replay_transactions.clone()),
150+
ReplayTransactionSet::new(replay_transactions.clone()),
146151
),
147152
};
148153
let state_machine = SignerStateMachine {
@@ -179,6 +184,78 @@ impl GlobalStateEvaluator {
179184
}
180185
}
181186

187+
/// A "wrapper" struct around Vec<StacksTransaction> that behaves like
188+
/// `None` when the vector is empty.
189+
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Eq, Hash)]
190+
pub struct ReplayTransactionSet(Vec<StacksTransaction>);
191+
192+
impl ReplayTransactionSet {
193+
/// Create a new `ReplayTransactionSet`
194+
pub fn new(tx_replay_set: Vec<StacksTransaction>) -> Self {
195+
Self(tx_replay_set)
196+
}
197+
198+
/// Check if the `ReplayTransactionSet` is empty
199+
pub fn is_empty(&self) -> bool {
200+
self.0.is_empty()
201+
}
202+
203+
/// Unwrap the `ReplayTransactionSet`. Panics if the set is empty.
204+
pub fn unwrap(self) -> Vec<StacksTransaction> {
205+
if self.is_empty() {
206+
panic!("Called `unwrap` on an empty `ReplayTransactionSet`");
207+
}
208+
self.0
209+
}
210+
211+
/// Map into an optional, returning `None` if the set is empty
212+
pub fn into_optional(&self) -> Option<Vec<StacksTransaction>> {
213+
if self.is_empty() {
214+
None
215+
} else {
216+
Some(self.0.clone())
217+
}
218+
}
219+
220+
/// Unwrap the `ReplayTransactionSet` or return a default vector if it is empty
221+
pub fn unwrap_or_default(self) -> Vec<StacksTransaction> {
222+
if self.is_empty() {
223+
vec![]
224+
} else {
225+
self.unwrap()
226+
}
227+
}
228+
229+
/// Map the transactions in the set to a new type, only
230+
/// if the set is not empty
231+
pub fn map<U, F>(self, f: F) -> Option<U>
232+
where
233+
F: Fn(Vec<StacksTransaction>) -> U,
234+
{
235+
if self.is_empty() {
236+
None
237+
} else {
238+
Some(f(self.0))
239+
}
240+
}
241+
242+
/// Create a new `ReplayTransactionSet` with no transactions
243+
pub fn none() -> Self {
244+
Self(vec![])
245+
}
246+
247+
/// Check if the `ReplayTransactionSet` isn't empty
248+
pub fn is_some(&self) -> bool {
249+
!self.is_empty()
250+
}
251+
}
252+
253+
impl Default for ReplayTransactionSet {
254+
fn default() -> Self {
255+
Self::none()
256+
}
257+
}
258+
182259
/// A signer state machine view. This struct can
183260
/// be used to encode the local signer's view or
184261
/// the global view.
@@ -193,7 +270,7 @@ pub struct SignerStateMachine {
193270
/// The active signing protocol version
194271
pub active_signer_protocol_version: u64,
195272
/// Transaction replay set
196-
pub tx_replay_set: Option<Vec<StacksTransaction>>,
273+
pub tx_replay_set: ReplayTransactionSet,
197274
}
198275

199276
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Eq, Hash)]

‎stacks-signer/src/tests/signer_state.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use libsigner::v0::messages::{
2323
StateMachineUpdate as StateMachineUpdateMessage, StateMachineUpdateContent,
2424
StateMachineUpdateMinerState,
2525
};
26-
use libsigner::v0::signer_state::{GlobalStateEvaluator, SignerStateMachine};
26+
use libsigner::v0::signer_state::{GlobalStateEvaluator, ReplayTransactionSet, SignerStateMachine};
2727

2828
use crate::signerdb::tests::{create_block_override, tmp_db_path};
2929
use crate::signerdb::SignerDb;
@@ -130,7 +130,7 @@ fn check_capitulate_miner_view() {
130130
burn_block,
131131
burn_block_height,
132132
current_miner: (&new_miner).into(),
133-
tx_replay_set: None,
133+
tx_replay_set: ReplayTransactionSet::none(),
134134
active_signer_protocol_version,
135135
};
136136

‎stacks-signer/src/v0/signer_state.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ use libsigner::v0::messages::{
2525
MessageSlotID, SignerMessage, StateMachineUpdate as StateMachineUpdateMessage,
2626
StateMachineUpdateContent, StateMachineUpdateMinerState,
2727
};
28-
use libsigner::v0::signer_state::{GlobalStateEvaluator, MinerState, SignerStateMachine};
28+
use libsigner::v0::signer_state::{
29+
GlobalStateEvaluator, MinerState, ReplayTransactionSet, SignerStateMachine,
30+
};
2931
use serde::{Deserialize, Serialize};
3032
use stacks_common::bitvec::BitVec;
3133
use stacks_common::codec::Error as CodecError;
@@ -148,7 +150,7 @@ impl LocalStateMachine {
148150
burn_block_height: 0,
149151
current_miner: MinerState::NoValidMiner,
150152
active_signer_protocol_version: SUPPORTED_SIGNER_PROTOCOL_VERSION,
151-
tx_replay_set: None,
153+
tx_replay_set: ReplayTransactionSet::none(),
152154
}
153155
}
154156

@@ -369,17 +371,17 @@ impl LocalStateMachine {
369371
}
370372
};
371373

372-
if let Some(replay_set_hash) =
373-
NakamotoBlockProposal::tx_replay_hash(&prior_state_machine.tx_replay_set)
374-
{
374+
if let Some(replay_set_hash) = NakamotoBlockProposal::tx_replay_hash(
375+
&prior_state_machine.tx_replay_set.into_optional(),
376+
) {
375377
match db.get_was_block_validated_by_replay_tx(signer_signature_hash, replay_set_hash) {
376378
Ok(true) => {
377379
// This block was validated by our current state machine's replay set,
378380
// and the block exhausted the replay set. Therefore, clear the tx replay set.
379381
info!("Signer State: Incoming Stacks block exhausted the replay set, clearing the tx replay set";
380382
"signer_signature_hash" => %signer_signature_hash,
381383
);
382-
prior_state_machine.tx_replay_set = None;
384+
prior_state_machine.tx_replay_set = ReplayTransactionSet::none();
383385
}
384386
Ok(false) => {}
385387
Err(e) => {
@@ -535,7 +537,7 @@ impl LocalStateMachine {
535537
&prior_state_machine,
536538
tx_replay_set.is_some(),
537539
)? {
538-
tx_replay_set = Some(new_replay_set);
540+
tx_replay_set = ReplayTransactionSet::new(new_replay_set);
539541
}
540542
}
541543

@@ -621,7 +623,12 @@ impl LocalStateMachine {
621623
burn_block_height,
622624
current_miner,
623625
..
624-
} => (burn_block, burn_block_height, current_miner, None),
626+
} => (
627+
burn_block,
628+
burn_block_height,
629+
current_miner,
630+
ReplayTransactionSet::none(),
631+
),
625632
StateMachineUpdateContent::V1 {
626633
burn_block,
627634
burn_block_height,
@@ -631,7 +638,7 @@ impl LocalStateMachine {
631638
burn_block,
632639
burn_block_height,
633640
current_miner,
634-
Some(replay_transactions),
641+
ReplayTransactionSet::new(replay_transactions.clone()),
635642
),
636643
};
637644

@@ -645,7 +652,7 @@ impl LocalStateMachine {
645652
burn_block_height: *burn_block_height,
646653
current_miner: current_miner.into(),
647654
active_signer_protocol_version,
648-
tx_replay_set: tx_replay_set.cloned(),
655+
tx_replay_set: tx_replay_set.clone(),
649656
});
650657
// Because we updated our active signer protocol version, update local_update so its included in the subsequent evaluations
651658
let Ok(update) =
@@ -670,7 +677,12 @@ impl LocalStateMachine {
670677
burn_block_height,
671678
current_miner,
672679
..
673-
} => (burn_block, burn_block_height, current_miner, None),
680+
} => (
681+
burn_block,
682+
burn_block_height,
683+
current_miner,
684+
ReplayTransactionSet::none(),
685+
),
674686
StateMachineUpdateContent::V1 {
675687
burn_block,
676688
burn_block_height,
@@ -680,7 +692,7 @@ impl LocalStateMachine {
680692
burn_block,
681693
burn_block_height,
682694
current_miner,
683-
Some(replay_transactions),
695+
ReplayTransactionSet::new(replay_transactions.clone()),
684696
),
685697
};
686698

@@ -817,7 +829,7 @@ impl LocalStateMachine {
817829
let Self::Initialized(state) = self else {
818830
return None;
819831
};
820-
state.tx_replay_set.clone()
832+
state.tx_replay_set.into_optional()
821833
}
822834

823835
/// Handle a possible bitcoin fork. If a fork is detetected,

‎stackslib/src/net/api/postblock_proposal.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,10 @@ impl NakamotoBlockProposal {
595595
// contains transactions from the replay set. Thus, if we're here,
596596
// the block contains a transaction that is not in the replay set,
597597
// and we should reject the block.
598+
warn!("Rejected block proposal. Block contains transactions beyond the replay set.";
599+
"txid" => %tx.txid(),
600+
"tx_index" => i,
601+
);
598602
return Err(BlockValidateRejectReason {
599603
reason_code: ValidateRejectCode::InvalidTransactionReplay,
600604
reason: "Block contains transactions beyond the replay set".into(),
@@ -653,6 +657,11 @@ impl NakamotoBlockProposal {
653657
}
654658
TransactionResult::Success(_) => {
655659
// Tx should have been included
660+
warn!("Rejected block proposal. Block doesn't contain replay transaction that should have been included.";
661+
"block_txid" => %tx.txid(),
662+
"block_tx_index" => i,
663+
"replay_txid" => %replay_tx.txid(),
664+
);
656665
return Err(BlockValidateRejectReason {
657666
reason_code: ValidateRejectCode::InvalidTransactionReplay,
658667
reason: "Transaction is not in the replay set".into(),

0 commit comments

Comments
 (0)