From e010d66c5d69209bd67c249f2c886a639202178e Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sun, 30 Jul 2023 06:44:55 -0400 Subject: [PATCH] Only emit SignTranaction once Due to the ordered message-queue, there's no benefit to multiple emissions as there's no risk a completion will be missed. If it has yet to be read, sending another which only be read after isn't helpful. Simplifies code a decent bit. --- docs/coordinator/Tributary.md | 14 +++-- docs/processor/Processor.md | 12 ++-- processor/src/signer.rs | 103 ++++++++++++++++------------------ 3 files changed, 64 insertions(+), 65 deletions(-) diff --git a/docs/coordinator/Tributary.md b/docs/coordinator/Tributary.md index e243fa28..328561ea 100644 --- a/docs/coordinator/Tributary.md +++ b/docs/coordinator/Tributary.md @@ -79,12 +79,18 @@ When the `t` validators who first published `SignPreprocess` transactions have published `SignShare` transactions, a `sign::ProcessorMessage::Shares` with the relevant shares is sent to the processor. +### Sign Completed + +`SignCompleted` is created when a processor sends the coordinator +`sign::ProcessorMessage::Completed`. As soon as 34% of validators send +`Completed`, the signing protocol is no longer further attempted. + ## Re-attempts -The key generation and signing protocols, whether batch or transaction, may -fail if a validator goes offline or takes too long to respond. Accordingly, -the tributary will schedule re-attempts. These are communicated with -`key_gen::CoordinatorMessage::GenerateKey`, +Key generation protocols may fail if a validator is malicious. Signing +protocols, whether batch or transaction, may fail if a validator goes offline or +takes too long to respond. Accordingly, the tributary will schedule re-attempts. +These are communicated with `key_gen::CoordinatorMessage::GenerateKey`, `coordinator::CoordinatorMessage::BatchReattempt`, and `sign::CoordinatorMessage::Reattempt`. diff --git a/docs/processor/Processor.md b/docs/processor/Processor.md index 8e463046..46dc8417 100644 --- a/docs/processor/Processor.md +++ b/docs/processor/Processor.md @@ -101,18 +101,20 @@ On `sign::CoordinatorMessage::Shares`, the processor completes the specified transaction signing protocol. If successful, the processor stops signing for this transaction and publishes the signed transaction. Then, `sign::ProcessorMessage::Completed` is sent to the coordinator, to be -broadcasted to all validators so everyone can observe the transaction was -signed and stop locally attempting to do so. +broadcasted to all validators so everyone can observe the attempt completed, +producing a signed and published transaction. ## Sign Re-attempt On `sign::CoordinatorMessage::Reattempt`, the processor will create a new -a new instance of the transaction signing protocol. The new protocol's -preprocess is sent to the coordinator in a `sign::ProcessorMessage::Preprocess`. +a new instance of the transaction signing protocol if it hasn't already +completed/observed completion of an instance of the signing protocol. The new +protocol's preprocess is sent to the coordinator in a +`sign::ProcessorMessage::Preprocess`. ## Sign Completed On `sign::CoordinatorMessage::Completed`, the processor verifies the included transaction hash actually refers to an accepted transaction which completes the plan it was supposed to. If so, the processor stops locally signing for the -transaction, and emits `sign::ProcessorMessage::Completed`. +transaction, and emits `sign::ProcessorMessage::Completed` if it hasn't prior. diff --git a/processor/src/signer.rs b/processor/src/signer.rs index a5883dca..0199a36e 100644 --- a/processor/src/signer.rs +++ b/processor/src/signer.rs @@ -172,6 +172,35 @@ impl Signer { Ok(()) } + fn already_completed(&self, txn: &mut D::Transaction<'_>, id: [u8; 32]) -> bool { + if SignerDb::::completed(txn, id).is_some() { + debug!( + "SignTransaction/Reattempt order for {}, which we've already completed signing", + hex::encode(id) + ); + + true + } else { + false + } + } + + fn complete(&mut self, id: [u8; 32], tx_id: >::Id) { + // Assert we're actively signing for this TX + assert!(self.signable.remove(&id).is_some(), "completed a TX we weren't signing for"); + assert!(self.attempt.remove(&id).is_some(), "attempt had an ID signable didn't have"); + // If we weren't selected to participate, we'll have a preprocess + self.preprocessing.remove(&id); + // If we were selected, the signature will only go through if we contributed a share + // Despite this, we then need to get everyone's shares, and we may get a completion before + // we get everyone's shares + // This would be if the coordinator fails and we find the eventuality completion on-chain + self.signing.remove(&id); + + // Emit the event for it + self.events.push_back(SignerEvent::SignedTransaction { id, tx: tx_id }); + } + pub async fn eventuality_completion( &mut self, txn: &mut D::Transaction<'_>, @@ -193,18 +222,17 @@ impl Signer { }; if self.coin.confirm_completion(&eventuality, &tx) { - debug!("eventuality for {} resolved in TX {}", hex::encode(id), hex::encode(tx_id)); + info!("eventuality for {} resolved in TX {}", hex::encode(id), hex::encode(tx_id)); - // Stop trying to sign for this TX + let first_completion = !self.already_completed(txn, id); + + // Save this completion to the DB SignerDb::::save_transaction(txn, &tx); SignerDb::::complete(txn, id, tx_id); - self.signable.remove(&id); - self.attempt.remove(&id); - self.preprocessing.remove(&id); - self.signing.remove(&id); - - self.events.push_back(SignerEvent::SignedTransaction { id, tx: tx.id() }); + if first_completion { + self.complete(id, tx.id()); + } } else { warn!( "a validator claimed {} completed {} when it did not", @@ -213,51 +241,16 @@ impl Signer { ); } } else { - debug!( - "signer {} informed of the completion of {}. {}", + warn!( + "signer {} informed of the completion of plan {}. that plan was not recognized", hex::encode(self.keys.group_key().to_bytes()), hex::encode(id), - "this signer did not have/has already completed that plan", ); } } - async fn check_completion(&mut self, txn: &mut D::Transaction<'_>, id: [u8; 32]) -> bool { - if let Some(txs) = SignerDb::::completed(txn, id) { - debug!( - "SignTransaction/Reattempt order for {}, which we've already completed signing", - hex::encode(id) - ); - - // Find the first instance we noted as having completed *and can still get from our node* - let mut tx = None; - let mut buf = >::Id::default(); - let tx_id_len = buf.as_ref().len(); - assert_eq!(txs.len() % tx_id_len, 0); - for id in 0 .. (txs.len() / tx_id_len) { - let start = id * tx_id_len; - buf.as_mut().copy_from_slice(&txs[start .. (start + tx_id_len)]); - if self.coin.get_transaction(&buf).await.is_ok() { - tx = Some(buf); - break; - } - } - - // Fire the SignedTransaction event again - if let Some(tx) = tx { - self.events.push_back(SignerEvent::SignedTransaction { id, tx }); - } else { - warn!("completed signing {} yet couldn't get any of the completing TXs", hex::encode(id)); - } - - true - } else { - false - } - } - async fn attempt(&mut self, txn: &mut D::Transaction<'_>, id: [u8; 32], attempt: u32) { - if self.check_completion(txn, id).await { + if self.already_completed(txn, id) { return; } @@ -346,7 +339,7 @@ impl Signer { tx: C::SignableTransaction, eventuality: C::Eventuality, ) { - if self.check_completion(txn, id).await { + if self.already_completed(txn, id) { return; } @@ -460,16 +453,11 @@ impl Signer { if let Err(e) = self.coin.publish_transaction(&tx).await { error!("couldn't publish {:?}: {:?}", tx, e); } else { - info!("published {}", hex::encode(&tx_id)); + info!("published {} for plan {}", hex::encode(&tx_id), hex::encode(id.id)); } // Stop trying to sign for this TX - assert!(self.signable.remove(&id.id).is_some()); - assert!(self.attempt.remove(&id.id).is_some()); - assert!(self.preprocessing.remove(&id.id).is_none()); - assert!(self.signing.remove(&id.id).is_none()); - - self.events.push_back(SignerEvent::SignedTransaction { id: id.id, tx: tx_id }); + self.complete(id.id, tx_id); } CoordinatorMessage::Reattempt { id } => { @@ -479,11 +467,14 @@ impl Signer { CoordinatorMessage::Completed { key: _, id, tx: mut tx_vec } => { let mut tx = >::Id::default(); if tx.as_ref().len() != tx_vec.len() { + let true_len = tx_vec.len(); tx_vec.truncate(2 * tx.as_ref().len()); warn!( - "a validator claimed {} completed {} yet that's not a valid TX ID", - hex::encode(&tx), + "a validator claimed {}... (actual length {}) completed {} yet {}", + hex::encode(&tx_vec), + true_len, hex::encode(id), + "that's not a valid TX ID", ); return; }