Skip to content

Commit 071f1cf

Browse files
committed
TQ: Implement prepare and commit for initial config
Initial configurations can be prepared and committed with the implemented handlers. This is tested along with aborts at Nexus for when the coordinator for the initial configuration has crashed in a new property based test. The new property based test runs all possible nodes in the universe as the system under test (SUT), rather than running only the coordinator. This allows a full deterministic simulation of the protocol and checking of invariants at all nodes. It's also easier to write and understand as we don't have to capture and mock replies to the coordinator. I had always intended to write this test, but started with modelling the coordinator first since I thought it would be easier to incrementally build the protocol that way. However, it appears just as easy to incrementally build with all nodes as the SUT. The new test does not have a model of the system, which is exceedingly hard to do for such a protocol. Instead the test checks invariants of the real state of the SUT after every action, and allows peppering in postconditions as necessary for each action or operation. The Node API has also changed to not worry about time at all, and instead deals in terms of connections and disconnections. This makes for simpler code IMO, and matches what was done for LRTQ. We always are operating over sprockets streams, which run over TLS over TCP and so it makes little sense to model things as if arbitrary packets can get dropped and reordered. As a result of the new proptest and the change in time usage, I've decided to drop the coordinator test altogether. It's too complicated for its value add and urgency is a priority.
1 parent 9a650c3 commit 071f1cf

File tree

11 files changed

+1134
-747
lines changed

11 files changed

+1134
-747
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

trust-quorum/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ bcs.workspace = true
1212
bootstore.workspace = true
1313
camino.workspace = true
1414
chacha20poly1305.workspace = true
15+
daft.workspace = true
1516
derive_more.workspace = true
1617
gfss.workspace = true
1718
hex.workspace = true

trust-quorum/src/coordinator_state.rs

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,11 @@
66
77
use crate::NodeHandlerCtx;
88
use crate::crypto::{LrtqShare, Sha3_256Digest, ShareDigestLrtq};
9-
use crate::messages::PeerMsg;
109
use crate::validators::{ReconfigurationError, ValidatedReconfigureMsg};
1110
use crate::{Configuration, Epoch, PeerMsgKind, PlatformId};
1211
use gfss::shamir::Share;
1312
use slog::{Logger, o, warn};
1413
use std::collections::{BTreeMap, BTreeSet};
15-
use std::time::Instant;
1614

