dev: Blocklist (#626) basic-review follow-ups

Non-blocking follow-ups from the basic review of #626 (Blocklist module). None block the PR.

Each item below is the verbatim text of its inline review comment on #626, with a link to that comment.

Hardening

• contracts/src/security/Blocklist.compact:11-17

  🔵 followup: Spell out the fail-open contract as a security obligation. Unlike Allowlist (fail-closed: an empty set rejects everyone), this is fail-open: an empty set permits everyone. The deny-list is only as strong as the consumer (a) gating _block/_unblock and (b) calling assertNotBlocked on every guarded path. A missed path silently lets a blocked account through, with no in-module signal. Add a short @notice SECURITY line stating the obligation and the consequence.

  added by claude (dev3-midnight-basic-review skill)

Docs

• contracts/src/security/Blocklist.compact:48

  🔵 followup: Add a one-line @notice/@dev on isBlocked (inherited by assertNotBlocked) that calling it discloses account to the public transcript. The module @notice covers the posture, but a dev reading only the circuit's API-ref entry sees @param account with no disclosure hint, and could leak an identifier when composing this into an otherwise-private contract.

  added by claude (dev3-midnight-basic-review skill)

Tests

• contracts/src/security/test/Blocklist.test.ts:111-123

  🔵 followup (test): This wiring test asserts a block shows up in the raw Blocklist__blocked ledger, but never asserts an unblock removes it. Close the asymmetry so the state invariant is checked against the resulting ledger output (the field indexers subscribe to), not only via the isBlocked circuit:

  it('should reflect unblock in the public ledger', async () => {
    const sim = await BlocklistSimulator.create();
    await sim.block(ALICE);
    expect((await sim.getPublicState()).Blocklist__blocked.member(ALICE)).toBe(true);
    await sim.unblock(ALICE);
    expect((await sim.getPublicState()).Blocklist__blocked.member(ALICE)).toBe(false);
  });
  

  added by claude (dev3-midnight-basic-review skill)

• contracts/src/security/test/Blocklist.test.ts:12

  🔵 followup (test): No test pins the all-zero Bytes<32> account. OZ Solidity special-cases address(0); this module deliberately does not, so a test guards against a future regression that adds an implicit exemption:

  const ZERO = new Uint8Array(32);
  it('should treat the all-zero account as an ordinary member', async () => {
    expect(await blocklist.isBlocked(ZERO)).toBe(false);
    await blocklist.block(ZERO);
    expect(await blocklist.isBlocked(ZERO)).toBe(true);
    await expect(blocklist.assertNotBlocked(ZERO)).rejects.toThrow('Blocklist: account blocked');
    await blocklist.unblock(ZERO);
    expect(await blocklist.isBlocked(ZERO)).toBe(false);
  });
  

  added by claude (dev3-midnight-basic-review skill)

• contracts/src/security/test/Blocklist.test.ts:56-64

  🔵 followup (test): The block→block→unblock case is asserted only through isBlocked. Exercise the check seam (assertNotBlocked) on the same path so both seams agree:

  it('should pass assertNotBlocked after a single unblock of a multiply-blocked account', async () => {
    await blocklist.block(ALICE);
    await blocklist.block(ALICE);
    await blocklist.unblock(ALICE);
    await blocklist.assertNotBlocked(ALICE);
  });
  

  added by claude (dev3-midnight-basic-review skill)

• contracts/src/security/test/Blocklist.test.ts:22-25

  🔵 followup (test): A view shouldn't mutate. Cheap assertion that isBlocked leaves the set untouched:

  it('should not mutate the set when querying isBlocked', async () => {
    const sim = await BlocklistSimulator.create();
    await sim.isBlocked(ALICE);
    expect((await sim.getPublicState()).Blocklist__blocked.member(ALICE)).toBe(false);
  });
  

  added by claude (dev3-midnight-basic-review skill)

Source: basic review of #626.

Review the referenced sections of contracts/src/security/Blocklist.compact and contracts/src/security/test/Blocklist.test.ts. Run the Blocklist tests first, then add the specified security and disclosure notices and cover ledger updates, the all-zero account, repeated blocking, and non-mutating queries. Done means every listed follow-up is addressed and the tests pass.