diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dc8e0ede..e56002baf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,18 @@ +## Diamond Node Software 4.0.2 + +OPTIONAL Update + +New behavior for validator nodes as main feature +- [Autoshutdown if a Node becomes a regular Node](https://github.com/DMDcoin/diamond-node/issues/322) + +Further stability improvements +- [FIXED: received transactions are getting pooled, if announced by another peer](https://github.com/DMDcoin/diamond-node/issues/304) +- [FIXED: dropped transactions are getting pooled](https://github.com/DMDcoin/diamond-node/issues/303) +- [FIXED: key generation can panic if faulty validators write malicious parts](https://github.com/DMDcoin/diamond-node/issues/100) + ## Diamond Node Software 4.0.1 -First hotfix +OPTIONAL: First hotfix Mitigates the transaction spam caused by flaws in the transaction management of report disconnectivity transactions. - [Reduce Intervals for connectivity checks](https://github.com/DMDcoin/diamond-node/issues/313) diff --git a/Cargo.lock b/Cargo.lock index f41aa956e..d6d86f352 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -850,7 +850,7 @@ dependencies = [ [[package]] name = "diamond-node" -version = "4.0.1" +version = "4.0.2-rc1" dependencies = [ "ansi_term 0.10.2", "atty", @@ -3579,7 +3579,7 @@ dependencies = [ [[package]] name = "parity-version" -version = "4.0.1" +version = "4.0.2-rc1" dependencies = [ "parity-bytes", "rlp", diff --git a/Cargo.toml b/Cargo.toml index edeb127ca..963b350a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ description = "Diamond Node" name = "diamond-node" # NOTE Make sure to update util/version/Cargo.toml as well -version = "4.0.1" +version = "4.0.2-rc1" license = "GPL-3.0" authors = [ "bit.diamonds developers", diff --git a/bin/oe/run.rs b/bin/oe/run.rs index 92dc4feb5..c58504c86 100644 --- a/bin/oe/run.rs +++ b/bin/oe/run.rs @@ -723,7 +723,7 @@ impl RunningClient { .name("diamond-node-force-quit".to_string()) .spawn(move || { - let duration_soft = 5; + let duration_soft = 15; // we make a force quit if after 90 seconds, if this shutdown routine std::thread::sleep(Duration::from_secs(duration_soft)); warn!(target: "shutdown", "shutdown not happened within {duration_soft} seconds, starting force exiting the process."); 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); } } diff --git a/crates/ethcore/src/engines/hbbft/hbbft_state.rs b/crates/ethcore/src/engines/hbbft/hbbft_state.rs index 725e5b75f..fe94ec2bd 100644 --- a/crates/ethcore/src/engines/hbbft/hbbft_state.rs +++ b/crates/ethcore/src/engines/hbbft/hbbft_state.rs @@ -222,10 +222,29 @@ impl HbbftState { } if sks.is_none() { - info!(target: "engine", "We are not part of the HoneyBadger validator set - running as regular node."); + info!(target: "engine", "We are not part of the HoneyBadger validator set - Running as regular node."); peers_service .send_message(HbbftConnectToPeersMessage::DisconnectAllValidators) .ok()?; + + if self.is_validator() { + let is_syncing = if let Some(full) = client.as_full_client() { + full.is_major_syncing() + } else { + info!(target: "engine", "Node was a validator: cannot be determinated, because client is not a full client. (https://github.com/DMDcoin/diamond-node/issues/322.)"); + return Some(()); + }; + + if is_syncing { + debug!(target: "engine", "Node was a validator, and became regular node, but we are syncing, not shutting down Node as defined in https://github.com/DMDcoin/diamond-node/issues/322."); + } else { + info!(target: "engine", "Node was a validator, and became regular node. shutting down Node as defined in https://github.com/DMDcoin/diamond-node/issues/322."); + // for unit tests no problem, demand shutddown won't to anything if its a unit test. + // e2e tests needs adaptation. + // this gracefully shuts down a node, if it was a validator before, but now it is not anymore. + client.demand_shutdown(); + } + } return Some(()); } 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..2053653fe 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; @@ -1439,7 +1447,20 @@ impl ChainSync { // and if we have nothing else to do, get the peer to give us at least some of announced but unfetched transactions let mut to_send = H256FastSet::default(); if let Some(peer) = self.peers.get_mut(&peer_id) { - // info: this check should do nothing, if everything is tracked correctly, + // remove lately received transactions from unfetched list + // depending witch collection is larger, we choose the appropriate method. + if self.lately_received_transactions.len() > 0 { + if peer.unfetched_pooled_transactions.len() + > self.lately_received_transactions.len() + { + self.lately_received_transactions.iter().for_each(|h| { + peer.unfetched_pooled_transactions.remove(h); + }); + } else { + 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. diff --git a/crates/util/version/Cargo.toml b/crates/util/version/Cargo.toml index be1b4000e..45eee360f 100644 --- a/crates/util/version/Cargo.toml +++ b/crates/util/version/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "parity-version" # NOTE: this value is used for OpenEthereum version string (via env CARGO_PKG_VERSION) -version = "4.0.1" +version = "4.0.2-rc1" authors = [ "bit.diamonds developers", "OpenEthereum developers",