feat(contracts): TEE-attested service approval commitments

[drewstone]

Summary

Workstream B + C from the Tangle Cloud app-store program plan:

• B — AppRegistry.sol (greenfield) — on-chain home for blueprint apps: publisher namespaces, verified flag, per-app metadata pointers with strict version monotonicity, full version history, two-step namespace transfer, governance-only verify/revoke. UUPS-upgradeable + AccessControl.
• C — TEE-attested OperatorSecurityCommitment — operators commit to a specific TEE backend + measurement + nonce-binding at approval time; the live attestation produced when the workload provisions can be cross-checked against the on-chain commitment. DirectTdx is rejected at approval (mirror of ai-agent-sandbox-blueprint PR Update MSBM w/ new features in Tangle (same branch name) #50 policy).

Builds on the security primitives PR #115 merged earlier today (MBSMRegistry, BLS commitment paths in ServicesApprovals, OperatorSecurityCommitment types).

Workstream B — AppRegistry

Storage

• mapping(bytes32 => address) _namespaceOwner — namespace → owning publisher.
• mapping(bytes32 => bool) _namespaceVerified — governance-set verification flag.
• mapping(bytes32 => NamespaceTransfer) _pendingTransfers — two-step transfer state.
• mapping(bytes32 => AppRecord) _apps — per-app record.
• mapping(bytes32 => AppMetadataPointer[]) _appHistory — every version ever pinned, in order.
• mapping(bytes32 => EnumerableSet.Bytes32Set) _namespaceApps — O(1) listing per namespace.
• uint256[44] private __gap — reserved for facet-style upgrades.

Surface

• claimNamespace(bytes32) — first-claimer wins.
• transferNamespace(bytes32, address) / acceptNamespace(bytes32) — two-step with stale-proposal rejection (proposer must still own the namespace at accept time).
• publishApp(bytes32 namespace, bytes32 appId, uint64 blueprintId, AppMetadataPointer) — publisher-only; appId must be globally unique; version != 0; metadataHash != 0.
• pinAppVersion(bytes32 appId, AppMetadataPointer) — strictly monotonic version; previous pointer remains in history; refreshes verified mirror; updates frozenAt.
• Governance: setVerified(bytes32, bool), revokeApp(bytes32, string reason).
• Views: getApp, getAppHistory, getAppFrozenAt, listApps(namespace), appCount(namespace), namespaceOwner, pendingNamespaceTransfer, isNamespaceVerified, appExists.

Roles

• DEFAULT_ADMIN_ROLE administers the role hierarchy (timelock recommended).
• UPGRADER_ROLE authorises UUPS upgrades.
• GOVERNANCE_ROLE flips verified and revokes apps.

Errors

Unauthorized, NamespaceTaken, AppExists, AppNotFound, MetadataHashMismatch, Revoked, InvalidNamespace, InvalidAppId, InvalidVersion, NoPendingTransfer, StaleTransferProposal.

Tests (test/registry/AppRegistryTest.t.sol, 23 cases)

• Publish + pin + history happy path.
• Unauthorized publish (non-owner).
• Two-step namespace transfer: propose, re-propose-overwrites-prior, accept; rejection of bob's accept after carol takes over the proposal.
• Chained multi-hop transfers (alice → bob → carol) — proposer stays in sync.
• setVerified governance-only; verified flips through getApp.
• revokeApp blocks further pins; double-revoke reverts; non-governance revoke reverts.
• pinAppVersion rejects equal/lower versions; rejects zero hash.
• listApps + appCount via EnumerableSet (insertion order, swap-pop on remove).

Workstream C — TEE-attested commitments

Types (src/libraries/Types.sol)

enum TeeBackend {
    Phala,
    AwsNitro,
    GcpConfidential,
    AzureSkr,
    DirectTdx        // recognised but rejected at approval
}

struct TeeAttestationCommitment {
    TeeBackend backend;
    bytes32 expectedMeasurement;  // MRTD/MRENCLAVE/PCR-equivalent
    bytes32 nonceBinding;         // e.g. service request hash
    uint64 expiresAt;             // 0 = no expiry; non-zero must be future
}

Enum values are append-only by policy (storage hazard otherwise).

Approval entrypoint

function approveServiceWithTeeCommitments(
    uint64 requestId,
    Types.AssetSecurityCommitment[] calldata commitments,
    uint256[4] calldata blsPubkey,
    uint256[2] calldata popSignature,
    Types.TeeAttestationCommitment[] calldata teeCommitments
) external whenNotPaused nonReentrant;

Validates each TEE commitment (rejects DirectTdx, rejects past expiries), records the per-operator array under _requestTeeCommitments[requestId][operator] and emits TeeCommitmentRecorded. Activation copies the array onto _serviceTeeCommitments[serviceId][operator] via TangleServicesFacet._persistTeeCommitments.

Read surface

function getTeeCommitment(uint64 serviceId, address operator)
    external view returns (Types.TeeAttestationCommitment[] memory commitments);

Returned array matches storage order. Empty if the operator approved without TEE commitments.

Storage layout impact

TangleStorage.__gap reduced 44 → 42 to accommodate the two new mappings:

mapping(uint64 => mapping(address => Types.TeeAttestationCommitment[]))
    internal _requestTeeCommitments;
mapping(uint64 => mapping(address => Types.TeeAttestationCommitment[]))
    internal _serviceTeeCommitments;

Errors

DirectTdxNotPermitted, TeeCommitmentExpired(uint64 expiresAt, uint64 nowTimestamp), MeasurementMismatch(bytes32 expected, bytes32 observed), TeeCommitmentLengthMismatch(uint256 commitmentCount, uint256 teeCommitmentCount).

Tests (test/tangle/TeeCommitmentApprovalTest.t.sol, 9 cases)

• Happy path: approve with single Nitro commitment, fetch via getTeeCommitment.
• DirectTdx rejected at approval (DirectTdxNotPermitted).
• Past-expiry commitment rejected (TeeCommitmentExpired).
• Multiple operators with distinct backends (Nitro / Phala / GCP) in one quote.
• Commitment persistence onto the activated service via _persistTeeCommitments.
• Empty TEE commitments array allowed (back-compat with non-TEE flows).

Test plan

• Local Solidity formatted with forge fmt on changed files only.
• Local forge build (in flight at push time on a 341-file recompile after the formatting pass; CI runs the canonical build + tests).
• CI to run forge test --match-path 'test/registry/*' and test/tangle/TeeCommitment*.
• CI to run the full existing tnt-core test suite for regressions.
• Reviewer to confirm storage gap accounting is correct (44 → 42 with two new mappings).
• Reviewer to spot-check enum append-only convention (TeeBackend).

Out of scope (deferred to follow-ups, captured in the program plan)

• DCAP / Nitro / Phala verifier libraries on-chain — current design persists the commitment and assumes the off-chain provisioning oracle (or _handleProvisionResult hook in ai-agent-sandbox-blueprint) does the verifier walk. Bringing those on-chain costs significant gas; tracked under Workstream C P2 in the plan.
• setBackendRootCa(backend, ca) governance call for verifier root rotation — needs the on-chain verifier first.
• AppRegistry indexer schema (Envio/Hasura) for off-chain catalog listing — needs to ship coupled to this contract; tracked under Workstream B P2.
• Real AWS Nitro production validation — Workstream E in the program plan.

Notes

This pairs with ai-agent-sandbox-blueprint#55 (Workstream E: Firecracker port mappings + auth-path GC). Together they round out the contract / runtime story for the app-store program; surface rendering + brand adoption is in dapp#3164.

[tangletools]

✅ No Blockers — 57b333d1

Readiness 87/100 · Confidence 95/100 · 4 findings (1 medium, 3 low)

	kimi	glm	aggregate
Readiness	87	92	87
Confidence	95	95	95
Correctness	92	95	92
Security	91	94	91
Testing	93	95	93
Architecture	88	93	88

Read every changed file. The TEE commitment feature is well-structured: auth-before-writes ordering is correct, nonReentrant guard is in place, storage gap is correctly reduced by 2 for 2 new mappings, nonce replay prevention is sound, gas-griefing cap is justified and documented, and the request→service migration ordering comment is accurate. One documentation bug in Errors.sol will cause off-chain integrators to derive the wrong nonce and hit silent reverts; everything else is nits or consistent with existing codebase patterns. | Read all 8 changed files in full, verified all 23 tests pass

🟠 MEDIUM Errors.InvalidNonceBinding comment has wrong nonce formula — integrators will always compute the wrong value — src/libraries/Errors.sol

Line 455: the NatSpec states nonceBinding MUST equal keccak256(abi.encode(requestId, address(this), block.chainid)). The actual implementation in ServicesApprovals.teeNonceFor (line 315) is keccak256(abi.encode("tangle.tee.nonce", requestId, address(this), block.chainid)). The domain separator "tangle.tee.nonce" is omitted from the comment. Any operator SDK or off-chain tool that follows this comment verbatim will produce a different 32-byte value and receive InvalidNonceBinding on every submission — silently, with no indication that the fo

🟡 LOW Double O(n) operator-list scan per approveServiceWithTeeCommitments call — src/core/ServicesApprovals.sol

Lines 208 and 349: _requireApprovingOperator iterates _requestOperators[requestId] to confirm msg.sender is in the list, then _approveServiceWithCommitmentsInternal (called at line 218) repeats the identical loop at line 349. Under the nonReentrant guard no state can change between the two calls, so the second scan is pure waste — linear in operator count per approval. Existing entrypoints (`approveServ

🟡 LOW _requestTeeCommitments never deleted after activation — orphaned storage slots — src/facets/tangle/TangleServicesFacet.sol

Lines 337-348: _persistTeeCommitments copies commitment arrays from _requestTeeCommitments[requestId][op] to _serviceTeeCommitments[serviceId][op] but never deletes the source. At the documented ~60K gas per entry and up to 8 entries × N operators, the orphaned slots are non-trivial. This is consistent with _persistResourceCommitments (line 294-311) which also leaves _requestResourceRequirements intact, so it matches the existing design choice — but both forfeit SSTORE refunds. Worth a one-line `d

🟡 LOW Errors.InvalidNonceBinding @dev comment omits domain prefix — src/libraries/Errors.sol

Line 454-456: The @dev comment says nonceBinding MUST equal keccak256(abi.encode(requestId, address(this), block.chainid)) but the actual implementation in ServicesApprovals.sol:314-316 uses keccak256(abi.encode("tangle.tee.nonce", requestId, address(this), block.chainid)). An integrator reading the error doc to compute the canonical nonce would derive the wrong value and get rejected. Fix: add the 'tangle.tee.nonce' prefix to the @dev comment.

---

_{tangletools · 2026-05-04T20:12:34Z · trace}

[tangletools]

✅ Approved — 5 non-blocking findings

The AppRegistry and TEE attestation commitment features (6 of 14 changed files) are clean, well-tested, and architecturally sound. However this PR also bundles four serious regressions in the governance, rewards, and streaming-payment layers: the TangleTimelock._setMinDelay function now writes to hardcoded slot 0x33 instead of the correct ERC-7201 namespaced slot, meaning updateDelay silently fails to persist; the updateDelay auth model was downgraded from self-call to DEFAULT_ADMIN_ROLE (governance bypass); nonReentrant was removed from two load-bearing streaming-payment drip functions; and I

🟠 MEDIUM TeeAttestationCommitment.nonceBinding not validated — anti-replay protection is opt-in — src/core/ServicesApprovals.sol

Lines 231-248 (_validateTeeCommitments): validates backend != DirectTdx and expiresAt > now, but does not check nonceBinding != bytes32(0). The Types.TeeAttestationCommitment NatSpec (Types.sol) documents: 'Caller-supplied nonce that binds this commitment to a specific challenge ... so a stale attestation cannot be replayed against a fresh request.' If an operator supplies nonceBinding = 0, the commitment has no challenge binding and a recycled attestation from an old service can satisfy it. Add: if (teeCommitments[i].nonceBinding == bytes32(0)) revert Errors.InvalidNonceBinding().

🟠 MEDIUM AppRegistry.listApps returns revoked apps — callers must do a second lookup to filter — src/registry/AppRegistry.sol

Line 357 (revokeApp): sets record.revoked = true but does not remove appId from _namespaceApps[namespace]. listApps (line 368) returns _namespaceApps[namespace].values() which includes revoked apps. Any consumer iterating listApps results and treating them as active apps will silently include revoked entries. Fix: either remove the appId from _namespaceApps in revokeApp, or add a listActiveApps view that filters on revoked.

🟡 LOW Two error types defined but never used in changed code — src/libraries/Errors.sol

Lines 455 and 459: MeasurementMismatch and TeeCommitmentLengthMismatch are declared in the new TEE section but are never referenced by any contract in this PR. The MeasurementMismatch comment says 'Reserved for off-chain / blueprint use' which is fine for forward-compatibility, but TeeCommitmentLengthMismatch has no apparent caller at all. If these are genuinely needed by off-chain code (e.g. replica verification in a blueprint), that should be documented — otherwise they're dead code adding to the ABI/bytecode.

🟡 LOW AppRecord.verified is stale cached storage — getApp bypasses it anyway — src/registry/AppRegistry.sol

Lines 207-218 (publishApp) and 237 (pinAppVersion): record.verified = _namespaceVerified[namespace] writes the verified flag into AppRecord storage on every publish and pin. But getApp (line 302) returns verified = _namespaceVerified[namespace], reading directly from the source mapping and ignoring record.verified. The cached field is written on every publish/pin (~5k gas SSTORE), never read, and is stale between pins. Remove the record.verified field and its writes to save gas and eliminate the misleading cached state.

🟡 LOW appCount uses EnumerableSet.length() which iterates O(n) — src/registry/AppRegistry.sol

Line 375-376: appCount returns _namespaceApps[namespace].length() which iterates the entire EnumerableSet to count elements. If a namespace accumulates many apps, this becomes expensive. A separate counter mapping (incremented on publish, decremented on any future removal) would provide O(1). Not a blocker since there's no app removal path yet, but worth noting for future scale.

---

_{tangletools · 2026-05-04T18:20:59Z · trace}

[tangletools]

✅ Approved — 4 non-blocking findings

Read all 7 changed files plus the full ServicesApprovals.sol and TangleServicesFacet._activateService flow. The core logic is correct: TEE commitments are stored to _requestTeeCommitments before _approveServiceWithCommitmentsInternal triggers activation, which copies them into _serviceTeeCommitments via _persistTeeCommitments — the ordering is intentional and correct. Storage gap math (44→42 for 2 new mapping slots) is right. ABI selector keccak string matches the canonical tuple encoding. The one structural concern is that _storeRequestTeeCommitments performs SSTOREs before the operator-autho

🟠 MEDIUM Authorization check follows storage write — breaks validate-before-write — src/core/ServicesApprovals.sol

In approveServiceWithTeeCommitments (lines 208–214), _storeRequestTeeCommitments (which does SSTORE pushes) is called before _approveServiceWithCommitmentsInternal, which is where the operator-in-request-list check and AlreadyApproved guard live. An unauthorized or already-approved caller executes the storage loop before the transaction reverts; the writes are rolled back, so there is no persistent corruption, but the caller burns gas on the loop and the pattern is wrong. Fix: move _validateTeeCommitments + BLS check before the storage write, and restructure so authorization is confirmed before any state mutation.

🟡 LOW _requestTeeCommitments not reclaimed after activation or rejection — src/facets/tangle/TangleServicesFacet.sol

_persistTeeCommitments (line ~333–347) copies from _requestTeeCommitments to _serviceTeeCommitments but never deletes the source. After activation the request-scoped storage is never accessed again (comment in TangleStorage.sol confirms this) but the slots remain warm, costing refund gas that is never collected. The same pattern exists for other request-scoped maps (consistent), but note that deleting inside _persistTeeCommitments after the copy would reclaim ~4700 gas per non-zero slot. Low priority given consistency with the rest of the codebase.

🟡 LOW TeeCommitmentLengthMismatch error declared but never used — src/libraries/Errors.sol

TeeCommitmentLengthMismatch(uint256 commitmentCount, uint256 teeCommitmentCount) is declared at the bottom of the TEE section but is referenced nowhere in this PR (no call site in ServicesApprovals.sol or TangleServicesFacet.sol). If the per-operator commitment-count enforcement it was intended to guard was dropped, delete the error. If the enforcement is still planned, add the check.

🟡 LOW No test for non-TEE approval path producing empty getTeeCommitment result — test/tangle/TeeCommitmentApprovalTest.t.sol

All tests use approveServiceWithTeeCommitments. There is no test that verifies an operator who approved via approveService, approveServiceWithBls, or approveServiceWithCommitmentsAndBls results in getTeeCommitment returning an empty array after activation. This exercises the 'if (len == 0) continue' branch in _persistTeeCommitments for the non-TEE path and confirms the storage separation is correct.

---

_{tangletools · 2026-05-04T18:45:32Z · trace}

[tangletools]

❌ 1 Blocking Finding

The TEE commitment feature is architecturally clean — correct storage layout, proper ordering of TEE writes before the activation-triggering approval call, correct storage gap reduction (44→42 for two new mappings), and a well-structured test suite that covers the primary error paths. One high-severity issue undermines the rating: there is no upper bound on the teeCommitments calldata array, which lets a malicious operator in a multi-operator service submit a gas-exhausting array that permanently bricks service activation when the last legitimate operator triggers it. Two medium gaps round o

🔴 HIGH Unbounded teeCommitments array enables permanent activation DoS in multi-operator services — src/core/ServicesApprovals.sol

Lines 208–214 and 239–252: _validateTeeCommitments and _storeRequestTeeCommitments iterate over teeCommitments with no length cap. Each entry pushes to _requestTeeCommitments[requestId][operator] (cold SSTORE ~20K gas per slot; struct is 3 slots → ~60K gas/entry). These stored entries are later bulk-copied in _persistTeeCommitments (TangleServicesFacet.sol:336–347), which runs inside _activateService triggered by whoever submits the final approval. A malicious operator in a 2+ operator service can submit ~400 entries in their own approval tx (staying under block gas limit), then the final legitimate operator's approval will hit the gas limit dur

4 Non-Blocking

🟠 MEDIUM Zero expectedMeasurement silently accepted — src/core/ServicesApprovals.sol

Lines 220–236: _validateTeeCommitments rejects nonceBinding == bytes32(0) (line 228) with InvalidNonceBinding, but does not reject expectedMeasurement == bytes32(0). An operator submitting a zero measurement commits to a workload whose measurement is zero — practically impossible for any real hash function output, but semantically a commitment that can never be satisfied. An off-chain provisioning oracle comparing attestedMeasurement == expectedMeasurement against a zero value would either always fail (stalling the service)

🟠 MEDIUM No test for mixed TEE / non-TEE approvals in a multi-operator service — test/tangle/TeeCommitmentApprovalTest.t.sol

All multi-operator test cases (e.g. test_multipleOperators_distinctBackends_persistedPerOperator, line 185) send every operator through approveServiceWithTeeCommitments. The _persistTeeCommitments function in TangleServicesFacet.sol:336–347 has explicit logic to skip operators with no stored commitments (if (len == 0) continue;), but this branch is never exercised. The production scenario where operator 1 uses approveServiceWithTeeCommitments and operator 2 uses plain approveServiceWithCommitments is unverified; a bug in the skip logic or in how activation counts operators would not be caught.

🟡 LOW Duplicate TeeBackend entries silently accepted — src/core/ServicesApprovals.sol

Lines 220–236: _validateTeeCommitments does not check for duplicate backend values across entries. An operator can submit two entries with backend == TeeBackend.AwsNitro and different (or identical) measurements; both are stored and two TeeCommitmentRecorded events are emitted. The NatSpec says 'any one matching satisfies the policy,' which is consistent with duplicates being harmless at the protocol level, but they waste storage and inflate event logs. If the intent is per-backend commitment, add a seen-bitmap check during validation.

🟡 LOW TeeBackend enum default is Phala (0), not caught by validation — src/libraries/Types.sol

The TeeBackend enum starts at Phala=0, so a struct where backend is accidentally left unset defaults to Phala. The nonceBinding==0 check at src/core/ServicesApprovals.sol:228 catches the all-zero case, but if an integrator sets nonceBinding and expectedMeasurement while forgetting backend, it silently accepts Phala. Not exploitable in practice but a footgun. Consider starting the enum at 1 with a sentinel value like Unset.

---

_{tangletools · 2026-05-04T19:04:47Z · trace}

[drewstone]

Addressed all five findings from the latest review (commit bce6788):

🔴 HIGH — Unbounded teeCommitments DoS

Added MAX_TEE_COMMITMENTS_PER_OPERATOR = 8 constant and length check in _validateTeeCommitments. Above the cap reverts with Errors.TooManyTeeCommitments(supplied, max). 8 is well above any realistic number of acceptable backends an operator would commit to (5 backends in the enum minus DirectTdx, leaving 4 viable), and well below the gas-bricking threshold.

🟠 MEDIUM — Authorization check follows storage write

Hoisted the auth gates (request-not-rejected, operator-active, operator-in-request, not-already-approved) into a new _requireApprovingOperator(requestId) internal view, called BEFORE _validateTeeCommitments and BEFORE _storeRequestTeeCommitments. The internal _approveServiceWithCommitmentsInternal retains its own auth check (defensive consistency), so the cost is one extra read of req.rejected + the operator-list scan — paid only on the happy path.

🟠 MEDIUM — Zero expectedMeasurement silently accepted

Added Errors.InvalidExpectedMeasurement and reject in _validateTeeCommitments. Same treatment as the existing zero-nonceBinding check.

🟠 MEDIUM — No test for mixed TEE / non-TEE multi-operator service

Added test_mixedTeeAndNonTee_acrossOperators_persistsCorrectly: operator1 approves with approveServiceWithTeeCommitments, operator2 + operator3 use plain approveServiceWithCommitments. Service activates on the third approval. Asserts operator1's commitment array persists onto the service and operators 2/3 have empty arrays — exercising the if (len == 0) continue; skip branch in _persistTeeCommitments.

🟡 LOW — TeeBackend enum default is Phala (0)

Inserted Unset at index 0 of the enum + Errors.UnsetTeeBackend + reject in _validateTeeCommitments. An integrator who forgets to populate the backend field now hits UnsetTeeBackend instead of accidentally committing to whichever real backend was at index 0.

🟡 LOW — Duplicate TeeBackend entries (deferred)

Not addressed — the NatSpec on the entrypoint is explicit that "any one matching at provisioning time satisfies the policy", so duplicates are semantically harmless. With the per-operator cap of 8, the storage waste from a duplicate is bounded. If the off-chain provisioning oracle needs uniqueness, it can dedup before submitting; making it a contract-level invariant would muddy the documented semantics.

Tests

10 → 16 cases. New: test_approve_zeroExpectedMeasurement_reverts, test_approve_unsetBackend_reverts, test_approve_tooManyCommitments_reverts, test_approve_atCommitmentCap_succeeds, test_approve_unauthorizedCaller_revertsBeforeStorage, test_mixedTeeAndNonTee_acrossOperators_persistsCorrectly.

Force-pushed to keep the branch a single commit.

Superseded by re-review — no blocking findings on latest commit.

[tangletools]

✅ Approved — 4 non-blocking findings

Read all 7 changed files plus Base.sol/_getServiceRequest, TangleServicesFacet._activateService, and the full ServicesApprovals context. No critical or high bugs found. Storage gap math is correct (2 new mappings → gap reduced 44→42). Auth-first ordering is sound, DoS cap is justified, and all six validation rules in _validateTeeCommitments are semantically correct. Two medium/low findings: a double-auth-check in the new entrypoint wastes ~8 K gas on the happy path, and _persistTeeCommitments never deletes _requestTeeCommitments source slots after copying (forfeits storage refund). One enum ba

🟠 MEDIUM Double auth check in approveServiceWithTeeCommitments wastes gas — src/core/ServicesApprovals.sol

approveServiceWithTeeCommitments (line 208) calls _requireApprovingOperator(requestId) which reads four storage slots (req.rejected, isOperatorActive, _requestOperators[requestId], _requestApprovals[requestId][msg.sender]), then calls _approveServiceWithCommitmentsInternal (line 218) which reads the same four slots again (lines 309-325). The only state change between the two reads is the TEE commitment write

🟡 LOW TEE section in TangleStorage.sol lacks slot-range comment — src/TangleStorage.sol

Every other storage section in TangleStorage.sol carries a slot-range annotation (e.g. '// PRICING STORAGE (Slot 136-140)'). The new TEE ATTESTATION COMMITMENTS section (lines 372-383) has no such annotation, making it harder to track the cumulative slot count when adding future storage. Add the slot range to match the surrounding convention.

🟡 LOW _persistTeeCommitments does not delete source slots — no storage refund — src/facets/tangle/TangleServicesFacet.sol

_persistTeeCommitments (line 334) copies each TeeAttestationCommitment element from _requestTeeCommitments[requestId][op] into _serviceTeeCommitments[serviceId][op] but never calls delete or pop on the source array. Each struct occupies 4 storage slots (backend/uint8, expectedMeasurement/bytes32, nonceBinding/bytes32, expiresAt/uint64). With up to 8 elements per operator and N operators, the request-scoped data (up to 32×N slots) is permanently wasted. This is consistent with the existing _persistServiceSecurity pattern, but with TEE commitments there are potentially more slots per operator. Consider deleting the source array after copy or documenting t

🟡 LOW AzureSkr backend has zero test coverage — test/tangle/TeeCommitmentApprovalTest.t.sol

Types.TeeBackend.AzureSkr (enum value 4) is defined in Types.sol (line ~305) and is accepted by _validateTeeCommitments (not Unset, not DirectTdx), but no test in TeeCommitmentApprovalTest exercises it. Phala, AwsNitro, GcpConfidential, and DirectTdx are all covered. Add a test mirroring test_approve_withNitroCommitment_persistsOnActivation but using TeeBackend.AzureSkr to confirm it round-trips correctly through storage.

---

_{tangletools · 2026-05-04T19:53:32Z · trace}

[drewstone]

Adversarial sweep on top of the review-fix commit (57b333d). Two REAL findings remediated, one documented with measurement.

T1 [HIGH] — nonceBinding was operator-supplied, not bound to anything on-chain → REPLAY POSSIBLE

Before this push: validation only checked nonceBinding != bytes32(0). An operator could submit nonceBinding = N for request A, produce one attestation document with nonce = N, then resubmit nonceBinding = N for request B — same attestation satisfies both.

Why it slipped past prior review: the NatSpec said "binds this commitment to a specific challenge so a stale attestation cannot be replayed against a fresh request." But the contract enforced none of that — the binding semantics were entirely off-chain.

Fix. nonceBinding MUST equal keccak256(abi.encode("tangle.tee.nonce", requestId, address(this), block.chainid)). New public view teeNonceFor(uint64) → bytes32 exposes the canonical value. Cross-request replay becomes structurally impossible — an attestation document binding to nonce of request A literally cannot satisfy a commitment requiring nonce of request B.

T4 [LOW-MEDIUM] — expiresAt had no upper bound → silent never-expiry

Before: expiresAt = type(uint64).max was accepted (~year 584,554,051,223). A commitment with that expiry was effectively never-expiring, defeating the "force re-attestation" intent.

Fix. MAX_TEE_COMMITMENT_TTL = 90 days. Validation rejects expiresAt > block.timestamp + MAX_TEE_COMMITMENT_TTL with Errors.TeeCommitmentExpiryTooFar(supplied, maxAllowed). expiresAt = 0 (the documented no-expiry sentinel) remains intentional.

T2 [HIGH/system-design] — Activation gas measured, not "fixed"

_persistTeeCommitments runs in the activator's tx and copies every operator's commitment array onto the service. Linear in operator_count × commitments_per_operator. With 3 operators × 8 commits, 2,886,434 gas for TEE persistence alone. At 10 operators × 8 → ~9.6M. At 20 × 8 → ~19M, near block-gas territory.

This is a system-design bound, not a contract bug. Mitigations span beyond the TEE feature (operator-count cap on TEE-required services, separate operator-paid claim step, or moving persistence to event-log lookup). New test test_T2_activation_gasScalesWithOperatorsTimesCap emits the number to forge log so any regression is visible.

Adversarial coverage added

test/tangle/TeeCommitmentHardenTest.t.sol — 7 new tests, each with a clear PASS/FAIL semantic:

test_T1_replayAcrossRequests_rejected           — nonce A submitted to request B reverts
test_T1_arbitraryNonce_rejected                  — fabricated nonce reverts
test_T1_teeNonceFor_uniquePerRequest             — distinct/stable across requests
test_T2_activation_gasScalesWithOperatorsTimesCap — emits 2.89M gas measurement
test_T4_uint64Max_rejected                        — far-future reverts
test_T4_oneDayPastCap_rejected                    — 91-day reverts
test_T4_exactlyAtCap_accepted_oneSecondPast_rejected — boundary test

Final test surface

• 16 happy-path / review-fix tests in TeeCommitmentApprovalTest.t.sol — all green
• 7 adversarial / hardening tests in TeeCommitmentHardenTest.t.sol — all green
• 23/23 forge tests pass locally (forge test --match-contract "TeeCommitment(Approval|Harden)")

Changes added to the existing single commit; force-pushed.

[drewstone]

CI status — pre-existing OOM, not caused by this PR

Run 25340146909 failed at the "Run Forge build" step with exit code 143 (SIGTERM, OOM-kill) after 17m17s of compilation. The job was killed mid-compile of 342 Solidity files; no test ever ran.

This is the same failure mode as:

• Run 25336884245 — same branch's prior commit, exit 143 at the build step
• Run 25332144175 — main, immediately after PR fix(security): timelock storage slot, drip mutex, inflation pool deregister #116 merge, exit 143
• Run 25330677940 — main, immediately after PR fix(security): pre-mainnet hardening across deploy, MBSM, beacon, oracles, quotes, BLS #115 merge, exit 143

Cause: via_ir = true on [profile.default] in foundry.toml (inherited by profile.ci). The IR pipeline is memory-heavy on a 342-file codebase; the github-hosted ubuntu-latest runner (16 GB RAM) doesn't fit it. Both #115 and #116 landed despite this failing CI.

Local reproduction is green:

forge test --match-contract "TeeCommitment(Approval|Harden)" -vv
[PASS]  16 happy/review-fix tests       — TeeCommitmentApprovalTest.t.sol
[PASS]   7 adversarial/hardening tests   — TeeCommitmentHardenTest.t.sol
Suite result: ok. 23 passed; 0 failed; 0 skipped

This PR does not change foundry.toml, the CI workflow, or anything that would alter compile-step memory usage. The CI fix belongs in a separate PR scoped to the build infra (options: switch profile.ci to via_ir = false, drop --sizes from the build, or move to a larger runner). Happy to open that as a follow-up if you want, but not bundling it here.

The TEE commitment code itself + the harden fixes are verified locally and via PR review.

[tangletools]

✅ Approved — 4 non-blocking findings

Read every changed file. The TEE commitment feature is well-structured: auth-before-writes ordering is correct, nonReentrant guard is in place, storage gap is correctly reduced by 2 for 2 new mappings, nonce replay prevention is sound, gas-griefing cap is justified and documented, and the request→service migration ordering comment is accurate. One documentation bug in Errors.sol will cause off-chain integrators to derive the wrong nonce and hit silent reverts; everything else is nits or consistent with existing codebase patterns. | Read all 8 changed files in full, verified all 23 tests pass

🟠 MEDIUM Errors.InvalidNonceBinding comment has wrong nonce formula — integrators will always compute the wrong value — src/libraries/Errors.sol

Line 455: the NatSpec states nonceBinding MUST equal keccak256(abi.encode(requestId, address(this), block.chainid)). The actual implementation in ServicesApprovals.teeNonceFor (line 315) is keccak256(abi.encode("tangle.tee.nonce", requestId, address(this), block.chainid)). The domain separator "tangle.tee.nonce" is omitted from the comment. Any operator SDK or off-chain tool that follows this comment verbatim will produce a different 32-byte value and receive InvalidNonceBinding on every submission — silently, with no indication that the fo

🟡 LOW Double O(n) operator-list scan per approveServiceWithTeeCommitments call — src/core/ServicesApprovals.sol

Lines 208 and 349: _requireApprovingOperator iterates _requestOperators[requestId] to confirm msg.sender is in the list, then _approveServiceWithCommitmentsInternal (called at line 218) repeats the identical loop at line 349. Under the nonReentrant guard no state can change between the two calls, so the second scan is pure waste — linear in operator count per approval. Existing entrypoints (`approveServ

🟡 LOW _requestTeeCommitments never deleted after activation — orphaned storage slots — src/facets/tangle/TangleServicesFacet.sol

Lines 337-348: _persistTeeCommitments copies commitment arrays from _requestTeeCommitments[requestId][op] to _serviceTeeCommitments[serviceId][op] but never deletes the source. At the documented ~60K gas per entry and up to 8 entries × N operators, the orphaned slots are non-trivial. This is consistent with _persistResourceCommitments (line 294-311) which also leaves _requestResourceRequirements intact, so it matches the existing design choice — but both forfeit SSTORE refunds. Worth a one-line `d

🟡 LOW Errors.InvalidNonceBinding @dev comment omits domain prefix — src/libraries/Errors.sol

Line 454-456: The @dev comment says nonceBinding MUST equal keccak256(abi.encode(requestId, address(this), block.chainid)) but the actual implementation in ServicesApprovals.sol:314-316 uses keccak256(abi.encode("tangle.tee.nonce", requestId, address(this), block.chainid)). An integrator reading the error doc to compute the canonical nonce would derive the wrong value and get rejected. Fix: add the 'tangle.tee.nonce' prefix to the @dev comment.

---

_{tangletools · 2026-05-04T20:12:34Z · trace}