Skip to content

Commit 89e5918

Browse files
committed
Update doc
1 parent 47a215b commit 89e5918

15 files changed

Lines changed: 2064 additions & 1934 deletions

AGENTS.md

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ Operation rules that treat `msg.sender` or `getTokenBound()` as a *token identit
2626
`CMTAT._mintOverride` calls `_checkTransferred(_msgSender(), address(0), to, value)`, so **on every mint the minter's address arrives at each rule as `spender`** via the 4-arg `transferred` overload. Plain `transfer()` passes `spender == address(0)` and takes the 3-arg path.
2727

2828
- `RuleWhitelist`, `RuleSpenderWhitelist`, `RuleWhitelistWrapper` explicitly exempt mint/burn from the spender check.
29-
- `RuleIdentityRegistry`, `RuleBlacklist`, `RuleSanctionsList`, `RuleERC2980` do **not** — they screen the minter. For the deny-lists this is intended; for `RuleIdentityRegistry` it means the minter must itself be identity-verified (see `RESULT.md` F-1).
29+
- `RuleIdentityRegistry`, `RuleBlacklist`, `RuleSanctionsList`, `RuleERC2980` do **not** — they screen the minter. For the deny-lists this is intended; for `RuleIdentityRegistry` it means the minter must itself be identity-verified (see `CLAUDE_AUDIT.md` F-1).
3030
- `RuleMintAllowance` is the only rule that *uses* the mint spender: it debits `mintAllowance[spender]`.
3131
- `RuleMaxTotalSupply` and `RuleChainlinkPoR` ignore the spender entirely — they cap *supply*, not identities, and act only when `from == address(0)`.
3232

