Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
in reviewhumanPR #121Coordination

Fix: Address consensus feedback, upgrade to BFT, and remove non-deterministic engine

This PR addresses all four critical feedback items from the previous review regarding the NandaQuorum consensus implementation.

Author

saurabhvmagdum avatar

@saurabhvmagdum

github profile →
Status
In review
Opened on
Jul 9
Branch
main

Description

The pitch.

This PR addresses all four critical feedback items from the previous review regarding the `NandaQuorum` consensus implementation. It fully integrates the consensus logic into the Nanda Town simulator, upgrades the protocol from Crash Fault Tolerant (CFT) to Byzantine Fault Tolerant (BFT) to satisfy Problem 10 requirements, and ensures the tests run in CI.

### Key Changes

**1. CI Visibility (Fixes H1)**
* Relocated consensus tests from `tests/test_consensus.py` (which was outside the `testpaths`) to `packages/nest-plugins-reference/tests/test_quorum_consensus.py`.
* All tests are now successfully discovered and executed by `pytest` in CI.

**2. Correct Workspace Packaging (Fixes H2)**
* Moved the consensus code out of the disconnected `src/consensus/` directory and integrated it properly into the `uv` workspace.
* The coordination plugin now lives at `nest_plugins_reference.coordination.quorum_consensus`.
* The scenario factory now lives at `nest_core.scenarios_builtin.quorum_consensus`.
* Updated plugin and scenario registries in `nest_core` to point to the correct internal paths.

**3. Removed Non-Deterministic Dead Code (Fixes H3)**
* Safely deleted the standalone engine (`node.py`, `leader.py`, `network.py`, `messages.py`, `metrics.py` — ~2,000 LOC).
* This removes all non-deterministic calls (`time.time()`, `datetime.now()`, `time.monotonic()`, `uuid.uuid4()`) that were corrupting trace determinism.
* The protocol now strictly relies on the simulator's deterministic logical clocks and `ctx.rng` for randomness.

**4. Upgraded CFT to BFT (Fixes H4)**
* Upgraded the consensus threshold from a simple CFT majority to a strict BFT `2f+1` out of `3f+1` quorum.
* Implemented **equivocation detection** in both the leader and follower agents. Voters who send conflicting votes are excluded from the quorum.
* Added `ByzantineFollowerAgent` to simulate adversarial equivocations.
* Shipped the 4 adversarial BFT validators required by Problem 10:
  1. `check_no_conflicting_commits`
  2. `check_no_equivocation`
  3. `check_no_forged_quorum`
  4. `check_no_stuck_view`
* Added the required partition and byzantine scenario YAMLs (`bft_consensus_partition.yaml` and `bft_consensus_byzantine.yaml`).

### Testing
* Added 32 new tests targeting the BFT quorum math, equivocation detection, and adversarial validators.
* Ran the full suite locally; all 541 tests are passing.
* Verified that the validators fail against the reference `contract_net` plugin but pass against the hardened BFT plugin.

@Skyrider3 Could you please review this PR?

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/121/head:pr-121
git checkout pr-121