Move the coordinator to expect multiple nonces

This mirrors how Provided TXs handle topics.

Now, instead of managing a global nonce stream, we can use items such as plan
IDs as topics.

This massively benefits re-attempts, as else we'd need a NOP TX to clear unused
nonces.
This commit is contained in:
Luke Parker 2023-12-01 06:38:54 -05:00
parent 1c3e8af922
commit c82d1283af
7 changed files with 86 additions and 194 deletions

View file

@ -37,7 +37,7 @@ use ::tributary::{
mod tributary;
use crate::tributary::{
TributarySpec, SignData, Transaction, NonceDecider, scanner::RecognizedIdType, PlanIds,
TributarySpec, SignData, Transaction, scanner::RecognizedIdType, PlanIds,
};
mod db;
@ -698,24 +698,7 @@ async fn handle_processor_message<D: Db, P: P2p>(
}
}
TransactionKind::Signed(_) => {
log::trace!("getting next nonce for Tributary TX in response to processor message");
let nonce = loop {
let Some(nonce) =
NonceDecider::nonce(&txn, genesis, &tx).expect("signed TX didn't have nonce")
else {
// This can be None if the following events occur, in order:
// 1) We scanned the relevant transaction(s) in a Tributary block
// 2) The processor was sent a message and responded
// 3) The Tributary TXN has yet to be committed
log::warn!("nonce has yet to be saved for processor-instigated transaction");
sleep(Duration::from_millis(100)).await;
continue;
};
break nonce;
};
tx.sign(&mut OsRng, genesis, key, nonce);
tx.sign(&mut OsRng, genesis, key);
publish_signed_transaction(&mut txn, tributary, tx).await;
}
}
@ -1066,7 +1049,7 @@ pub async fn run<D: Db, Pro: Processors, P: P2p>(
}
});
move |set: ValidatorSet, genesis, id_type, id: Vec<u8>, nonce| {
move |set: ValidatorSet, genesis, id_type, id: Vec<u8>| {
log::debug!("recognized ID {:?} {}", id_type, hex::encode(&id));
let mut raw_db = raw_db.clone();
let key = key.clone();
@ -1104,7 +1087,7 @@ pub async fn run<D: Db, Pro: Processors, P: P2p>(
}),
};
tx.sign(&mut OsRng, genesis, &key, nonce);
tx.sign(&mut OsRng, genesis, &key);
let mut first = true;
loop {

View file

@ -88,7 +88,7 @@ async fn dkg_test() {
handle_new_blocks::<_, _, _, _, _, _, LocalP2p>(
&mut scanner_db,
key,
|_, _, _, _, _| async {
|_, _, _, _| async {
panic!("provided TX caused recognized_id to be called in new_processors")
},
&processors,
@ -114,7 +114,7 @@ async fn dkg_test() {
handle_new_blocks::<_, _, _, _, _, _, LocalP2p>(
&mut scanner_db,
&keys[0],
|_, _, _, _, _| async {
|_, _, _, _| async {
panic!("provided TX caused recognized_id to be called after Commitments")
},
&processors,
@ -193,7 +193,7 @@ async fn dkg_test() {
handle_new_blocks::<_, _, _, _, _, _, LocalP2p>(
&mut scanner_db,
&keys[0],
|_, _, _, _, _| async {
|_, _, _, _| async {
panic!("provided TX caused recognized_id to be called after some shares")
},
&processors,
@ -241,7 +241,7 @@ async fn dkg_test() {
handle_new_blocks::<_, _, _, _, _, _, LocalP2p>(
&mut scanner_db,
&keys[0],
|_, _, _, _, _| async { panic!("provided TX caused recognized_id to be called after shares") },
|_, _, _, _| async { panic!("provided TX caused recognized_id to be called after shares") },
&processors,
|_, _| async { panic!("test tried to publish a new Serai TX") },
&spec,
@ -311,7 +311,7 @@ async fn dkg_test() {
handle_new_blocks::<_, _, _, _, _, _, LocalP2p>(
&mut scanner_db,
&keys[0],
|_, _, _, _, _| async {
|_, _, _, _| async {
panic!("provided TX caused recognized_id to be called after DKG confirmation")
},
&processors,

View file

@ -27,7 +27,6 @@ use crate::{
processors::Processors,
tributary::{
Transaction, TributarySpec, SeraiBlockNumber, Topic, DataSpecification, DataSet, Accumulation,
nonce_decider::NonceDecider,
dkg_confirmer::DkgConfirmer,
scanner::{RecognizedIdType, RIDTrait},
FatallySlashed, DkgShare, PlanIds, ConfirmationNonces, AttemptDb, DataDb,
@ -498,11 +497,6 @@ pub(crate) async fn handle_application_tx<
genesis,
Topic::SubstrateSign(SubstrateSignableId::CosigningSubstrateBlock(hash)),
);
NonceDecider::handle_substrate_signable(
txn,
genesis,
SubstrateSignableId::CosigningSubstrateBlock(hash),
);
let block_number = SeraiBlockNumber::get(txn, hash)
.expect("CosignSubstrateBlock yet didn't save Serai block number");
@ -528,9 +522,7 @@ pub(crate) async fn handle_application_tx<
genesis,
Topic::SubstrateSign(SubstrateSignableId::Batch(batch)),
);
let nonce =
NonceDecider::handle_substrate_signable(txn, genesis, SubstrateSignableId::Batch(batch));
recognized_id(spec.set(), genesis, RecognizedIdType::Batch, batch.to_vec(), nonce).await;
recognized_id(spec.set(), genesis, RecognizedIdType::Batch, batch.to_vec()).await;
}
Transaction::SubstrateBlock(block) => {
@ -539,10 +531,9 @@ pub(crate) async fn handle_application_tx<
despite us not providing that transaction",
);
let nonces = NonceDecider::handle_substrate_block(txn, genesis, &plan_ids);
for (nonce, id) in nonces.into_iter().zip(plan_ids.into_iter()) {
for id in plan_ids.into_iter() {
AttemptDb::recognize_topic(txn, genesis, Topic::Sign(id));
recognized_id(spec.set(), genesis, RecognizedIdType::Plan, id.to_vec(), nonce).await;
recognized_id(spec.set(), genesis, RecognizedIdType::Plan, id.to_vec()).await;
}
}
@ -569,7 +560,6 @@ pub(crate) async fn handle_application_tx<
) {
Accumulation::Ready(DataSet::Participating(mut preprocesses)) => {
unflatten(spec, &mut preprocesses);
NonceDecider::selected_for_signing_substrate(txn, genesis, data.plan);
processors
.send(
spec.set().network,
@ -645,7 +635,6 @@ pub(crate) async fn handle_application_tx<
) {
Accumulation::Ready(DataSet::Participating(mut preprocesses)) => {
unflatten(spec, &mut preprocesses);
NonceDecider::selected_for_signing_plan(txn, genesis, data.plan);
processors
.send(
spec.set().network,

View file

@ -35,9 +35,6 @@ use tributary::{
mod db;
pub use db::*;
mod nonce_decider;
pub use nonce_decider::*;
mod dkg_confirmer;
mod handle;
@ -191,8 +188,8 @@ impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> Debug for SignData<Id
}
}
impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> ReadWrite for SignData<Id> {
fn read<R: io::Read>(reader: &mut R) -> io::Result<Self> {
impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> SignData<Id> {
fn read<R: io::Read>(reader: &mut R, nonce: u32) -> io::Result<Self> {
let plan = Id::decode(&mut scale::IoReader(&mut *reader))
.map_err(|_| io::Error::other("invalid plan in SignData"))?;
@ -217,7 +214,7 @@ impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> ReadWrite for SignDat
all_data
};
let signed = Signed::read(reader)?;
let signed = Signed::read_without_nonce(reader, nonce)?;
Ok(SignData { plan, attempt, data, signed })
}
@ -242,7 +239,7 @@ impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> ReadWrite for SignDat
writer.write_all(data)?;
}
self.signed.write(writer)
self.signed.write_without_nonce(writer)
}
}
@ -403,7 +400,7 @@ impl ReadWrite for Transaction {
commitments
};
let signed = Signed::read(reader)?;
let signed = Signed::read_without_nonce(reader, 0)?;
Ok(Transaction::DkgCommitments(attempt, commitments, signed))
}
@ -440,7 +437,7 @@ impl ReadWrite for Transaction {
let mut confirmation_nonces = [0; 64];
reader.read_exact(&mut confirmation_nonces)?;
let signed = Signed::read(reader)?;
let signed = Signed::read_without_nonce(reader, 1)?;
Ok(Transaction::DkgShares { attempt, shares, confirmation_nonces, signed })
}
@ -465,7 +462,8 @@ impl ReadWrite for Transaction {
let mut blame = vec![0; u16::from_le_bytes(blame_len).into()];
reader.read_exact(&mut blame)?;
let signed = Signed::read(reader)?;
// This shares a nonce with DkgConfirmed as only one is expected
let signed = Signed::read_without_nonce(reader, 2)?;
Ok(Transaction::InvalidDkgShare {
attempt,
@ -484,7 +482,7 @@ impl ReadWrite for Transaction {
let mut confirmation_share = [0; 32];
reader.read_exact(&mut confirmation_share)?;
let signed = Signed::read(reader)?;
let signed = Signed::read_without_nonce(reader, 2)?;
Ok(Transaction::DkgConfirmed(attempt, confirmation_share, signed))
}
@ -509,11 +507,11 @@ impl ReadWrite for Transaction {
Ok(Transaction::SubstrateBlock(u64::from_le_bytes(block)))
}
8 => SignData::read(reader).map(Transaction::SubstratePreprocess),
9 => SignData::read(reader).map(Transaction::SubstrateShare),
8 => SignData::read(reader, 0).map(Transaction::SubstratePreprocess),
9 => SignData::read(reader, 1).map(Transaction::SubstrateShare),
10 => SignData::read(reader).map(Transaction::SignPreprocess),
11 => SignData::read(reader).map(Transaction::SignShare),
10 => SignData::read(reader, 0).map(Transaction::SignPreprocess),
11 => SignData::read(reader, 1).map(Transaction::SignShare),
12 => {
let mut plan = [0; 32];
@ -557,7 +555,7 @@ impl ReadWrite for Transaction {
for commitments in commitments {
writer.write_all(commitments)?;
}
signed.write(writer)
signed.write_without_nonce(writer)
}
Transaction::DkgShares { attempt, shares, confirmation_nonces, signed } => {
@ -586,7 +584,7 @@ impl ReadWrite for Transaction {
}
writer.write_all(confirmation_nonces)?;
signed.write(writer)
signed.write_without_nonce(writer)
}
Transaction::InvalidDkgShare { attempt, accuser, faulty, blame, signed } => {
@ -602,14 +600,14 @@ impl ReadWrite for Transaction {
writer.write_all(&blame_len.to_le_bytes())?;
writer.write_all(blame.as_ref().unwrap_or(&vec![]))?;
signed.write(writer)
signed.write_without_nonce(writer)
}
Transaction::DkgConfirmed(attempt, share, signed) => {
writer.write_all(&[4])?;
writer.write_all(&attempt.to_le_bytes())?;
writer.write_all(share)?;
signed.write(writer)
signed.write_without_nonce(writer)
}
Transaction::CosignSubstrateBlock(block) => {
@ -663,21 +661,21 @@ impl TransactionTrait for Transaction {
match self {
Transaction::RemoveParticipant(_) => TransactionKind::Provided("remove"),
Transaction::DkgCommitments(_, _, signed) => TransactionKind::Signed(signed),
Transaction::DkgShares { signed, .. } => TransactionKind::Signed(signed),
Transaction::InvalidDkgShare { signed, .. } => TransactionKind::Signed(signed),
Transaction::DkgConfirmed(_, _, signed) => TransactionKind::Signed(signed),
Transaction::DkgCommitments(attempt, _, signed) => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
Transaction::DkgShares { attempt, signed, .. } => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
Transaction::InvalidDkgShare { attempt, signed, .. } => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
Transaction::DkgConfirmed(attempt, _, signed) => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
Transaction::CosignSubstrateBlock(_) => TransactionKind::Provided("cosign"),
Transaction::Batch(_, _) => TransactionKind::Provided("batch"),
Transaction::SubstrateBlock(_) => TransactionKind::Provided("serai"),
Transaction::SubstratePreprocess(data) => TransactionKind::Signed(&data.signed),
Transaction::SubstrateShare(data) => TransactionKind::Signed(&data.signed),
Transaction::SubstratePreprocess(data) => TransactionKind::Signed((b"substrate", data.0.plan, data.0.attempt).encode(), &data.signed),
Transaction::SubstrateShare(data) => TransactionKind::Signed((b"substrate", data.0.plan, data.0.attempt).encode(), &data.signed),
Transaction::SignPreprocess(data) => TransactionKind::Signed(&data.signed),
Transaction::SignShare(data) => TransactionKind::Signed(&data.signed),
Transaction::SignPreprocess(data) => TransactionKind::Signed((b"sign", data.0.plan, data.0.attempt).encode(), &data.signed),
Transaction::SignShare(data) => TransactionKind::Signed((b"sign", data.0.plan, data.0.attempt).encode(), &data.signed),
Transaction::SignCompleted { .. } => TransactionKind::Unsigned,
}
}
@ -729,7 +727,6 @@ impl Transaction {
rng: &mut R,
genesis: [u8; 32],
key: &Zeroizing<<Ristretto as Ciphersuite>::F>,
nonce: u32,
) {
fn signed(tx: &mut Transaction) -> &mut Signed {
match tx {
@ -756,7 +753,27 @@ impl Transaction {
let signed_ref = signed(self);
signed_ref.signer = Ristretto::generator() * key.deref();
signed_ref.nonce = nonce;
signed_ref.nonce = match tx {
Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"),
Transaction::DkgCommitments(_, _, _) => 0,
Transaction::DkgShares { .. } => 1,
Transaction::InvalidDkgShare { .. } => 2,
Transaction::DkgConfirmed(_, _, _) => 2,
Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"),
Transaction::Batch(_, _) => panic!("signing Batch"),
Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),
Transaction::SubstratePreprocess(_) => 0,
Transaction::SubstrateShare(_) => 1,
Transaction::SignPreprocess(_) => 0,
Transaction::SignShare(_) => 1,
Transaction::SignCompleted { .. } => panic!("signing SignCompleted"),
};
let sig_nonce = Zeroizing::new(<Ristretto as Ciphersuite>::F::random(rng));
signed(self).signature.R = <Ristretto as Ciphersuite>::generator() * sig_nonce.deref();

View file

@ -1,123 +0,0 @@
use serai_db::{Get, DbTxn, create_db};
use processor_messages::coordinator::SubstrateSignableId;
use crate::tributary::Transaction;
use scale::Encode;
const SUBSTRATE_CODE: u8 = 0;
const SUBSTRATE_SIGNING_CODE: u8 = 1;
const PLAN_CODE: u8 = 2;
const PLAN_SIGNING_CODE: u8 = 3;
create_db!(
NonceDeciderDb {
NextNonceDb: (genesis: [u8; 32]) -> u32,
ItemNonceDb: (genesis: [u8; 32], code: u8, id: &[u8]) -> u32,
}
);
impl NextNonceDb {
pub fn allocate_nonce(txn: &mut impl DbTxn, genesis: [u8; 32]) -> u32 {
let next = Self::get(txn, genesis).unwrap_or(3);
Self::set(txn, genesis, &(next + 1));
next
}
}
/// Decides the nonce which should be used for a transaction on a Tributary.
///
/// Deterministically builds a list of nonces to use based on the on-chain events and expected
/// transactions in response. Enables rebooting/rebuilding validators with full safety.
pub struct NonceDecider;
impl NonceDecider {
pub fn handle_substrate_signable(
txn: &mut impl DbTxn,
genesis: [u8; 32],
id: SubstrateSignableId,
) -> u32 {
let nonce_for = NextNonceDb::allocate_nonce(txn, genesis);
ItemNonceDb::set(txn, genesis, SUBSTRATE_CODE, &id.encode(), &nonce_for);
nonce_for
}
pub fn handle_substrate_block(
txn: &mut impl DbTxn,
genesis: [u8; 32],
plans: &[[u8; 32]],
) -> Vec<u32> {
let mut res = Vec::with_capacity(plans.len());
for plan in plans {
let nonce_for = NextNonceDb::allocate_nonce(txn, genesis);
ItemNonceDb::set(txn, genesis, PLAN_CODE, plan, &nonce_for);
res.push(nonce_for);
}
res
}
// TODO: The processor won't yield shares for this if the signing protocol aborts. We need to
// detect when we're expecting shares for an aborted protocol and insert a dummy transaction
// there.
pub fn selected_for_signing_substrate(
txn: &mut impl DbTxn,
genesis: [u8; 32],
id: SubstrateSignableId,
) {
let nonce_for = NextNonceDb::allocate_nonce(txn, genesis);
ItemNonceDb::set(txn, genesis, SUBSTRATE_SIGNING_CODE, &id.encode(), &nonce_for);
}
// TODO: Same TODO as selected_for_signing_substrate
pub fn selected_for_signing_plan(txn: &mut impl DbTxn, genesis: [u8; 32], plan: [u8; 32]) {
let nonce_for = NextNonceDb::allocate_nonce(txn, genesis);
ItemNonceDb::set(txn, genesis, PLAN_SIGNING_CODE, &plan, &nonce_for);
}
pub fn nonce(getter: &impl Get, genesis: [u8; 32], tx: &Transaction) -> Option<Option<u32>> {
match tx {
Transaction::RemoveParticipant(_) => None,
Transaction::DkgCommitments(attempt, _, _) => {
assert_eq!(*attempt, 0);
Some(Some(0))
}
Transaction::DkgShares { attempt, .. } => {
assert_eq!(*attempt, 0);
Some(Some(1))
}
// InvalidDkgShare and DkgConfirmed share a nonce due to the expected existence of only one
// on-chain
Transaction::InvalidDkgShare { attempt, .. } => {
assert_eq!(*attempt, 0);
Some(Some(2))
}
Transaction::DkgConfirmed(attempt, _, _) => {
assert_eq!(*attempt, 0);
Some(Some(2))
}
Transaction::CosignSubstrateBlock(_) => None,
Transaction::Batch(_, _) => None,
Transaction::SubstrateBlock(_) => None,
Transaction::SubstratePreprocess(data) => {
assert_eq!(data.attempt, 0);
Some(ItemNonceDb::get(getter, genesis, SUBSTRATE_CODE, &data.plan.encode()))
}
Transaction::SubstrateShare(data) => {
assert_eq!(data.attempt, 0);
Some(ItemNonceDb::get(getter, genesis, SUBSTRATE_SIGNING_CODE, &data.plan.encode()))
}
Transaction::SignPreprocess(data) => {
assert_eq!(data.attempt, 0);
Some(ItemNonceDb::get(getter, genesis, PLAN_CODE, &data.plan.encode()))
}
Transaction::SignShare(data) => {
assert_eq!(data.attempt, 0);
Some(ItemNonceDb::get(getter, genesis, PLAN_SIGNING_CODE, &data.plan.encode()))
}
Transaction::SignCompleted { .. } => None,
}
}
}

View file

@ -35,10 +35,10 @@ pub enum RecognizedIdType {
}
pub(crate) trait RIDTrait<FRid>:
Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>, u32) -> FRid
Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>) -> FRid
{
}
impl<FRid, F: Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>, u32) -> FRid>
impl<FRid, F: Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>) -> FRid>
RIDTrait<FRid> for F
{
}

View file

@ -81,6 +81,32 @@ impl ReadWrite for Signed {
}
}
impl Signed {
fn read_without_nonce<R: io::Read>(reader: &mut R, nonce: u32) -> io::Result<Self> {
let signer = Ristretto::read_G(reader)?;
let mut signature = SchnorrSignature::<Ristretto>::read(reader)?;
if signature.R.is_identity().into() {
// Anyone malicious could remove this and try to find zero signatures
// We should never produce zero signatures though meaning this should never come up
// If it does somehow come up, this is a decent courtesy
signature.zeroize();
Err(io::Error::other("signature nonce was identity"))?;
}
Ok(Signed { signer, nonce, signature })
}
fn write_without_nonce<W: io::Write>(&self, writer: &mut W) -> io::Result<()> {
// This is either an invalid signature or a private key leak
if self.signature.R.is_identity().into() {
Err(io::Error::other("signature nonce was identity"))?;
}
writer.write_all(&self.signer.to_bytes())?;
self.signature.write(writer)
}
}
#[allow(clippy::large_enum_variant)]
#[derive(Clone, PartialEq, Eq, Debug)]
pub enum TransactionKind<'a> {