@@ -76,7 +76,7 @@ Rules that implement a standardized interface must match that standard's semanti
7676
| `RuleERC2980Ownable2Step` | Ownable2Step variant of RuleERC2980 |
7777
| `RuleConditionalTransferLight` | Require operator approval before each transfer; bound to exactly one token at a time (`bindToken` reverts if a token is already bound; use `unbindToken` first to migrate) |
7878
| `RuleConditionalTransferLightOwnable2Step` | Owner-only approval and execution for conditional transfers |
79-
| `RuleConditionalTransferLightMultiToken` / `…Ownable2Step` | Conditional transfers with approvals keyed `(token, from, to, value)`. **Direct-binding-only (Topology B)** — approvals are *consumed* under `msg.sender`, so this rule must NOT be added to a RuleEngine; behind an engine it either reverts or loses all per-token isolation. See `RESULT.md` F-4 and `doc/technical/RuleConditionalTransferLightMultiToken.md` |
79+
| `RuleConditionalTransferLightMultiToken` / `…Ownable2Step` | Conditional transfers with approvals keyed `(token, from, to, value)`. **Direct-binding-only (Topology B)** — approvals are *consumed* under `msg.sender`, so this rule must NOT be added to a RuleEngine; behind an engine it either reverts or loses all per-token isolation. See `CLAUDE_AUDIT.md` F-4 and `doc/technical/RuleConditionalTransferLightMultiToken.md` |
8080
| `RuleMintAllowance` / `RuleMintAllowanceOwnable2Step` | Per-minter mint quota, debited on the 4-arg `transferred(spender, from=0, to, value)` path. Requires CMTAT ≥ v3.3. `canTransfer` is **not** authoritative for this rule — use `canTransferFrom(minter, address(0), to, value)` |
8181
| `AccessControlModuleStandalone` | Base RBAC module; admin implicitly holds all roles |
8282
| `MetaTxModuleStandalone` | ERC-2771 meta-transaction support. Note: the operation rules deliberately do **not** inherit this, so `_msgSender()` used as a binding identity is never forwarder-controlled |
@@ -154,14 +154,12 @@ sees stay context-free. Hardhat compiles only `src/` (Foundry's `src`), so it ne
154154
- Use `require(condition, CustomError(...))` for custom errors; avoid direct `revert CustomError(...)`.
155155
- **No emoji in code comments or NatSpec.** Use a plain word marker instead: `WARNING:`, `NOTE:`, `IMPORTANT:`. Emoji render inconsistently across editors, terminals, `forge doc` output and diffs; they are not searchable (`grep WARNING` finds the marker, `grep ⚠️` depends on the shell); and they encode as multi-byte sequences that can be silently mangled by tooling. This applies to `src/`, `test/` and `script/`. Markdown documentation may use emoji freely — the restriction is Solidity comments only.
156156
- `AGENTS.md` and `CLAUDE.md` are identical — always update both together.
157-
- Always update README.md with the latest change
158-
- New rule or features implemented: create/update technical documentation in `doc/technical`, update README, create/update test (target: 100% of code coverage), update CHANGELOG.md. Code coverage, run `forge coverage --report summary`
157+
- **Two READMEs.** `README.md` at the root is a short summary (purpose, architecture, rule list, quick start) and is the GitHub front page; `doc/README.md` is the full reference. Update `doc/README.md` with the latest change, and the root `README.md` only when the summary itself becomes wrong (a new rule, a changed code range, a moved document). Links inside `doc/README.md` are relative to `doc/`.
158+
- New rule or features implemented: create/update technical documentation in `doc/technical`, update `doc/README.md` (and the root summary if the rule table or code ranges change), create/update test (target: 100% of code coverage), update CHANGELOG.md. Code coverage, run `forge coverage --report summary`
159159
- After each implemented feature or fix, provide a one-line GitHub commit message for all changes since the last commit.
160160

161161
## Security Findings Reference
162-
- [`THREAT_MODEL.md`](THREAT_MODEL.md) — trust model, 30 catalogued threats with IDs, data-flow diagrams, 12 invariants.
163-
- [`RESULT.md`](RESULT.md) — findings (0 High/Medium, 2 Low, 8 Info), invariant and access-control verification, disposition of every threat ID.
164-
- [`TEST_IMPROVEMENT.md`](TEST_IMPROVEMENT.md) — test-gap analysis and the deferred test backlog.
162+
- [`doc/security/audits/tools/v0.4.0/claude-audit/CLAUDE_AUDIT.md`](doc/security/audits/tools/v0.4.0/claude-audit/CLAUDE_AUDIT.md) — the v0.4.0 security audit: trust model, catalogued threats and invariants, findings (0 High/Medium, 2 Low, 8 Info), and the disposition of every threat ID. Source comments cite it by bare filename, `CLAUDE_AUDIT.md`.
165163
- [`doc/security/audits/tools/v0.5.0/CLAUDE_ANALYSIS.md`](doc/security/audits/tools/v0.5.0/CLAUDE_ANALYSIS.md) — code-quality review (duplication, missing events, gas, `virtual` convention, behaviour at odds with the library's purpose). 28 findings with the disposition and commit for each, including two whose gas claims were wrong and one whose proposed remedy did not work. Source comments cite it by bare filename, `CLAUDE_ANALYSIS.md`, so the path can move.
166164
- [`doc/security/audits/tools/v0.5.0/CLAUDE_ANALYSIS_SCRIPT.md`](doc/security/audits/tools/v0.5.0/CLAUDE_ANALYSIS_SCRIPT.md) — deployment-script review (`script/`). 12 findings, all implemented, including three scripts that reverted under `forge script` and a test-methodology gap that hid it. Source comments cite it by bare filename, `CLAUDE_ANALYSIS_SCRIPT.md`, so the path can move.
167165
- [`test/ThreatModel/ThreatModelTests.t.sol`](test/ThreatModel/ThreatModelTests.t.sol) — 18 PoCs. Tests suffixed `_CurrentBehaviour` assert behaviour the audit considers wrong; **fixing the underlying issue must make them fail**, at which point update the test and the finding together.

CHANGELOG.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ screening, F-1; `transferFrom` delegation, F-2).
154154
- Only the loops moved. Single-address `add` / `remove` / `contains` / `length` stay as one-line delegations to `EnumerableSet`, where a library would add indirection without removing duplication.
155155
- **Each rule keeps its own zero-address error.** `RuleAddressSet_ZeroAddressNotAllowed` and `RuleERC2980_ZeroAddressNotAllowed` are distinct per the one-error-namespace-per-rule convention, which a shared loop cannot name. The guard is therefore passed to `addBatch` as an `internal pure` function pointer: a required parameter, so it cannot be forgotten, while the revert data each rule produces is unchanged.
156156
- Costs ~34 gas per entry on a batch add and ~4 on a batch remove, from the indirect jump — 0.07% of a 20-address batch, which is dominated by cold `SSTORE`s, and an operator path rather than a per-transfer holder path. `RuleERC2980`'s runtime bytecode shrinks 200 bytes; `RuleWhitelist`'s grows 62.
157-
- **`RuleWhitelistWrapper`: the child-rule scan's early exit is now O(1) instead of a full rescan** (`CLAUDE_ANALYSIS.md` A-2). `_detectTransferRestrictionForTargets` used to re-derive "have all targets been resolved?" by walking the whole `result` array after every child rule; it now maintains a counter of unresolved targets and breaks when it reaches zero. Behaviour is identical — including the documented consequence that a pair already resolved by an earlier child never reaches a later, broken child (`RESULT.md` WW-2). Saves ~85 gas per child scanned (~1% of the ~8.8k per-child cost, which is dominated by the external `STATICCALL`); ~850 gas on a rejected transfer through a 10-child wrapper, paid by the transferring user.
157+
- **`RuleWhitelistWrapper`: the child-rule scan's early exit is now O(1) instead of a full rescan** (`CLAUDE_ANALYSIS.md` A-2). `_detectTransferRestrictionForTargets` used to re-derive "have all targets been resolved?" by walking the whole `result` array after every child rule; it now maintains a counter of unresolved targets and breaks when it reaches zero. Behaviour is identical — including the documented consequence that a pair already resolved by an earlier child never reaches a later, broken child (documented wrapper behaviour WW-2). Saves ~85 gas per child scanned (~1% of the ~8.8k per-child cost, which is dominated by the external `STATICCALL`); ~850 gas on a rejected transfer through a 10-child wrapper, paid by the transferring user.
158158
- The resolved-counter form needs a `!result[j]` guard so an address listed in several children is counted once. Without it the counter would reach zero early and break out of the scan before a later child could resolve a *different* target, rejecting a valid transfer. Pinned by `testDetectTransferRestrictionOkWhenAddressListedInSeveralChildRules`, which covers both the 2-target and the `checkSpender` 3-target paths.
159159

160160
- **Conditional-transfer rules: the approval counter is no longer re-read from storage to populate `TransferApproved`** (`CLAUDE_ANALYSIS.md` B-1). `approveTransfer` (`RuleConditionalTransferLightApprovalBase`) and `_approveTransfer` (`RuleConditionalTransferLightMultiTokenBase`) computed `approvalCounts[hash] += 1` and then read the slot back for the event; they now keep the new value in a local. Event data is byte-identical. Saves ~109 gas per approval, measured with the variants isolated in single-function contracts so dispatch cost is held constant.
@@ -204,14 +204,14 @@ screening, F-1; `transferFrom` delegation, F-2).
204204
- New [`doc/technical/RuleChainlinkPoR.md`](./doc/technical/RuleChainlinkPoR.md), including a point-by-point comparison against Chainlink's `SecureMintPolicy 1.2.0` (vendored at `lib/chainlink-ace/`) and a *Token compatibility: ERC-20 only* section; `RULE_SEMANTICS.md` and `README.md` updated with the new rule.
205205
- **README ERC-721/ERC-1155 section corrected.** It listed only `RuleConditionalTransferLight` and `RuleMaxTotalSupply` as ERC-20 only, and described `IERC7943NonFungibleCompliance*` as "implemented by validation rules only" — a category claim that `RuleChainlinkPoR`, itself a validation rule, does not satisfy, so the README implied the opposite of the truth for the new rule. The exclusions are now named individually and the missing `totalSupply()` requirement is stated. The `ITransferContext` paragraph had the same defect and claimed `RuleMaxTotalSupply` exposes the fungible variant, which it does not — it exposes neither, as do `RuleChainlinkPoR` and `RuleMintAllowance`. `RULE_SEMANTICS.md` already had the correct matrix row for all of them; the README sections now link to it.
206206

