feat(host-contracts): implement context-aware KMSVerifier

[obatirou]

Closes https://github.com/zama-ai/fhevm-internal/issues/1062

Add per-context signer sets and thresholds so that old KMS contexts remain verifiable after governance rotates signers via defineNewContext. Includes destroyKmsContext, v0/v1 extraData parsing, reinitializeV2 migration path, legacy view redirection, and IKMSVerifier interface updates.

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for 8774604.

🟢 All jobs passed!

But CI Insights is watching 👀

[obatirou]

@claude /pr-review

[claude]

Claude finished @obatirou's task in 7m 8s —— View job

---

PR Review: Context-Aware KMSVerifier

• Pre-review validation
• Gather context (PR details, guidelines, linked issue)
• Run parallel review agents (bug hunting, guidelines, error handling)
• Validate findings
• Post final review

---

Summary

[AUTOMATED]

This PR implements context-aware KMS verification, allowing per-context signer sets and thresholds so that old KMS contexts remain verifiable after governance rotates signers. Two independent bug-hunting passes (opus), an error handling audit, and a test coverage analysis were run in parallel.

No critical or high-severity bugs were found. The core security properties are correctly implemented:

• ✅ Context isolation (cross-context signer rejection via per-context mappings)
• ✅ EIP-712 signature binding (extraData is part of the signed digest, preventing context ID swapping)
• ✅ Transient storage cleanup on all non-reverting paths (reverts auto-rollback tstore)
• ✅ Upgrade safety (reinitializer(3) guard, intentional __EIP712_init omission in reinitializeV2)
• ✅ Storage layout (new fields appended at offsets +3 through +7, no collision with legacy slots)
• ✅ Access control on all state-mutating functions (onlyACLOwner)
• ✅ Assembly correctness (mload(add(extraData, 33)) correctly reads 32 bytes at content offset 1)

---

🟡 Design Observations (Non-blocking)

1. setThreshold mutates a live context's threshold, breaking invariant of context immutability (Medium, Confidence: 85)

KMSVerifier.sol L201-205

setThreshold modifies the threshold of the current context in-place without creating a new context. Unlike defineNewContext (which increments the counter and preserves old contexts), setThreshold retroactively changes the rules. Any in-flight decryption proofs signed under the old threshold (via v0 or v1 extraData) will be rejected if the threshold was raised. This breaks the immutability invariant that the rest of the context-aware design carefully preserves.

Consider whether setThreshold should instead call defineNewContext with the existing signers and the new threshold, preserving the old threshold for in-flight proofs.

2. setThreshold emits NewContextSet — potentially misleading for indexers (Low, Confidence: 90)

KMSVerifier.sol L204

setThreshold emits NewContextSet when only the threshold changes (no new context ID is created). Downstream indexers keying on NewContextSet may interpret this as a context rotation. Consider a dedicated ThresholdUpdated event or documenting this clearly.

3. Inconsistent return-vs-revert in signature verification (Low, Informational, Confidence: 95)

KMSVerifier.sol L497-537

_verifySignaturesDigestForContext reverts for invalid context, zero sigs, below-threshold count, and invalid signer — but returns false if duplicates reduce the unique count below threshold. This asymmetry is by design (matching existing behavior, confirmed by test_VerifyDecryptionEIP712KMSSignaturesFailAsExpectedIfSameSignerIsUsedTwice), but the natspec should document this explicitly for integrators.

4. v0 extraData TOCTOU in batched transactions (Low, Informational, Confidence: 85)

KMSVerifier.sol L464-470

v0 extraData (0x00 or empty) resolves to currentKmsContextId at call time. In batched/multicall scenarios, if defineNewContext is called earlier in the same transaction, subsequent v0-routed verifications would resolve to the new context. Using v1 extraData with explicit context IDs eliminates this. Not a bug per se, but worth documenting for integrators.

---

🧪 Test Coverage Gaps (Recommended additions)

The test suite is thorough with 40+ tests covering context management, verification, migration, and edge cases. However, three high-priority gaps were identified:

1. v1 extraData with trailing bytes not tested (High priority)

The contract explicitly documents forward-compatibility at L474-475: "Trailing bytes after byte 33 are ignored for forward-compatibility." There's test_V0ExtraDataWithTrailingBytesUsesCurrentContext but no v1 equivalent. A test with abi.encodePacked(uint8(0x01), contextId, uint256(12345)) would exercise this.

Add v1 trailing bytes test →

2. getContextSignersAndThresholdFromExtraData has zero direct tests (High priority)

This is the new external function called by FHE.sol. It is never called directly in the test file — only indirectly through verification flow. Specifically untested: revert on destroyed contexts (L344-345), revert on non-existent contexts, v0 path, v1 path, malformed v1, unsupported version.

Add getContextSignersAndThreshold tests →

3. reinitializeV2 after initializeFromEmptyProxy not tested (High priority)

Both use reinitializer(3). There's test_ReinitializeV2CannotBeCalledTwice for double-reinitializeV2, but no test verifying that reinitializeV2() reverts on a contract already initialized via initializeFromEmptyProxy().

Add reinitialize guard test →

Medium priority gaps:

• reinitializeV2 ownership test (non-owner should be rejected)
• initializeFromEmptyProxy with duplicate/null signers or invalid thresholds
• Threshold == signers count edge case (requires all signatures)
• Multiple context transitions (5+ contexts) stress test

---

✅ Verified Correct

Concern	Result
_extractKmsContextId assembly (mload(add(extraData, 33)))	✅ Correct — reads 32 bytes at memory offset 33 (after 32-byte length + 1-byte version)
reinitializeV2 omitting __EIP712_init	✅ Intentional — V2 already initialized EIP-712 domain; annotation confirms
Context ID overflow	✅ Not a concern — Solidity 0.8+ reverts on overflow; ~10⁷⁷ rotations needed
ERC-7201 storage layout	✅ No collisions — new fields at offsets +3 through +7
FHE.sol ↔ KMSVerifier consistency	✅ Same EIP-712 digest; single getContextSignersAndThresholdFromExtraData call replaces two separate calls
Transient storage reentrancy	✅ Safe — cleanup on all non-revert paths; EVM revert rolls back tstore
_setContextSigners append-only enforcement	✅ Enforced implicitly — second call for same context would revert with KMSAlreadySigner
destroyKmsContext storage cleanup	✅ Not needed — context IDs are monotonic, never reused
Signature malleability	✅ Handled by OZ ECDSA.recover (KMSVerifier) and FhevmECDSA (FHE.sol)
calldata → memory conversion in getContextSignersAndThresholdFromExtraData	✅ Solidity handles automatically

---

Verdict

The contract implementation is solid and well-tested. The core security properties (context isolation, signature binding, upgrade safety) are correctly implemented. The design observation about setThreshold mutating live contexts is worth discussing but is a design choice, not a bug. The three high-priority test gaps would strengthen confidence and are recommended before merge.

LGTM 🟢 — with the suggested test coverage improvements.

---

[isaacdecoded]

LGTM 👍

[obatirou]

Just for posterity: isValidKmsContext was removed following the fact that Connector will consider a new context activated as soon as it sees the request and not rely on a KMSVerifier call for verification

• https://github.com/zama-ai/tech-spec/pull/389/
• https://zama-ai.slack.com/archives/C0A8BJPD5UG/p1772201562185559?thread_ts=1772121420.401659&cid=C0A8BJPD5UG