1715
/// The state of a reconfiguration coordinator.
1816
///
@@ -29,10 +27,6 @@ use std::time::Instant;
2927
pub struct CoordinatorState {
3028
log: Logger,
3129

32-
/// When the reconfiguration started
33-
#[expect(unused)]
34-
start_time: Instant,
35-
3630
/// A copy of the message used to start this reconfiguration
3731
reconfigure_msg: ValidatedReconfigureMsg,
3832

@@ -42,9 +36,6 @@ pub struct CoordinatorState {
4236

4337
/// What is the coordinator currently doing
4438
op: CoordinatorOperation,
45-
46-
/// When to resend prepare messages next
47-
retry_deadline: Instant,
4839
}
4940

5041
impl CoordinatorState {
@@ -54,7 +45,6 @@ impl CoordinatorState {
5445
/// `PrepareMsg` so that it can be persisted.
5546
pub fn new_uninitialized(
5647
log: Logger,
57-
now: Instant,
5848
msg: ValidatedReconfigureMsg,
5949
) -> Result<(CoordinatorState, Configuration, Share), ReconfigurationError>
6050
{
@@ -81,7 +71,7 @@ impl CoordinatorState {
8171
prepare_acks: BTreeSet::new(),
8272
};
8373

84-
let state = CoordinatorState::new(log, now, msg, config.clone(), op);
74+
let state = CoordinatorState::new(log, msg, config.clone(), op);
8575

8676
// Safety: Construction of a `ValidatedReconfigureMsg` ensures that
8777
// `my_platform_id` is part of the new configuration and has a share.
@@ -92,7 +82,6 @@ impl CoordinatorState {
9282
/// A reconfiguration from one group to another
9383
pub fn new_reconfiguration(
9484
log: Logger,
95-
now: Instant,
9685
msg: ValidatedReconfigureMsg,
9786
last_committed_config: &Configuration,
9887
) -> Result<CoordinatorState, ReconfigurationError> {
@@ -107,7 +96,7 @@ impl CoordinatorState {
10796
new_shares,
10897
};
10998

110-
Ok(CoordinatorState::new(log, now, msg, config, op))
99+
Ok(CoordinatorState::new(log, msg, config, op))
111100
}
112101

113102
// Intentionally private!
@@ -116,20 +105,15 @@ impl CoordinatorState {
116105
// more specific, and perform validation of arguments.
117106
fn new(
118107
log: Logger,
119-
now: Instant,
120108
reconfigure_msg: ValidatedReconfigureMsg,
121109
configuration: Configuration,
122110
op: CoordinatorOperation,
123111
) -> CoordinatorState {
124-
// We want to send any pending messages immediately
125-
let retry_deadline = now;
126112
CoordinatorState {
127113
log: log.new(o!("component" => "tq-coordinator-state")),
128-
start_time: now,
129114
reconfigure_msg,
130115
configuration,
131116
op,
132-
retry_deadline,
133117
}
134118
}
135119

@@ -138,6 +122,10 @@ impl CoordinatorState {
138122
&self.reconfigure_msg
139123
}
140124

125+
pub fn op(&self) -> &CoordinatorOperation {
126+
&self.op
127+
}
128+
141129
// Send any required messages as a reconfiguration coordinator
142130
//
143131
// This varies depending upon the current `CoordinatorState`.
@@ -147,31 +135,56 @@ impl CoordinatorState {
147135
// will return a copy of it.
148136
//
149137
// This method is "in progress" - allow unused parameters for now
150-
#[expect(unused)]
151138
pub fn send_msgs(&mut self, ctx: &mut impl NodeHandlerCtx) {
152-
let now = ctx.now();
153-
if now < self.retry_deadline {
154-
return;
155-
}
156-
self.retry_deadline = now + self.reconfigure_msg.retry_timeout();
157139
match &self.op {
140+
#[expect(unused)]
158141
CoordinatorOperation::CollectShares {
159142
epoch,
160143
members,
161144
collected_shares,
162145
..
163146
} => {}
147+
#[expect(unused)]
164148
CoordinatorOperation::CollectLrtqShares { members, shares } => {}
165-
CoordinatorOperation::Prepare { prepares, prepare_acks } => {
166-
let rack_id = self.reconfigure_msg.rack_id();
149+
CoordinatorOperation::Prepare { prepares, .. } => {
167150
for (platform_id, (config, share)) in
168151
prepares.clone().into_iter()
169152
{
153+
if ctx.connected().contains(&platform_id) {
154+
ctx.send(
155+
platform_id,
156+
PeerMsgKind::Prepare { config, share },
157+
);
158+
}
159+
}
160+
}
161+
}
162+
}
163+
164+
// Send any required messages to a newly connected node
165+
// This method is "in progress" - allow unused parameters for now
166+
#[expect(unused)]
167+
pub fn send_msgs_to(
168+
&mut self,
169+
ctx: &mut impl NodeHandlerCtx,
170+
to: PlatformId,
171+
) {
172+
match &self.op {
173+
CoordinatorOperation::CollectShares {
174+
epoch,
175+
members,
176+
collected_shares,
177+
..
178+
} => {}
179+
CoordinatorOperation::CollectLrtqShares { members, shares } => {}
180+
CoordinatorOperation::Prepare { prepares, prepare_acks } => {
181+
let rack_id = self.reconfigure_msg.rack_id();
182+
if let Some((config, share)) = prepares.get(&to) {
170183
ctx.send(
171-
platform_id,
172-
PeerMsg {
173-
rack_id,
174-
kind: PeerMsgKind::Prepare { config, share },
184+
to,
185+
PeerMsgKind::Prepare {
186+
config: config.clone(),
187+
share: share.clone(),
175188
},
176189
);
177190
}
@@ -217,7 +230,6 @@ impl CoordinatorState {
217230
/// What should the coordinator be doing?
218231
pub enum CoordinatorOperation {
219232
// We haven't started implementing this yet
220-
#[expect(unused)]
221233
CollectShares {
222234
epoch: Epoch,
223235
members: BTreeMap<PlatformId, Sha3_256Digest>,
@@ -226,7 +238,6 @@ pub enum CoordinatorOperation {
226238
},
227239
// We haven't started implementing this yet
228240
// Epoch is always 0
229-
#[allow(unused)]
230241
CollectLrtqShares {
231242
members: BTreeMap<PlatformId, ShareDigestLrtq>,
232243
shares: BTreeMap<PlatformId, LrtqShare>,
@@ -250,4 +261,14 @@ impl CoordinatorOperation {
250261
CoordinatorOperation::Prepare { .. } => "prepare",
251262
}
252263
}
264+
265+
/// Return the members that have acked prepares, if the current operation
266+
/// is `Prepare`. Otherwise return an empty set.
267+
pub fn acked_prepares(&self) -> BTreeSet<PlatformId> {
268+
if let CoordinatorOperation::Prepare { prepare_acks, .. } = self {
269+
prepare_acks.clone()
270+
} else {
271+
BTreeSet::new()
272+
}
273+
}
253274
}

trust-quorum/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ mod node_ctx;
2121
mod persistent_state;
2222
mod validators;
2323
pub use configuration::Configuration;
24-
pub(crate) use coordinator_state::CoordinatorState;
24+
pub use coordinator_state::{CoordinatorOperation, CoordinatorState};
25+
2526
pub use crypto::RackSecret;
2627
pub use messages::*;
2728
pub use node::Node;
@@ -38,6 +39,7 @@ pub use persistent_state::{PersistentState, PersistentStateSummary};
3839
Eq,
3940
PartialOrd,
4041
Ord,
42+
Hash,
4143
Serialize,
4244
Deserialize,
4345
Display,

trust-quorum/src/messages.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{Configuration, Epoch, PlatformId, Threshold};
99
use gfss::shamir::Share;
1010
use omicron_uuid_kinds::RackUuid;
1111
use serde::{Deserialize, Serialize};
12-
use std::{collections::BTreeSet, time::Duration};
12+
use std::collections::BTreeSet;
1313

1414
/// A request from nexus informing a node to start coordinating a
1515
/// reconfiguration.
@@ -20,9 +20,6 @@ pub struct ReconfigureMsg {
2020
pub last_committed_epoch: Option<Epoch>,
2121
pub members: BTreeSet<PlatformId>,
2222
pub threshold: Threshold,
23-
24-
// The timeout before we send a follow up request to a peer
25-
pub retry_timeout: Duration,
2623
}
2724

2825
/// Messages sent between trust quorum members over a sprockets channel

0 commit comments

Comments
 (0)