From 5bcf071891c55062164cfe11f2295dcdb593ca3b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 3 Nov 2025 18:40:48 +0100 Subject: [PATCH 1/2] memorized lately received transactions to avoid requesting them again in ChainSync: lately_received_transactions. --- crates/ethcore/sync/src/chain/handler.rs | 9 +++++---- crates/ethcore/sync/src/chain/mod.rs | 11 +++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/ethcore/sync/src/chain/handler.rs b/crates/ethcore/sync/src/chain/handler.rs index a4a74f0ba..0c4e92ea5 100644 --- a/crates/ethcore/sync/src/chain/handler.rs +++ b/crates/ethcore/sync/src/chain/handler.rs @@ -884,10 +884,11 @@ impl SyncHandler { return Ok(()); } - if io - .chain() - .transaction_if_readable(&hash, &deadline.time_left()) - .is_none() + if !sync.lately_received_transactions.contains(&hash) + && io + .chain() + .transaction_if_readable(&hash, &deadline.time_left()) + .is_none() { sync.peers .get_mut(&peer_id) diff --git a/crates/ethcore/sync/src/chain/mod.rs b/crates/ethcore/sync/src/chain/mod.rs index 3a204b3c1..8494e8a7c 100644 --- a/crates/ethcore/sync/src/chain/mod.rs +++ b/crates/ethcore/sync/src/chain/mod.rs @@ -758,6 +758,8 @@ pub struct ChainSync { statistics: SyncPropagatorStatistics, /// memorizing currently pooled transaction to reduce the number of pooled transaction requests. asking_pooled_transaction_overview: PooledTransactionOverview, + /// memorized lately received transactions to avoid requesting them again. + lately_received_transactions: H256FastSet, } #[derive(Debug, Default)] @@ -849,6 +851,7 @@ impl ChainSync { new_transactions_stats_period: config.new_transactions_stats_period, statistics: SyncPropagatorStatistics::new(), asking_pooled_transaction_overview: PooledTransactionOverview::new(), + lately_received_transactions: H256FastSet::default(), }; sync.update_targets(chain); sync @@ -952,6 +955,9 @@ impl ChainSync { trace!(target: "sync", "Received {:?}", txs.iter().map(|t| t.hash).map(|t| t.0).collect::>()); } + self.lately_received_transactions + .extend(txs.iter().map(|tx| tx.hash())); + // Remove imported txs from all request queues let imported = txs.iter().map(|tx| tx.hash()).collect::(); for (pid, peer_info) in &mut self.peers { @@ -1024,6 +1030,7 @@ impl ChainSync { // Reactivate peers only if some progress has been made // since the last sync round of if starting fresh. self.active_peers = self.peers.keys().cloned().collect(); + self.lately_received_transactions.clear(); debug!(target: "sync", "resetting sync state to {:?}", self.state); } @@ -1267,6 +1274,7 @@ impl ChainSync { debug!(target: "sync", "sync_peer: {} force {} state: {:?}", peer_id, force, self.state ); + if !self.active_peers.contains(&peer_id) { trace!(target: "sync", "Skipping deactivated peer {}", peer_id); return; @@ -1441,6 +1449,9 @@ impl ChainSync { if let Some(peer) = self.peers.get_mut(&peer_id) { // info: this check should do nothing, if everything is tracked correctly, + peer.unfetched_pooled_transactions + .retain(|h| !self.lately_received_transactions.contains(h)); + if peer.asking_pooled_transactions.is_empty() { // todo: we might just request the same transactions from multiple peers here, at the same time. // we should keep track of how many replicas of a transaction we had requested. From 2c145457e64ca2ab38a5ee3b41ead2c151498292 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 3 Nov 2025 19:30:57 +0100 Subject: [PATCH 2/2] error handling for malicious keys --- .../engines/hbbft/contracts/keygen_history.rs | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs b/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs index ee41a17ef..a4d5abd73 100644 --- a/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs +++ b/crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs @@ -156,12 +156,25 @@ pub fn acks_of_address( if serialized_ack.is_empty() { return Err(CallError::ReturnValueInvalid); } - let deserialized_ack: Ack = bincode::deserialize(&serialized_ack).unwrap(); - let outcome = skg - .handle_ack(vmap.get(&address).unwrap(), deserialized_ack) - .unwrap(); + let deserialized_ack: Ack = match bincode::deserialize(&serialized_ack) { + Ok(ack) => ack, + Err(e) => { + error!(target: "engine", "Failed to deserialize Ack #{} for address {}: {:?}", n, address, e); + return Err(CallError::ReturnValueInvalid); + } + }; + + let outcome = match skg.handle_ack(vmap.get(&address).unwrap(), deserialized_ack) { + Ok(s) => s, + Err(e) => { + error!(target: "engine", "Failed to handle Ack #{} for address {}: {:?}", n, address, e); + return Err(CallError::ReturnValueInvalid); + } + }; + if let AckOutcome::Invalid(fault) = outcome { - panic!("Expected Ack Outcome to be valid. {:?}", fault); + error!(target: "engine", "Invalid Ack Outcome for #{} for address {}: {:?}", n, address, fault); + return Err(CallError::ReturnValueInvalid); } }