Adjust snapshot timing

core/src/consensus/tendermint/worker.rs

@@ -1645,6 +1645,8 @@ impl Worker {
             }
         };
 
+        self.send_snapshot_notify(c.as_ref(), enacted.as_slice());
+
         if self.step.is_commit() && (imported.len() + enacted.len() == 1) {
             let (_, committed_block_hash) = self.step.committed().expect("Commit state always has block_hash");
             if imported.first() == Some(&committed_block_hash) {
@@ -1662,28 +1664,6 @@ impl Worker {
             }
         }
 
-        let mut last_term_end = None;
-        for block_hash in &enacted {
-            let header = c.block_header(&BlockId::Hash(*block_hash)).expect("Block is enacted").decode();
-            let parent_header = match c.block_header(&BlockId::Hash(*header.parent_hash())) {
-                Some(h) => h.decode(),
-                // NOTE: Only the genesis block and the snapshot target don't have the parent in the blockchain
-                None => continue,
-            };
-            let term_seconds = if let Some(p) = c.term_common_params(parent_header.hash().into()) {
-                p.term_seconds()
-            } else {
-                continue
-            };
-            if super::engine::is_term_changed(&header, &parent_header, term_seconds) {
-                last_term_end = Some(*block_hash);
-            }
-        }
-        if let Some(last_term_end) = last_term_end {
-            // TODO: Reduce the snapshot frequency.
-            self.snapshot_notify_sender.notify(last_term_end);
-        }
-
         if let Some((last, rest)) = imported.split_last() {
             let (imported, last_proposal_header) = {
                 let header =
@@ -1718,6 +1698,26 @@ impl Worker {
         }
     }
 
+    // Notify once for the latest block even if multiple blocks have been enacted.
+    fn send_snapshot_notify(&mut self, c: &dyn ConsensusClient, enacted: &[BlockHash]) {
+        let mut last_snapshot_point = None;
+        for block_hash in enacted.iter().rev() {
+            let block_id = BlockId::Hash(*block_hash);
+            let last_term_finished_block_num = c.last_term_finished_block_num(block_id).expect("Block is enacted");
+            let block_number = c.block_number(&block_id).expect("Block number should exist for enacted block");
+
+            if let Some(params) = c.term_common_params(block_id) {
+                if params.era() == 1 && (last_term_finished_block_num + 1 == block_number) {
+                    last_snapshot_point = Some(block_hash);
+                }
+            }
+        }
+        if let Some(last_snapshot_point) = last_snapshot_point {
+            // TODO: Reduce the snapshot frequency.
+            self.snapshot_notify_sender.notify(*last_snapshot_point);
+        }
+    }
+
     fn send_proposal_block(
         &self,
         signature: SchnorrSignature,

test/src/e2e.dynval/2/snapshot.test.ts

@@ -17,13 +17,15 @@
 import * as chai from "chai";
 import { expect } from "chai";
 import * as chaiAsPromised from "chai-as-promised";
+import * as stake from "codechain-stakeholder-sdk";
 import * as fs from "fs";
 import "mocha";
 import * as path from "path";
 
 import mkdirp = require("mkdirp");
 import { validators } from "../../../tendermint.dynval/constants";
 import { PromiseExpect } from "../../helper/promise";
+import CodeChain from "../../helper/spawn";
 import { setTermTestTimeout, withNodes } from "../setup";
 
 chai.use(chaiAsPromised);
@@ -37,7 +39,8 @@ describe("Snapshot for Tendermint with Dynamic Validator", function() {
     const { nodes } = withNodes(this, {
         promiseExpect,
         overrideParams: {
-            maxNumOfValidators: 3
+            maxNumOfValidators: 3,
+            era: 1
         },
         validators: snapshotValidators.map((signer, index) => ({
             signer,
@@ -63,30 +66,32 @@ describe("Snapshot for Tendermint with Dynamic Validator", function() {
 
     it("should be exist after some time", async function() {
         const termWaiter = setTermTestTimeout(this, {
-            terms: 1
+            terms: 2
         });
         const termMetadata = await termWaiter.waitNodeUntilTerm(nodes[0], {
             target: 2,
             termPeriods: 1
         });
-
-        const blockHash = (await nodes[0].sdk.rpc.chain.getBlockHash(
-            termMetadata.lastTermFinishedBlockNumber
-        ))!;
-        const stateRoot = (await nodes[0].sdk.rpc.chain.getBlock(blockHash))!
-            .stateRoot;
+        const snapshotBlock = await getSnapshotBlock(nodes[0], termMetadata);
         expect(
-            fs.existsSync(
-                path.join(
-                    nodes[0].snapshotPath,
-                    blockHash.toString(),
-                    stateRoot.toString()
-                )
+            path.join(
+                nodes[0].snapshotPath,
+                snapshotBlock.hash.toString(),
+                snapshotBlock.stateRoot.toString()
             )
-        ).to.be.true;
+        ).to.satisfy(fs.existsSync);
     });
 
     afterEach(async function() {
         promiseExpect.checkFulfilled();
     });
 });
+
+async function getSnapshotBlock(
+    node: CodeChain,
+    termMetadata: stake.TermMetadata
+) {
+    const blockNumber = termMetadata.lastTermFinishedBlockNumber + 1;
+    await node.waitBlockNumber(blockNumber);
+    return (await node.sdk.rpc.chain.getBlock(blockNumber))!;
+}