From 0b8181b912892cb48f37bae4413709f50553a4fe Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Sun, 13 Nov 2022 18:15:19 -0500
Subject: [PATCH] Remove the precommit signature hash
It cached signatures per-block. Precommit signatures are bound to each
round. This would lead to forming invalid commits when a commit should
be formed. Under debug, the machine would catch that and panic. On
release, it'd have everyone who wasn't a validator fail to continue
syncing.
---
substrate/tendermint/machine/src/lib.rs | 18 +++++++++---------
.../tendermint/machine/src/message_log.rs | 11 ++++-------
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/substrate/tendermint/machine/src/lib.rs b/substrate/tendermint/machine/src/lib.rs
index 43056f9f..166dbd6c 100644
--- a/substrate/tendermint/machine/src/lib.rs
+++ b/substrate/tendermint/machine/src/lib.rs
@@ -345,15 +345,15 @@ impl<N: Network + 'static> TendermintMachine<N> {
Ok(Some(block)) => {
let mut validators = vec![];
let mut sigs = vec![];
- for (v, sig) in self
- .block
- .log
- .precommitted
- .iter()
- .filter_map(|(k, (id, sig))| Some((*k, sig.clone())).filter(|_| id == &block.id()))
- {
- validators.push(v);
- sigs.push(sig);
+ // Get all precommits for this round
+ for (validator, msgs) in &self.block.log.log[&msg.round] {
+ if let Some(Data::Precommit(Some((id, sig)))) = msgs.get(&Step::Precommit) {
+ // If this precommit was for this block, include it
+ if id == &block.id() {
+ validators.push(*validator);
+ sigs.push(sig.clone());
+ }
+ }
}
let commit = Commit {
diff --git a/substrate/tendermint/machine/src/message_log.rs b/substrate/tendermint/machine/src/message_log.rs
index 0592160d..cb64676e 100644
--- a/substrate/tendermint/machine/src/message_log.rs
+++ b/substrate/tendermint/machine/src/message_log.rs
@@ -4,10 +4,7 @@ use crate::{ext::*, RoundNumber, Step, Data, DataFor, MessageFor, TendermintErro
pub(crate) struct MessageLog<N: Network> {
weights: Arc<N::Weights>,
- pub(crate) precommitted: HashMap<
- N::ValidatorId,
- (<N::Block as Block>::Id, <N::SignatureScheme as SignatureScheme>::Signature),
- >,
+ precommitted: HashMap<N::ValidatorId, <N::Block as Block>::Id>,
pub(crate) log: HashMap<RoundNumber, HashMap<N::ValidatorId, HashMap<Step, DataFor<N>>>>,
}
@@ -34,13 +31,13 @@ impl<N: Network> MessageLog<N> {
}
// If they already precommitted to a distinct hash, error
- if let Data::Precommit(Some((hash, sig))) = &msg.data {
- if let Some((prev, _)) = self.precommitted.get(&msg.sender) {
+ if let Data::Precommit(Some((hash, _))) = &msg.data {
+ if let Some(prev) = self.precommitted.get(&msg.sender) {
if hash != prev {
Err(TendermintError::Malicious(msg.sender))?;
}
}
- self.precommitted.insert(msg.sender, (*hash, sig.clone()));
+ self.precommitted.insert(msg.sender, *hash);
}
msgs.insert(step, msg.data);