Category: demonstrated defect (conditional on the collection ABI). settle(token) and claim(token) take any address and treat balanceOf(vault) as a fungible balance. Passing the pinned seat collection (IERC20(address(collection))) settles the vault's NFT count (1) as a reward: owner share floor(1*7000/10000)=0, provider share 1. The provider's claim then executes collection.transfer(provider, 1) through SafeERC20.

On an ERC-721 that implements the pre-standard transfer(address,uint256) (CryptoKitties-era ABI, still shipped by some collections) that call moves token id 1, and _pushExact accepts it because both balanceOf deltas equal 1. The core guarantee 'the host can never move the NFT' is thereby broken whenever tokenId == the claimable count (1, or 2 when an agent NFT of the same collection sits in the vault).

Verified against the pinned IMD collection 0x0000ec93127baa929e58e97dd0095a2bfb38ec1d: its Sourcify-verified ABI (compilation IdentityMD, not a proxy) has no transfer(address,uint256) and no fallback, so the call reverts there and the seat cannot be moved on mainnet; the identity registry is an upgradeable proxy whose future ABI is not pinned. rescueERC721(other=rewardToken, ...) has the mirror gap: only (collection, tokenId) is rejected.

Impact: conditional loss of the seat to the provider; on the pinned collection only spurious ledger entries (accounted[collection]=1).

Suggested fix: in _settle (or settle/claim) revert with UnsupportedToken when address(token) == address(collection) || address(token) == identityRegistry, and in rescueERC721 revert when address(other) == address(rewardToken); optionally restrict settle/claim to rewardToken plus an owner-designated allowlist.

Category: demonstrated defect in the pairing helper's validation (production prerequisite for live use). The pairing-response schema is unverified (assumption 2). If IMD returns expiresAt as a JavaScript-style millisecond number (Date.now()+300000), parseExpiry multiplies it by 1000 again and produces a date ~50,000 years out, so validatePairing never reports 'pairing code has expired' and expiryProblems in complete never reports 'IMD's pairing code expired'.

A 13-digit millisecond string is rejected as 'not a timestamp' (Date.parse returns NaN), so the string path fails closed while the number path fails open.

Impact: the 'two clocks' guard the round-two fix introduced is bypassed; the owner may mine an approvePairing transaction (gas spent, live approval left on-chain for up to an hour) for a code IMD will refuse. No key or fund risk.

Suggested fix: treat numbers >= 1e11 (or 13-digit strings) as milliseconds, or reject them explicitly; add the two cases to the self-test and to test/pair-vault.test.mjs.

Category: design choice (owner-only, self-affecting). Any 4+ byte calldata goes to the pinned registry with the vault as msg.sender, and AgentRegistered(data) is emitted even for non-registration calls.

Because the registry is an ERC-721 (agent NFTs), the owner can be talked into sending setApprovalForAll, approve or transferFrom calldata that IMD's register-intent endpoint proposes, handing the agent identity to a third party (tracked witness test_registryCalldataCanAuthorizeAnAgentNFTTransfer). It cannot touch the seat or the reward token: the vault never approves the registry on either, and the call is CALL (not DELEGATECALL) with zero value and a reentrancy guard.

Answer to brief item 4: an allowlist of the three register selectors (register(), register(string), register(string,(string,bytes)[])) is better despite the proxy. The proxy argument cuts the other way: if the implementation's ABI changes, the pinned EIP-712 domain and the factory would need a new version anyway, and an allowlist fails closed (revert) rather than open. Rescue of the agent NFT stays available through rescueERC721.

Also note the registry may later require a registration fee in ETH; the vault has no payable path, which is a production prerequisite to confirm.

Category: test adequacy. foundry.toml configures [invariant] (64 runs, depth 64) but no invariant test exists in contracts/test.

Untested edges found during this review: (1) settle/claim with token == collection or token == identityRegistry (finding 1); (2) provider end() before deposit and front-running deposit() (finding 2); (3) withdrawNFT(address(0)) and rescueERC721(..., address(0)) — the vault has no zero check and relies on the collection (OpenZeppelin v5 and the pinned Solady-style collection both revert); (4) approvePairing at exactly block.timestamp + 1 hours (accepted; only +1 is tested); (5) fuzzing isValidSignature over random hashes/signature bytes; (6) deposit() executed by an approved operator on the owner's behalf (from == owner path); (7) the pairing helper with millisecond timestamps (finding 3).

A scratch conservation invariant (accounted[t] == claimable[t][owner] + claimable[t][provider] and balanceOf(vault) >= accounted[t] for an honest token, handler: mint/settle/claim/claim/end) held for 64 runs x 64 depth (4096 calls, 0 reverts) and is worth adopting as a tracked invariant.

Suggested fix: add the invariant file and the seven unit cases above.

Category: production prerequisite (testnet rehearsal only; statically reviewed, not executed). send hands the key to cast mktx --private-key "$who" and cast wallet address --private-key, so the key is visible in the process table of the machine for the duration of each call; key()/addr() invoke python (not python3), which is absent on many systems, and default KEYS=$HOME/.ssh mixes wallet JSON with SSH material.

The at-most-once logic itself is sound as reviewed: one cast mktx per call, same-bytes rebroadcast only, receipt required for the locally computed hash with a block number and status 1, no rebuild on 'nonce too low'/'already known'/'underpriced' or on a receipt for another hash, nonce read once per key and advanced locally, and set -e stops the run on the first failure so a restart needs manual reconciliation (documented).

One diagnostic nit: 'transaction underpriced' (never accepted by the node) is treated like 'already known', so the script waits 45 s and prints 'check that hash by hand' for a transaction that was never in a mempool.

Suggested fix: use cast wallet import/keystore or --keystore with a password file instead of --private-key arguments, call python3, default KEYS to a dedicated directory, and distinguish 'replacement transaction underpriced' from 'transaction underpriced' in the retry condition.