From 75f259cd5491f9c06b9e4fffb398bd1b95f745b4 Mon Sep 17 00:00:00 2001 From: sh-cha Date: Tue, 27 May 2025 15:26:58 +0900 Subject: [PATCH 1/2] return cached signature on regression request --- src/chain/state.rs | 64 ++++++++++++++++++++++++++++++++++++++++++++-- src/privval.rs | 2 +- src/session.rs | 26 ++++++++++++++----- 3 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/chain/state.rs b/src/chain/state.rs index 34cf7b6..5cdedfe 100644 --- a/src/chain/state.rs +++ b/src/chain/state.rs @@ -10,7 +10,7 @@ pub use self::error::{StateError, StateErrorKind}; use crate::{ error::{Error, ErrorKind::*}, - prelude::*, + prelude::*, privval::SignableMsg, }; use std::{ fs, @@ -19,11 +19,15 @@ use std::{ }; use tempfile::NamedTempFile; use tendermint::consensus; +use std::collections::BTreeMap; +use tendermint::block::{Height, Round}; /// State tracking for double signing prevention pub struct State { consensus_state: consensus::State, state_file_path: PathBuf, + + last_signs: BTreeMap<(Height,Round,i8), SignableMsg>, } impl State { @@ -46,6 +50,7 @@ impl State { Ok(Self { consensus_state, state_file_path: path.as_ref().to_owned(), + last_signs: BTreeMap::new(), }) } Err(e) if e.kind() == io::ErrorKind::NotFound => { @@ -172,6 +177,7 @@ impl State { let initial_state = Self { consensus_state, state_file_path: path.to_owned(), + last_signs: BTreeMap::new(), }; initial_state.sync_to_disk()?; @@ -204,12 +210,22 @@ impl State { Ok(()) } + + pub fn get_last_vote(&self, height: Height, round: Round, step: i8) -> Option { + self.last_signs.get(&(height, round, step)).cloned() + } + + pub fn save_sign(&mut self, height: Height, round: Round, step: i8, signable_msg: SignableMsg) { + self.last_signs.insert((height, round, step), signable_msg); + self.last_signs.retain(|key, _| height.value() < 100 + key.0.value()); + } } #[cfg(test)] mod tests { use super::*; - use tendermint::block; + use crate::privval::SignableMsg; + use tendermint::{block, proposal::{Proposal, Type}}; const EXAMPLE_BLOCK_ID: &str = "26C0A41F3243C6BCD7AD2DFF8A8D83A71D29D307B5326C227F734A1A512FE47D"; @@ -246,6 +262,7 @@ mod tests { State { consensus_state: $old_state, state_file_path: EXAMPLE_PATH.into(), + last_signs: BTreeMap::new(), } .update_consensus_state($new_state) .unwrap(); @@ -261,6 +278,7 @@ mod tests { let err = State { consensus_state: $old_state, state_file_path: EXAMPLE_PATH.into(), + last_signs: BTreeMap::new(), } .update_consensus_state($new_state) .expect_err("expected StateErrorKind::DoubleSign but succeeded"); @@ -311,4 +329,46 @@ mod tests { state!(1, 1, 2, None), state!(1, 1, 2, block_id!(EXAMPLE_BLOCK_ID)) ); + + #[test] + fn test_save_sign() { + let mut state = State { + consensus_state: state!(1, 1, 0, None), + state_file_path: EXAMPLE_PATH.into(), + last_signs: BTreeMap::new(), + }; + + state.save_sign(Height::from(1 as u32), Round::from(1 as u16), 0, SignableMsg::Proposal(Proposal{ + msg_type: Type::Proposal, + pol_round: None, + block_id: None, + timestamp: None, + signature: None, + height: Height::from(1 as u32), + round: Round::from(1 as u16), + })); + + assert_eq!(state.last_signs.len(), 1); + state.save_sign(Height::from(100 as u32), Round::from(1 as u16), 0, SignableMsg::Proposal(Proposal{ + msg_type: Type::Proposal, + pol_round: None, + block_id: None, + timestamp: None, + signature: None, + height: Height::from(100 as u32), + round: Round::from(1 as u16), + })); + assert_eq!(state.last_signs.len(), 2); + + state.save_sign(Height::from(101 as u32), Round::from(1 as u16), 0, SignableMsg::Proposal(Proposal{ + msg_type: Type::Proposal, + pol_round: None, + block_id: None, + timestamp: None, + signature: None, + height: Height::from(101 as u32), + round: Round::from(1 as u16), + })); + assert_eq!(state.last_signs.len(), 2); + } } diff --git a/src/privval.rs b/src/privval.rs index 9368734..59f3472 100644 --- a/src/privval.rs +++ b/src/privval.rs @@ -21,7 +21,7 @@ const PRECOMMIT_CODE: SignedMsgCode = 0x02; const PROPOSAL_CODE: SignedMsgCode = 0x20; /// Trait for signed messages. -#[derive(Debug)] +#[derive(Debug,Clone)] pub enum SignableMsg { /// Proposals Proposal(Proposal), diff --git a/src/session.rs b/src/session.rs index ca86332..c4243bd 100644 --- a/src/session.rs +++ b/src/session.rs @@ -1,13 +1,7 @@ //! A session with a validator node use crate::{ - chain::{self, state::StateErrorKind, Chain}, - config::ValidatorConfig, - connection::{tcp, unix::UnixConnection, Connection}, - error::{Error, ErrorKind::*}, - prelude::*, - privval::SignableMsg, - rpc::{Request, Response}, + chain::{self, state::StateErrorKind, Chain}, config::ValidatorConfig, connection::{tcp, unix::UnixConnection, Connection}, error::{Error, ErrorKind::*}, keyring::Signature, prelude::*, privval::SignableMsg, rpc::{Request, Response} }; use std::{os::unix::net::UnixStream, time::Instant}; use tendermint::{consensus, TendermintKey}; @@ -134,6 +128,10 @@ impl Session { panic!("chain '{}' missing from registry!", &self.config.chain_id); }); + if let Some(last_sign) = self.check_existing_sign(chain, &signable_msg) { + return Ok(last_sign.into()); + } + if let Some(remote_err) = self.update_consensus_state(chain, &signable_msg)? { // In the event of double signing we send a response to notify the validator return Ok(Response::error(signable_msg, remote_err)); @@ -165,6 +163,7 @@ impl Session { } } + self.save_sign(chain, signable_msg.clone()); Ok(signable_msg.into()) } @@ -229,6 +228,19 @@ impl Session { } } + fn check_existing_sign(&self, chain: &Chain, signable_msg: &SignableMsg) -> Option { + let request_state = signable_msg.consensus_state(); + let chain_state = chain.state.lock().unwrap(); + + chain_state.get_last_vote(request_state.height, request_state.round, request_state.step) + } + + fn save_sign(&self, chain: &Chain, signable_msg: SignableMsg) { + let request_state = signable_msg.consensus_state(); + let mut chain_state = chain.state.lock().unwrap(); + chain_state.save_sign(request_state.height, request_state.round, request_state.step, signable_msg); + } + /// Get the public key for (the only) public key in the keyring fn get_public_key(&mut self) -> Result { let registry = chain::REGISTRY.get(); From d82ea05f103cc202b08b2f86da31b0b1c2d34406 Mon Sep 17 00:00:00 2001 From: sh-cha Date: Tue, 27 May 2025 15:36:52 +0900 Subject: [PATCH 2/2] only check existing sign when the error is regression --- src/session.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/session.rs b/src/session.rs index c4243bd..eb5e5b0 100644 --- a/src/session.rs +++ b/src/session.rs @@ -128,12 +128,16 @@ impl Session { panic!("chain '{}' missing from registry!", &self.config.chain_id); }); - if let Some(last_sign) = self.check_existing_sign(chain, &signable_msg) { - return Ok(last_sign.into()); - } - if let Some(remote_err) = self.update_consensus_state(chain, &signable_msg)? { + // regression + if remote_err.code == 3 { + if let Some(last_sign) = self.check_existing_sign(chain, &signable_msg) { + return Ok(last_sign.into()); + } + } + // In the event of double signing we send a response to notify the validator + return Ok(Response::error(signable_msg, remote_err)); }