207-
- **`RuleMintAllowance`'s non-authoritative views are documented one level up** (`CLAUDE_ANALYSIS.md` F-6). `canTransfer` / `detectTransferRestriction` are hardcoded to "allowed" because the 3-argument signature carries no minter identity — already documented for the rule itself (`RESULT.md` F-7, resolution I-8). What was missing is that the answer **propagates**: `RuleEngineBase` aggregates by calling each rule's 3-argument view, and CMTAT's `ValidationModuleERC1404` forwards the token's ERC-1404 views to the engine, so `ruleEngine.canTransfer` and `cmtat.canTransfer` report a mint as allowed that then reverts. The token is the address integrators actually call, so the audience most likely to be misled was the one furthest from the existing warnings.
207+
- **`RuleMintAllowance`'s non-authoritative views are documented one level up** (`CLAUDE_ANALYSIS.md` F-6). `canTransfer` / `detectTransferRestriction` are hardcoded to "allowed" because the 3-argument signature carries no minter identity — already documented for the rule itself (`CLAUDE_AUDIT.md` F-7). What was missing is that the answer **propagates**: `RuleEngineBase` aggregates by calling each rule's 3-argument view, and CMTAT's `ValidationModuleERC1404` forwards the token's ERC-1404 views to the engine, so `ruleEngine.canTransfer` and `cmtat.canTransfer` report a mint as allowed that then reverts. The token is the address integrators actually call, so the audience most likely to be misled was the one furthest from the existing warnings.
208208
- `doc/technical/RuleMintAllowance.md` gains a per-entrypoint table covering the engine and token levels, and a callout naming `cmtat.detectTransferRestrictionFrom(minter, address(0), to, value)` as the authoritative pre-flight for anyone holding only the token address.
209209
- `README.md` gains a *Views that are not authoritative* section covering `RuleMintAllowance` and `RuleConditionalTransferLightMultiToken` together, with the propagation mechanism and why returning a restriction code instead would be worse — ERC-1404 has no "cannot answer" value, so the token would report every mint as forbidden, including those that will succeed.
210210
- **Behaviour unchanged; no Solidity modified.**
211211
- **The approval-key preimage is documented, and the comment that described it wrongly is fixed** (`CLAUDE_ANALYSIS.md` F-4). `_transferHash` hashes a project-specific encoding — 32-byte words with each address **left**-aligned and right-padded — which is neither `abi.encodePacked` (72 bytes, unpadded) nor `abi.encode` (96 bytes, right-aligned). The inline comment said "hash packed values", pointing anyone reimplementing the key off-chain at the wrong encoding; because the result is a mapping key the mistake is **silent**, reading `0` and looking exactly like "no approval exists".
212212
- NatSpec on both rules now gives the word-by-word layout, the warning, and two formulations that reproduce the key: `keccak256(abi.encodePacked(from, bytes12(0), to, bytes12(0), value))` and `keccak256(abi.encode(bytes32(bytes20(from)), bytes32(bytes20(to)), value))`. The multi-token variant is the same shape with `token` prepended (128 bytes).
213213
- It also points readers at `approvedCount`, which resolves `(from, to, value)` directly — the hash is only needed to derive the storage slot for `eth_getStorageAt`, a state proof, or an indexer reading storage rather than events.
214-
- **The assembly is unchanged.** It is on the transfer write path and ~109 gas cheaper per call than `abi.encodePacked` (1,032 vs 1,141, measured with each variant in its own single-function contract), its injectivity is verified in `RESULT.md` F-12, and switching encodings would change every storage key — orphaning outstanding approvals in any deployed instance.
214+
- **The assembly is unchanged.** It is on the transfer write path and ~109 gas cheaper per call than `abi.encodePacked` (1,032 vs 1,141, measured with each variant in its own single-function contract), its injectivity is verified in `CLAUDE_AUDIT.md` F-12, and switching encodings would change every storage key — orphaning outstanding approvals in any deployed instance.
215215
- **The batch-operation convention is documented accurately** (`CLAUDE_ANALYSIS.md` F-5). Every add path in the library rejects `address(0)`, batch included, but the documentation said the opposite in three places: `CLAUDE.md` / `AGENTS.md` invariant I-12 claimed "single adds revert, batch adds skip it"; the Conventions list claimed "batch add/remove operations are non-reverting" without qualification; and `README.md` repeated that in the ERC-2980 section — while contradicting itself in the static-analysis triage table, which already recorded that batch adds revert on the sentinel on purpose. The NatSpec on all six batch-add functions documented only the duplicate-skipping half. All corrected; **no Solidity behaviour changed** — the code is deliberate and its inline reasoning was already right.
216216
- New `README.md` section *Zero address in batch operations*: a single-vs-batch behaviour table, why the sentinel is rejected rather than skipped (the batch event echoes the input array, so skipping would name a non-member as a member), and the operational consequence — a batch containing one zero entry is rejected whole rather than partially applied.
217217
- `CLAUDE.md` / `AGENTS.md`: the toolchain section now records **where** the ONCHAINID remapping is declared and why it must not go back into `remappings.txt`.
@@ -321,7 +321,7 @@ Two new rule families, two standards-conformance fixes, and hardening from an in
321321
- Add `doc/technical/INVARIANT_TESTS.md` — documents the stateful invariant suite: handler architecture and ghost variables, each of the four invariants and what it proves, the mutation-testing negative controls, the coverage map against the threat-model invariants, and how to add a new invariant. Linked from a new "Invariant testing" section in the README.
322322
- Add `doc/technical/RULE_SEMANTICS.md` — a per-rule comparison table (who each rule screens for `from` / `to` / spender on `transferFrom` / mint / burn, behaviour when the oracle/registry is unset, stateful?, and which pre-flight view is authoritative), with a highlights summary and link added to the README.
323323
- `RuleMintAllowance`: document that `canTransfer` / `detectTransferRestriction` are **not authoritative** (hardcoded to "allowed" because the 3-arg path has no minter identity) and that a mint pre-flight must use the spender-aware `canTransferFrom(minter, address(0), to, value)` / `detectTransferRestrictionFrom`. Added a bold callout and an eligibility-views table to `doc/technical/RuleMintAllowance.md` and a warning to the README rule section.
324-
- Add [`CLAUDE_AUDIT.md`](./doc/security/audits/tools/v0.4.0/claude-audit/CLAUDE_AUDIT.md) — the published AI-assisted security audit report for `v0.4.0` (0 Critical/High/Medium, 2 Low, 8 Info), with invariant verification, access-control verification, the remediation record and the open improvement backlog. Backed by the working deliverables `THREAT_MODEL.md`, `RESULT.md` and `TEST_IMPROVEMENT.md`, plus Slither call-graph / inheritance / function-summary comprehension artifacts.
324+
- Add [`CLAUDE_AUDIT.md`](./doc/security/audits/tools/v0.4.0/claude-audit/CLAUDE_AUDIT.md) — the published AI-assisted security audit report for `v0.4.0` (0 Critical/High/Medium, 2 Low, 8 Info), with invariant verification, access-control verification, the remediation record and the open improvement backlog. Backed by internal working deliverables (threat model, findings, test-gap analysis) plus Slither call-graph / inheritance / function-summary comprehension artifacts.
325325
- Add a "Manual Threat Model & Review" section to `README.md`.
326326
- `CLAUDE.md` / `AGENTS.md`: correct the version string to `0.4.0`, document the two integration topologies and the CMTAT v3.3+ mint `spender` convention, and add the missing `RuleMintAllowance`, `RuleConditionalTransferLightMultiToken`, `RuleNFTAdapter` and restriction code `70` entries.
327327
- Added technical documentation: `doc/technical/RuleConditionalTransferLightMultiToken.md`.

0 commit comments

Comments
 (0)