Merge pull request #332 from DMDcoin/4.0

4.0.2 release

Author: SurfingNerd

CHANGELOG.md

@@ -1,6 +1,26 @@
+## Diamond Node Software 4.0.2
+
+
+### New behavior for validator nodes
+- [Autoshutdown if a Node becomes a regular Node](https://github.com/DMDcoin/diamond-node/issues/322)
+- [Remove empty blocks during key gen phases behaviour](https://github.com/DMDcoin/diamond-node/issues/327)
+- [Service Transaction cleanup (garbage collect)](https://github.com/DMDcoin/diamond-node/issues/172) 
+
+### RPC
+- [Gas price from contracts](https://github.com/DMDcoin/diamond-node/issues/159)
+
+### Further 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)
+- [FIXED: already included transactions are refretched from peers](https://github.com/DMDcoin/diamond-node/issues/196)
+- [FIXED:not staked nodes write log entries about service transactions](https://github.com/DMDcoin/diamond-node/issues/323)
+- [Gracefull Node Shutdown: increase to 15 seconds](https://github.com/DMDcoin/diamond-node/issues/321)
+
+
 ## 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)

Cargo.lock

@@ -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"
 license = "GPL-3.0"
 authors = [
 	"bit.diamonds developers",

Cargo.toml

@@ -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.");

bin/oe/run.rs

@@ -41,6 +41,8 @@ use crate::pool::{
     verifier, PendingOrdering, PendingSettings, PrioritizationStrategy,
 };
 
+use super::VerifiedTransaction;
+
 type Listener = (
     LocalTransactionsList,
     (listener::Notifier, listener::Logger),
@@ -413,6 +415,17 @@ impl TransactionQueue {
             .collect()
     }
 
+    /// Performs garbage collection on the pool of this `TransactionQueue` for free service transactions.
+    /// Removes transactions that are not valid anymore.
+    /// The process executes listener calls.
+    pub fn garbage_collect<F: Fn(&VerifiedTransaction) -> bool>(
+        &self,
+        service_transaction_check: F,
+    ) {
+        let mut pool = self.pool.write();
+        pool.garbage_collect(service_transaction_check);
+    }
+
     /// Computes unordered set of pending hashes.
     ///
     /// Since strict nonce-checking is not required, you may get some false positive future transactions as well.

crates/concensus/miner/src/pool/queue.rs

@@ -233,7 +233,9 @@ impl IoHandler<ClientIoMessage> for ClientIoHandler {
             ClientIoMessage::Execute(ref exec) => {
                 (*exec.0)(&self.client);
             }
-            _ => {} // ignore other messages
+            ClientIoMessage::NewChainHead => {
+                self.client.garbage_collect_in_queue();
+            }
         }
     }
 }

crates/ethcore/service/src/service.rs

@@ -306,6 +306,10 @@ pub struct Client {
 
     shutdown: Arc<ShutdownManager>,
 
+    /// block number and block hash of latest gc.
+    /// this information is used to avoid double garbage collection.
+    garbage_collect_latest_block: Mutex<(u64, H256)>,
+
     statistics: ClientStatistics,
 }
 
@@ -842,6 +846,8 @@ impl Importer {
             warn!("Failed to prune ancient state data: {}", e);
         }
 
+        client.schedule_garbage_collect_in_queue();
+
         route
     }
 
@@ -1107,6 +1113,7 @@ impl Client {
             importer,
             config,
             shutdown,
+            garbage_collect_latest_block: Mutex::new((0, H256::zero())),
             statistics,
         });
 
@@ -1540,6 +1547,52 @@ impl Client {
         }
     }
 
+    /// Schedule garbage collection of invalid service transactions from the transaction queue based on the given block hash.
+    pub fn schedule_garbage_collect_in_queue(&self) {
+        let m = ClientIoMessage::execute(|c| c.garbage_collect_in_queue());
+        if let Err(e) = self.io_channel.read().send(m) {
+            error!(target: "client", "Failed to schedule garbage collection in transaction queue for block {:?}", e);
+        }
+    }
+
+    /// Garbage collect invalid service transactions from the transaction queue based on the given block header.
+    pub fn garbage_collect_in_queue(&self) {
+        let machine = self.engine().machine();
+
+        match &self.block_header_decoded(BlockId::Latest) {
+            Some(block_header) => {
+                {
+                    // scope for mutex.
+                    let mut last_gc = self.garbage_collect_latest_block.lock();
+
+                    if block_header.number() == last_gc.0 && block_header.hash() == last_gc.1 {
+                        // already gced for this block, or gc is ongoing.
+                        // we can return here.
+                        return;
+                    }
+
+                    // we treat ongoing gc as DONE, to avoid blocking of the message channel
+                    last_gc.0 = block_header.number();
+                    last_gc.1 = block_header.hash();
+                }
+
+                // here hides an accepted race condition.
+                // latest block could change during long ongoing GCs.
+                // this could be avoided developing a more complex GC logic.
+                // but the GC blocks the tx queue, so it has to be blazing fast.
+                self.importer.miner.collect_garbage(|tx|
+                    match machine.verify_transaction(tx.signed(), block_header, self) {
+                        Ok(_) => true,
+                        Err(e) => {
+                            trace!(target: "client", "collected garbage transaction from {:?}: {:?} reason: {:?}", tx.signed().sender(), tx.signed().hash, e);                   
+                            false
+                        },
+                    });
+            }
+            None => {}
+        }
+    }
+
     fn check_garbage(&self) {
         self.chain.read().collect_garbage();
         self.importer.block_queue.collect_garbage();

crates/ethcore/src/client/client.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);
         }
     }
 

crates/ethcore/src/engines/hbbft/contracts/keygen_history.rs

@@ -35,11 +35,6 @@ pub fn get_posdao_epoch_start(
     call_const_staking!(c, staking_epoch_start_block)
 }
 
-pub fn start_time_of_next_phase_transition(client: &dyn EngineClient) -> Result<U256, CallError> {
-    let c = BoundContract::bind(client, BlockId::Latest, *STAKING_CONTRACT_ADDRESS);
-    call_const_staking!(c, start_time_of_next_phase_transition)
-}
-
 pub fn candidate_min_stake(client: &dyn EngineClient) -> Result<U256, CallError> {
     let c = BoundContract::bind(client, BlockId::Latest, *STAKING_CONTRACT_ADDRESS);
     call_const_staking!(c, candidate_min_stake)

crates/ethcore/src/engines/hbbft/contracts/staking.rs

@@ -112,7 +112,7 @@ impl HbbftEarlyEpochEndManager {
             signing_address: signing_address.clone(),
         };
 
-        info!(target: "engine", "early-epoch-end: HbbftEarlyEpochEndManager created. start_time {now:?}, start_block: {epoch_start_block}");
+        trace!(target: "engine", "early-epoch-end: HbbftEarlyEpochEndManager created. start_time {now:?}, start_block: {epoch_start_block}");
 
         return Some(result);
     }