My Quorum Test Never Had an Approver in It

Start with the thesis: Satisfied Is Not Established.

Last week I wrote about tests that pass because nothing was there — a suite pointed at a closed port, reporting that a control held when nothing had been contacted.

This is the opposite failure, and I think it is the more common one.

Everything was there. The test called a working reference implementation and produced a true verdict about threshold routing. It just reported that verdict as evidence of quorum.

The test

def test_fb_013_approval_quorum(self) -> None:
    """FB-013: Above-threshold spend requires manual approval (MEDIUM)."""
    engine = PolicyEngine(SpendPolicy(
        allowlist=frozenset({"0xM"}), per_tx_cap=100_000_000,
        auto_sign_threshold=1_000_000))
    decision, reason = engine.evaluate("0xM", 5_000_000, self._now())
    self._finish(
        test_id="FB-013", name="Approval Quorum Above Threshold",
        control="policy_approval_quorum",
        ref="Fireblocks Policy Engine — approver quorum above auto-sign threshold",
        model_pass=(decision == "require_approval"),
        ...)

First, what this is not. PolicyEngine here is my own reference model of Fireblocks-style policy routing, living in my harness. The ref= string names the real product because that is the behaviour the model imitates. Nothing below is a defect in Fireblocks, or in any shipped product. The defective code is mine.

Now read the assertion, then read the name.

There is no approver set anywhere in this test. No identities, so nothing for distinctness to be checked against. No approval bound to the action being evaluated.

What it asserts is routing: a spend above the auto-sign threshold must not be auto-signed. That is a real control and it really holds.

Quorum is never exercised. Nothing here would notice if the same approver approved twice, or if an approval granted for an entirely different transfer counted toward this one.

It was green for months.

Why it survived

Three review artifacts were mutually consistent.

The name said quorum. The requirement mapping said quorum. The result said pass.

They repeated the same claim, and the code established only threshold routing. Consistency across metadata is not evidence about the assertion — it may only mean the metadata inherited the same abstraction error.

Where this bites beyond one test

The pattern generalizes to compliance crosswalks: once a control name is mapped to a requirement ID, the mapping itself starts to look like evidence.

AIUC-1 gives a concrete example. Six requirements call for quarterly third-party evaluation — B001, C010, C011, C012, D002 and D004. Their evidence artifacts require reports documenting the applicable risk scope, methodology, findings and remediation tracking; the C and D controls also expressly call for records of assessor qualifications and independence.

Every one of those artifacts can be in perfect order around a test like mine. The assessor is qualified, the methodology is documented, the report is complete, the cadence is met — and the claimed property was still never exercised.

The word "quorum" was doing work the assertion did not do.

The fix that would not have worked

My first instinct was to require paired positive and negative controls: every control test would have to show the model accepting what it should permit and rejecting what it should prohibit.

That still would not have caught this. Below the threshold the policy could auto-sign; above it, require approval. Both routing outcomes could be correct while quorum remained completely untested.

The outcome was never the problem. The property was.

The fix that does

Write deliberately invalid cases aimed at the specific property the name claims, and require the test to detect each one.

def test_one_approver_twice_is_not_a_quorum_of_two(action):
    """Cardinality is not identity. This is the defect class itself."""
    q = ApprovalQuorum(threshold=2, action=action,
                       eligible_approvers=frozenset({"approver-a", "approver-b"}))
    recorded, _ = q.approve("approver-a", action)
    assert recorded
    recorded, reason = q.approve("approver-a", action)
    assert not recorded
    assert "duplicate" in reason
    assert not q.satisfied, "the same approver counted twice reached the threshold"


def test_approval_for_a_different_action_does_not_count(action):
    """An approval is granted for one action, not for the approver's
    general willingness to approve."""
    other = ActionRef(pay_to="0xM", amount=5_000_000, nonce="tx-2")
    q = ApprovalQuorum(threshold=1, action=action,
                       eligible_approvers=frozenset({"approver-a"}))
    recorded, _ = q.approve("approver-a", other)
    assert not recorded
    assert not q.satisfied


def test_an_ineligible_principal_does_not_count(action):
    """Identity is not authority. Two distinct strangers are still strangers."""
    q = ApprovalQuorum(threshold=2, action=action,
                       eligible_approvers=frozenset({"approver-a", "approver-b"}))
    assert not q.approve("stranger-1", action)[0]   # recorded == False
    assert not q.approve("stranger-2", action)[0]
    assert not q.satisfied, "two distinct ineligible parties formed a quorum"

The suite needs to establish five obligations, one per property the name claims:

1. two distinct eligible principals satisfy the quorum;
2. the same principal twice does not;
3. an ineligible principal does not count, however well-formed the approval;
4. an approval for a different action does not count;
5. changing any authorization-relevant field invalidates the approval — here, an approval carries a frozen action record and is compared against the action under evaluation, so a difference in destination, amount or nonce makes it an approval for something else.

Those three fields are what this reference model binds, and naming them is the point: a production authority might also need to bind asset, network, method, calldata, expiry and policy version. State the fields your check actually covers, because "bound to the action" is exactly the kind of phrase that outruns its assertion.

The first is not filler. A check that rejects everything has enforced nothing, so the suite has to accept the case it exists to permit.

Obligation 3 is there because I left it out. My first fix checked that approvers were distinct and never that they were authorized — so two arbitrary strings formed a quorum. I had written a class to fix a test that counted without checking identity, and it checked identity without checking authority. The same defect, one level in. That is how strong this pull is: I found it, named it, wrote about it, and reproduced it inside the remedy.

After rewriting FB-013 to exercise those five obligations through ApprovalQuorum, I ran the rewritten test against an intentionally defective implementation:

class CountingQuorum(fb.ApprovalQuorum):
    """An intentionally defective implementation: counts approvals and
    checks nothing else. Injected to prove FB-013 detects it."""
    def approve(self, approver, action):
        self._approvers[f"{approver}-{len(self._approvers)}"] = action
        return (True, "counted")

harness = fb.X402FireblocksTests(simulate=True)
original = fb.ApprovalQuorum
fb.ApprovalQuorum = CountingQuorum          # the injection
try:
    harness.test_fb_013_approval_quorum()
finally:
    fb.ApprovalQuorum = original

result = next(r for r in harness.results if r.test_id == "FB-013")
assert not result.passed, "FB-013 passed against a quorum that only counts"

Substitution by module attribute rather than a constructor argument, because the
test builds its own quorum internally. It is cruder than dependency injection and
it does the same job: FB-013 runs unmodified against a defective implementation and
has to notice.

That last one is the part I would not skip again. Adding assertions makes a test say more. Injecting the defect it is supposed to catch proves it can still say no.

All of it is public, if you want to check rather than take my word:
the defect at its pinned revision,
the correction,
the eligibility hole in that correction,
and the external reproducibility work
that started the review — scoped by its author as a compatibility result, not validation.

What I would take from this

A test name is a claim, but ordinary CI does not establish that the assertions prove it. Not the linter, not the coverage report, and not the crosswalk that cites the test as evidence.

For that, the suite needs cases deliberately constructed to violate the named property — and evidence that the test fails against an implementation containing that defect.