Fix `ledger.py` behaviour with retired nodes (#2457)

python/ccf/ledger.py

@@ -4,6 +4,7 @@
 import io
 import struct
 import os
+from enum import Enum
 
 from typing import BinaryIO, NamedTuple, Optional, Tuple, Dict
 
@@ -35,6 +36,12 @@ NODES_TABLE_NAME = "public:ccf.gov.nodes.info"
 WELL_KNOWN_SINGLETON_TABLE_KEY = bytes(bytearray(8))
 
 
+class NodeStatus(Enum):
+    PENDING = "Pending"
+    TRUSTED = "Trusted"
+    RETIRED = "Retired"
+
+
 def to_uint_32(buffer):
     return struct.unpack("@I", buffer)[0]
 
@@ -221,10 +228,15 @@ class LedgerValidator:
             for node_id, node_info in node_table.items():
                 node_id = node_id.decode()
                 node_info = json.loads(node_info)
-                # Add the nodes certificate
+                # Add the node certificate
                 self.node_certificates[node_id] = node_info["cert"].encode()
                 # Update node trust status
-                self.node_activity_status[node_id] = node_info["status"]
+                # Also record the seqno at which the node status changed to
+                # track when a primary node should stop issuing signatures
+                self.node_activity_status[node_id] = (
+                    node_info["status"],
+                    transaction_public_domain.get_seqno(),
+                )
 
         # This is a merkle root/signature tx if the table exists
         if SIGNATURE_TX_TABLE_NAME in tables:
@@ -240,13 +252,13 @@ class LedgerValidator:
                 # Get binary representations for the cert, existing root, and signature
                 cert = self.node_certificates[signing_node]
                 existing_root = bytes.fromhex(signature["root"])
-                signature = base64.b64decode(signature["sig"])
+                sig = base64.b64decode(signature["sig"])
 
                 tx_info = TxBundleInfo(
                     self.merkle,
                     existing_root,
                     cert,
-                    signature,
+                    sig,
                     self.node_activity_status,
                     signing_node,
                 )
@@ -255,6 +267,14 @@ class LedgerValidator:
                 # throws if ledger validation failed.
                 self._verify_tx_set(tx_info)
 
+                # Forget about nodes whose retirement has been committed
+                for node_id, (status, seqno) in list(self.node_activity_status.items()):
+                    if (
+                        status == NodeStatus.RETIRED.value
+                        and signature["commit_seqno"] >= seqno
+                    ):
+                        self.node_activity_status.pop(node_id)
+
                 self.last_verified_seqno = current_seqno
                 self.last_verified_view = current_view
 
@@ -274,8 +294,13 @@ class LedgerValidator:
 
     @staticmethod
     def _verify_node_status(tx_info: TxBundleInfo):
-        """Verify item 1, The merkle root is signed by a Trusted node in the given network"""
-        if tx_info.node_activity[tx_info.signing_node] != "Trusted":
+        """Verify item 1, The merkle root is signed by a valid node in the given network"""
+        # Note: A retired primary will still issue signature transactions until
+        # its retirement is committed
+        if tx_info.node_activity[tx_info.signing_node][0] not in (
+            NodeStatus.TRUSTED.value,
+            NodeStatus.RETIRED.value,
+        ):
             LOG.error(
                 f"The signing node {tx_info.signing_node!r} is not trusted by the network"
             )

tests/governance.py

@@ -8,7 +8,7 @@ import infra.network
 import infra.path
 import infra.proc
 import infra.net
-from infra.node import NodeStatus
+from ccf.ledger import NodeStatus
 import infra.e2e_args
 import suite.test_requirements as reqs
 import infra.logging_app as app

tests/infra/consortium.py

@@ -13,6 +13,7 @@ import infra.checker
 import infra.node
 import infra.crypto
 import infra.member
+from ccf.ledger import NodeStatus
 import ccf.proposal_generator
 import ccf.ledger
 from infra.proposal import ProposalState
@@ -296,12 +297,10 @@ class Consortium:
 
         with remote_node.client() as c:
             r = c.get(f"/node/network/nodes/{node_to_retire.node_id}")
-            assert r.body.json()["status"] == infra.node.NodeStatus.RETIRED.value
+            assert r.body.json()["status"] == NodeStatus.RETIRED.value
 
     def trust_node(self, remote_node, node_id, timeout=3):
-        if not self._check_node_exists(
-            remote_node, node_id, infra.node.NodeStatus.PENDING
-        ):
+        if not self._check_node_exists(remote_node, node_id, NodeStatus.PENDING):
             raise ValueError(f"Node {node_id} does not exist in state PENDING")
 
         if os.getenv("JS_GOVERNANCE"):
@@ -319,9 +318,7 @@ class Consortium:
             timeout=timeout,
         )
 
-        if not self._check_node_exists(
-            remote_node, node_id, infra.node.NodeStatus.TRUSTED
-        ):
+        if not self._check_node_exists(remote_node, node_id, NodeStatus.TRUSTED):
             raise ValueError(f"Node {node_id} does not exist in state TRUSTED")
 
     def remove_member(self, remote_node, member_to_remove):
@@ -566,5 +563,5 @@ class Consortium:
     def wait_for_all_nodes_to_be_trusted(self, remote_node, nodes, timeout=3):
         for n in nodes:
             self.wait_for_node_to_exist_in_store(
-                remote_node, n.node_id, timeout, infra.node.NodeStatus.TRUSTED
+                remote_node, n.node_id, timeout, NodeStatus.TRUSTED
             )

tests/infra/network.py

@@ -10,6 +10,7 @@ import infra.path
 import infra.proc
 import infra.node
 import infra.consortium
+from ccf.ledger import NodeStatus
 from ccf.tx_status import TxStatus
 from ccf.tx_id import TxID
 import random
@@ -589,9 +590,9 @@ class Network:
                 new_node.node_id,
                 timeout=timeout,
                 node_status=(
-                    infra.node.NodeStatus.PENDING
+                    NodeStatus.PENDING
                     if self.status == ServiceStatus.OPEN
-                    else infra.node.NodeStatus.TRUSTED
+                    else NodeStatus.TRUSTED
                 ),
             )
         except TimeoutError as e:

tests/infra/node.py

@@ -23,12 +23,6 @@ class NodeNetworkState(Enum):
     joined = auto()
 
 
-class NodeStatus(Enum):
-    PENDING = "Pending"
-    TRUSTED = "Trusted"
-    RETIRED = "Retired"
-
-
 class State(Enum):
     UNINITIALIZED = "Uninitialized"
     INITIALIZED = "Initialized"

tests/suite/test_requirements.py

@@ -1,9 +1,10 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the Apache 2.0 License.
 
+import infra.network
+from ccf.ledger import NodeStatus
 import functools
 
-import infra.network
 from loguru import logger as LOG
 from math import ceil
 
@@ -105,7 +106,7 @@ def can_kill_n_nodes(nodes_to_kill_count):
                 [
                     node
                     for node in r.body.json()["nodes"]
-                    if node["status"] == infra.node.NodeStatus.TRUSTED.value
+                    if node["status"] == NodeStatus.TRUSTED.value
                 ]
             )
             running_nodes_count = len(network.get_joined_nodes())