Skip to content

Commit

Permalink
Remove usage of tuples for non-trivial Tributary transactions
Browse files Browse the repository at this point in the history
  • Loading branch information
kayabaNerve committed Dec 10, 2023
1 parent 24f4dae commit ee84753
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 48 deletions.
20 changes: 14 additions & 6 deletions coordinator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,11 @@ async fn handle_processor_message<D: Db, P: P2p>(
let txs = match msg.msg.clone() {
ProcessorMessage::KeyGen(inner_msg) => match inner_msg {
key_gen::ProcessorMessage::Commitments { id, commitments } => {
vec![Transaction::DkgCommitments(id.attempt, commitments, Transaction::empty_signed())]
vec![Transaction::DkgCommitments {
attempt: id.attempt,
commitments,
signed: Transaction::empty_signed(),
}]
}
key_gen::ProcessorMessage::InvalidCommitments { id: _, faulty } => {
// This doesn't need the ID since it's a Provided transaction which everyone will provide
Expand Down Expand Up @@ -487,7 +491,11 @@ async fn handle_processor_message<D: Db, P: P2p>(

match share {
Ok(share) => {
vec![Transaction::DkgConfirmed(id.attempt, share, Transaction::empty_signed())]
vec![Transaction::DkgConfirmed {
attempt: id.attempt,
confirmation_share: share,
signed: Transaction::empty_signed(),
}]
}
Err(p) => {
vec![Transaction::RemoveParticipant(p)]
Expand Down Expand Up @@ -596,13 +604,13 @@ async fn handle_processor_message<D: Db, P: P2p>(
preprocesses.into_iter().map(Into::into).collect(),
);

let intended = Transaction::Batch(
block.0,
match id.id {
let intended = Transaction::Batch {
block: block.0,
batch: match id.id {
SubstrateSignableId::Batch(id) => id,
_ => panic!("BatchPreprocess did not contain Batch ID"),
},
);
};

// If this is the new key's first Batch, only create this TX once we verify all
// all prior published `Batch`s
Expand Down
15 changes: 11 additions & 4 deletions coordinator/src/tests/tributary/dkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ async fn dkg_test() {
let mut commitments = vec![0; 256];
OsRng.fill_bytes(&mut commitments);

let mut tx =
Transaction::DkgCommitments(attempt, vec![commitments], Transaction::empty_signed());
let mut tx = Transaction::DkgCommitments {
attempt,
commitments: vec![commitments],
signed: Transaction::empty_signed(),
};
tx.sign(&mut OsRng, spec.genesis(), key);
txs.push(tx);
}
Expand All @@ -79,7 +82,7 @@ async fn dkg_test() {
.iter()
.enumerate()
.map(|(i, tx)| {
if let Transaction::DkgCommitments(_, commitments, _) = tx {
if let Transaction::DkgCommitments { commitments, .. } = tx {
(Participant::new((i + 1).try_into().unwrap()).unwrap(), commitments[0].clone())
} else {
panic!("txs had non-commitments");
Expand Down Expand Up @@ -319,7 +322,11 @@ async fn dkg_test() {
crate::tributary::generated_key_pair::<MemDb>(&mut txn, key, &spec, &key_pair, 0).unwrap();
txn.commit();

let mut tx = Transaction::DkgConfirmed(attempt, share, Transaction::empty_signed());
let mut tx = Transaction::DkgConfirmed {
attempt,
confirmation_share: share,
signed: Transaction::empty_signed(),
};
tx.sign(&mut OsRng, spec.genesis(), key);
txs.push(tx);
}
Expand Down
20 changes: 10 additions & 10 deletions coordinator/src/tests/tributary/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ fn serialize_transaction() {
OsRng.fill_bytes(&mut temp);
commitments.push(temp);
}
test_read_write(Transaction::DkgCommitments(
random_u32(&mut OsRng),
test_read_write(Transaction::DkgCommitments {
attempt: random_u32(&mut OsRng),
commitments,
random_signed_with_nonce(&mut OsRng, 0),
));
signed: random_signed_with_nonce(&mut OsRng, 0),
});
}

{
Expand Down Expand Up @@ -193,15 +193,15 @@ fn serialize_transaction() {
});
}

test_read_write(Transaction::DkgConfirmed(
random_u32(&mut OsRng),
{
test_read_write(Transaction::DkgConfirmed {
attempt: random_u32(&mut OsRng),
confirmation_share: {
let mut share = [0; 32];
OsRng.fill_bytes(&mut share);
share
},
random_signed_with_nonce(&mut OsRng, 2),
));
signed: random_signed_with_nonce(&mut OsRng, 2),
});

{
let mut key = [0; 32];
Expand All @@ -225,7 +225,7 @@ fn serialize_transaction() {
OsRng.fill_bytes(&mut block);
let mut batch = [0; 5];
OsRng.fill_bytes(&mut batch);
test_read_write(Transaction::Batch(block, batch));
test_read_write(Transaction::Batch { block, batch });
}
test_read_write(Transaction::SubstrateBlock(OsRng.next_u64()));

Expand Down
7 changes: 5 additions & 2 deletions coordinator/src/tests/tributary/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ async fn tx_test() {

// Create the TX with a null signature so we can get its sig hash
let block_before_tx = tributaries[sender].1.tip().await;
let mut tx =
Transaction::DkgCommitments(attempt, vec![commitments.clone()], Transaction::empty_signed());
let mut tx = Transaction::DkgCommitments {
attempt,
commitments: vec![commitments.clone()],
signed: Transaction::empty_signed(),
};
tx.sign(&mut OsRng, spec.genesis(), &key);

assert_eq!(tributaries[sender].1.add_transaction(tx.clone()).await, Ok(true));
Expand Down
8 changes: 4 additions & 4 deletions coordinator/src/tributary/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ impl<
self.fatal_slash_with_participant_index(i, "RemoveParticipant Provided TX").await
}

Transaction::DkgCommitments(attempt, commitments, signed) => {
Transaction::DkgCommitments { attempt, commitments, signed } => {
let Ok(_) = self.check_sign_data_len(signed.signer, commitments.len()).await else {
return;
};
Expand Down Expand Up @@ -456,10 +456,10 @@ impl<
.await;
}

Transaction::DkgConfirmed(attempt, shares, signed) => {
Transaction::DkgConfirmed { attempt, confirmation_share, signed } => {
let data_spec =
DataSpecification { topic: Topic::DkgConfirmation, label: Label::Share, attempt };
match self.handle_data(&data_spec, shares.to_vec(), &signed).await {
match self.handle_data(&data_spec, confirmation_share.to_vec(), &signed).await {
Accumulation::Ready(DataSet::Participating(shares)) => {
log::info!("got all DkgConfirmed for {}", hex::encode(genesis));

Expand Down Expand Up @@ -595,7 +595,7 @@ impl<
self.processors.send(self.spec.set().network, msg).await;
}

Transaction::Batch(_, batch) => {
Transaction::Batch { block: _, batch } => {
// Because this Batch has achieved synchrony, its batch ID should be authorized
AttemptDb::recognize_topic(
self.txn,
Expand Down
55 changes: 33 additions & 22 deletions coordinator/src/tributary/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,11 @@ pub enum Transaction {
RemoveParticipant(Participant),

// Once this completes successfully, no more instances should be created.
DkgCommitments(u32, Vec<Vec<u8>>, Signed),
DkgCommitments {
attempt: u32,
commitments: Vec<Vec<u8>>,
signed: Signed,
},
DkgShares {
attempt: u32,
// Sending Participant, Receiving Participant, Share
Expand All @@ -269,7 +273,11 @@ pub enum Transaction {
blame: Option<Vec<u8>>,
signed: Signed,
},
DkgConfirmed(u32, [u8; 32], Signed),
DkgConfirmed {
attempt: u32,
confirmation_share: [u8; 32],
signed: Signed,
},

DkgRemoval(SignData<[u8; 32]>),

Expand All @@ -281,7 +289,10 @@ pub enum Transaction {
// which would be binding over the block hash and automatically achieve synchrony on all
// relevant batches. ExternalBlock was removed for this due to complexity around the pipeline
// with the current processor, yet it would still be an improvement.
Batch([u8; 32], [u8; 5]),
Batch {
block: [u8; 32],
batch: [u8; 5],
},
// When a Serai block is finalized, with the contained batches, we can allow the associated plan
// IDs
SubstrateBlock(u64),
Expand Down Expand Up @@ -309,7 +320,7 @@ impl Debug for Transaction {
.debug_struct("Transaction::RemoveParticipant")
.field("participant", participant)
.finish(),
Transaction::DkgCommitments(attempt, _, signed) => fmt
Transaction::DkgCommitments { attempt, commitments: _, signed } => fmt
.debug_struct("Transaction::DkgCommitments")
.field("attempt", attempt)
.field("signer", &hex::encode(signed.signer.to_bytes()))
Expand All @@ -325,7 +336,7 @@ impl Debug for Transaction {
.field("accuser", accuser)
.field("faulty", faulty)
.finish_non_exhaustive(),
Transaction::DkgConfirmed(attempt, _, signed) => fmt
Transaction::DkgConfirmed { attempt, confirmation_share: _, signed } => fmt
.debug_struct("Transaction::DkgConfirmed")
.field("attempt", attempt)
.field("signer", &hex::encode(signed.signer.to_bytes()))
Expand All @@ -337,7 +348,7 @@ impl Debug for Transaction {
.debug_struct("Transaction::CosignSubstrateBlock")
.field("block", &hex::encode(block))
.finish(),
Transaction::Batch(block, batch) => fmt
Transaction::Batch { block, batch } => fmt
.debug_struct("Transaction::Batch")
.field("block", &hex::encode(block))
.field("batch", &hex::encode(batch))
Expand Down Expand Up @@ -404,7 +415,7 @@ impl ReadWrite for Transaction {

let signed = Signed::read_without_nonce(reader, 0)?;

Ok(Transaction::DkgCommitments(attempt, commitments, signed))
Ok(Transaction::DkgCommitments { attempt, commitments, signed })
}

2 => {
Expand Down Expand Up @@ -486,7 +497,7 @@ impl ReadWrite for Transaction {

let signed = Signed::read_without_nonce(reader, 2)?;

Ok(Transaction::DkgConfirmed(attempt, confirmation_share, signed))
Ok(Transaction::DkgConfirmed { attempt, confirmation_share, signed })
}

5 => SignData::read(reader).map(Transaction::DkgRemoval),
Expand All @@ -502,7 +513,7 @@ impl ReadWrite for Transaction {
reader.read_exact(&mut block)?;
let mut batch = [0; 5];
reader.read_exact(&mut batch)?;
Ok(Transaction::Batch(block, batch))
Ok(Transaction::Batch { block, batch })
}

8 => {
Expand Down Expand Up @@ -540,7 +551,7 @@ impl ReadWrite for Transaction {
writer.write_all(&u16::from(*i).to_le_bytes())
}

Transaction::DkgCommitments(attempt, commitments, signed) => {
Transaction::DkgCommitments { attempt, commitments, signed } => {
writer.write_all(&[1])?;
writer.write_all(&attempt.to_le_bytes())?;
if commitments.is_empty() {
Expand Down Expand Up @@ -604,10 +615,10 @@ impl ReadWrite for Transaction {
signed.write_without_nonce(writer)
}

Transaction::DkgConfirmed(attempt, share, signed) => {
Transaction::DkgConfirmed { attempt, confirmation_share, signed } => {
writer.write_all(&[4])?;
writer.write_all(&attempt.to_le_bytes())?;
writer.write_all(share)?;
writer.write_all(confirmation_share)?;
signed.write_without_nonce(writer)
}

Expand All @@ -621,7 +632,7 @@ impl ReadWrite for Transaction {
writer.write_all(block)
}

Transaction::Batch(block, batch) => {
Transaction::Batch { block, batch } => {
writer.write_all(&[7])?;
writer.write_all(block)?;
writer.write_all(batch)
Expand Down Expand Up @@ -658,7 +669,7 @@ impl TransactionTrait for Transaction {
match self {
Transaction::RemoveParticipant(_) => TransactionKind::Provided("remove"),

Transaction::DkgCommitments(attempt, _, signed) => {
Transaction::DkgCommitments { attempt, commitments: _, signed } => {
TransactionKind::Signed((b"dkg", attempt).encode(), signed)
}
Transaction::DkgShares { attempt, signed, .. } => {
Expand All @@ -667,7 +678,7 @@ impl TransactionTrait for Transaction {
Transaction::InvalidDkgShare { attempt, signed, .. } => {
TransactionKind::Signed((b"dkg", attempt).encode(), signed)
}
Transaction::DkgConfirmed(attempt, _, signed) => {
Transaction::DkgConfirmed { attempt, signed, .. } => {
TransactionKind::Signed((b"dkg", attempt).encode(), signed)
}

Expand All @@ -677,7 +688,7 @@ impl TransactionTrait for Transaction {

Transaction::CosignSubstrateBlock(_) => TransactionKind::Provided("cosign"),

Transaction::Batch(_, _) => TransactionKind::Provided("batch"),
Transaction::Batch { .. } => TransactionKind::Provided("batch"),
Transaction::SubstrateBlock(_) => TransactionKind::Provided("serai"),

Transaction::SubstrateSign(data) => {
Expand Down Expand Up @@ -736,16 +747,16 @@ impl Transaction {
let nonce = match tx {
Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"),

Transaction::DkgCommitments(_, _, _) => 0,
Transaction::DkgCommitments { .. } => 0,
Transaction::DkgShares { .. } => 1,
Transaction::InvalidDkgShare { .. } => 2,
Transaction::DkgConfirmed(_, _, _) => 2,
Transaction::DkgConfirmed { .. } => 2,

Transaction::DkgRemoval(data) => data.label.nonce(),

Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"),

Transaction::Batch(_, _) => panic!("signing Batch"),
Transaction::Batch { .. } => panic!("signing Batch"),
Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),

Transaction::SubstrateSign(data) => data.label.nonce(),
Expand All @@ -758,16 +769,16 @@ impl Transaction {
match tx {
Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"),

Transaction::DkgCommitments(_, _, ref mut signed) => signed,
Transaction::DkgCommitments { ref mut signed, .. } => signed,
Transaction::DkgShares { ref mut signed, .. } => signed,
Transaction::InvalidDkgShare { ref mut signed, .. } => signed,
Transaction::DkgConfirmed(_, _, ref mut signed) => signed,
Transaction::DkgConfirmed { ref mut signed, .. } => signed,

Transaction::DkgRemoval(ref mut data) => &mut data.signed,

Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"),

Transaction::Batch(_, _) => panic!("signing Batch"),
Transaction::Batch { .. } => panic!("signing Batch"),
Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),

Transaction::SubstrateSign(ref mut data) => &mut data.signed,
Expand Down

0 comments on commit ee84753

Please sign in to comment.