Merge pull request #68 from CMTA/dev

v3.3.0-rc5

Author: rya-sge

.gitignore

@@ -6,6 +6,9 @@ lib/
 out/
 docOut/
 cache/
+#coverage scratch output (the published report lives in doc/coverage/)
+lcov.info
+/coverage/
 .~lock.test.odt#
 nethereum-gen.settings
 #hardhat

.gitmodules

@@ -13,3 +13,6 @@
 [submodule "lib/CMTATv3.0.0"]
 	path = lib/CMTATv3.0.0
 	url = https://github.com/CMTA/CMTAT
+[submodule "ERC-3643"]
+	path = lib/ERC-3643
+	url = https://github.com/ERC-3643/ERC-3643

AGENTS.md

@@ -2,14 +2,14 @@
 
 This file helps AI agents (Cursor, Claude Code, etc.) understand and work with this codebase.
 
-AGENTS.md and CLAUDE.md files must always be identical
+AGENTS.md and CLAUDE.md files must always be identical — always update both together.
 
 ## Project Summary
 
 **RuleEngine** is a Solidity smart contract system that enforces transfer restrictions for [CMTAT](https://github.com/CMTA/CMTAT) and [ERC-3643](https://eips.ethereum.org/EIPS/eip-3643) tokens. It acts as an external controller that calls pluggable rule contracts on each token transfer, mint, or burn.
 
 - **Version:** 3.0.0 (defined in `src/modules/VersionModule.sol`)
-- **Solidity:** ^0.8.20 (compiled with 0.8.34)
+- **Solidity:** ^0.8.20 (compiled with 0.8.36)
 - **EVM target:** Prague
 - **License:** MPL-2.0
 
@@ -21,13 +21,27 @@ forge test               # Run all tests
 forge test -vvv          # Verbose test output
 forge test --match-contract <Name> --match-test <fn>  # Run specific test
 forge coverage           # Code coverage
-forge coverage --no-match-coverage "(script|mocks|test)" --report lcov  # Production coverage
+forge coverage --no-match-coverage "(mocks|test)" --report lcov  # Production coverage (src/ + script/)
 forge fmt                # Format code
 ```
 
 Dependencies are git submodules. Initialize with `forge install`, update with `forge update`.
 CMTAT submodule also needs `cd lib/CMTAT && npm install` for its OpenZeppelin deps.
 
+## Agent Workflow
+
+- **Never create git commits.** Provide commit messages only when they are requested.
+- **Always run the full test suite (`forge test`) after any code modification** — including lint-driven or mechanical refactors — before reporting completion.
+- **Always update the documentation** to reflect the latest change. There are two READMEs: `README.md` at the root is the short overview (project, architecture, main files, quick start); `doc/README.md` is the full reference (interfaces, Ethereum API, deployment, UML, audits). Update whichever the change affects — often both.
+- After each implemented feature or fix, provide a **one-line GitHub commit message** covering all changes since the last commit.
+
+### When implementing a new rule or feature
+
+1. Create or update the technical documentation in `doc/technical`
+2. Update `README.md` (root overview) and `doc/README.md` (full reference) as applicable
+3. Create or update tests, targeting **100% code coverage** — check with `forge coverage --report summary`
+4. Update `CHANGELOG.md`
+
 ## Import Remappings
 
 | Alias | Path |
@@ -132,31 +146,42 @@ function _checkRule(address rule_) internal view virtual override {
 ### Rule Execution Flow
 
 ```
-Token operation → RuleEngine.transferred(spender, from, to, value)   ← CMTAT v3.3.0+ primary path
+CMTAT only:       RuleEngine.transferred(spender, from, to, value)   ← transferFrom, mint, burn (spender = _msgSender())
                    ├── onlyBoundToken modifier (caller must be bound)
                    └── for each rule in _rules:
                          rule.transferred(spender, from, to, value)  // reverts if disallowed
 
-                  RuleEngine.transferred(from, to, value)            ← 3-arg fallback (spender == address(0))
+CMTAT + ERC-3643: RuleEngine.transferred(from, to, value)            ← standard transfer (spender == address(0))
                    ├── onlyBoundToken modifier
                    └── for each rule in _rules:
                          rule.transferred(from, to, value)
 
-                  RuleEngine.created(to, value)                      ← ERC-3643 mint entry point
+ERC-3643 only:    RuleEngine.created(to, value)                      ← ERC-3643 mint entry point
                    ├── onlyBoundToken modifier
                    └── calls _transferred(address(0), to, value)
 
-                  RuleEngine.destroyed(from, value)                  ← ERC-3643 burn entry point
+ERC-3643 only:    RuleEngine.destroyed(from, value)                  ← ERC-3643 burn entry point
                    ├── onlyBoundToken modifier
                    └── calls _transferred(from, address(0), value)
 ```
 
-Since CMTAT v3.3.0, mint (`from == address(0)`) and burn (`to == address(0)`) also go through the 4-argument overload with the operator as `spender`. Rules that check `spender` must skip or adapt that check for mint/burn to avoid blocking those operations unintentionally.
+**CMTAT and ERC-3643 use disjoint entry points.** The 4-argument `transferred` is declared by CMTAT's `IRuleEngine` (`lib/CMTAT/contracts/interfaces/engine/IRuleEngine.sol`), so an ERC-3643 token never reaches it. Conversely `created` / `destroyed` are declared by `IERC3643Compliance` and CMTAT never calls them — CMTAT routes mint and burn through the 4-argument `transferred` instead. Only the 3-argument `transferred` is shared by both.
+
+CMTAT selects the overload in `ValidationModuleRuleEngine._callRuleEngineTransferred`, branching on `spender != address(0)`. A standard `transfer` has no spender (`CMTATBaseCommon.transfer` passes `address(0)` internally), so the `else` branch calls the **3-argument** `transferred(from, to, value)` — the zero address is a branch condition only and is never forwarded to the engine. `transferFrom`, `mint` and `burn` carry `_msgSender()` as spender and take the **4-argument** overload. Neither is a fallback: which one is called depends purely on the operation.
+
+Since CMTAT v3.3.0, mint (`from == address(0)`) and burn (`to == address(0)`) therefore also reach the 4-argument overload with the operator as `spender`. Rules that check `spender` must skip or adapt that check for mint/burn to avoid blocking those operations unintentionally.
 
 `created` and `destroyed` use the 3-argument `_transferred` path (no spender), consistent with the ERC-3643 spec which does not carry a spender for mint/burn.
 
 View path: `detectTransferRestriction()` iterates rules, returns first non-zero code.
 
+**The 3-argument view path fails open for spender-dependent rules.** `detectTransferRestriction` and
+`canTransfer` carry no `spender`, so a rule keyed by spender (e.g. a per-minter mint allowance) cannot
+evaluate the operation and must answer "no restriction". The engine aggregates that answer, so these two views
+can report a mint as allowed that `transferred(spender, ...)` will revert. Use the 4-argument
+`detectTransferRestrictionFrom` / `canTransferFrom` to pre-check an operation that has an operator. See
+`H-1` in `doc/security/audits/tools/v3.0.0-rc5/CLAUDE_ANALYSIS.md`.
+
 ### Storage: EnumerableSet
 
 Both rules and bound tokens use `EnumerableSet.AddressSet`:
@@ -255,8 +280,11 @@ Key points:
 - NatSpec comments on all public/external functions
 - Function ordering: constructor, receive, fallback, external, public, internal, private (view/pure last within each group)
 - Function declaration order: visibility, mutability, virtual, override, custom modifiers
+- All `internal` functions must be marked `virtual`, so inheriting contracts can override them.
+- Use `require(condition, CustomError(...))` for custom errors; avoid direct `revert CustomError(...)`.
 - In `src/`, avoid `super` calls and prefer explicit parent-contract calls (e.g., `AccessControl.grantRole(...)`) for readability and deterministic inheritance behavior.
 - Section headers: `/* ============ SECTION ============ */`
+- **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.
 - Run `forge fmt` before committing
 
 ## Common Tasks

CHANGELOG.md

@@ -47,8 +47,56 @@ forge lint
 
 
 
+### v3.0.0-rc5
+
+### Changed
+
+- `ERC3643ComplianceModule._bindToken` / `_unbindToken`: rely on the `EnumerableSet` mutation return value instead of a preceding `contains()` lookup, keeping the `TokenAlreadyBound` / `TokenNotBound` diagnostics (269 gas measured).
+- `_bindToken`, `_unbindToken` and `RuleEngineBase._supportsRuleEngineBaseInterface` are now `virtual`, along with the remaining non-`virtual` internals in the mock rules, per the project convention.
+- `RuleAddressList.addressIsListedBatch`: `memory` parameter changed to `calldata` (587 gas measured for 10 addresses).
+- Deployment now emits `SetMaxRules` with the initial cap, so the event log alone is sufficient to reconstruct `maxRules`.
+- `RulesManagementModule`: the rule-cap write and its event moved into a new `internal virtual _setMaxRules(uint256)`, called by `setMaxRules` and by the deployable contracts' constructors. `_maxRules` is now written from a single place, so the invariant "every change to the cap emits `SetMaxRules`" holds structurally rather than by convention, and the non-zero check guards every path including construction.
+- `RulesManagementModule`: rule insertion moved into a new `internal virtual _addRule(IRule)`, called by `addRule` and by the `setRules` loop. `AddRule` is now emitted from a single site. The `maxRules` cap is deliberately checked by the callers, since `addRule` checks per insertion while `setRules` checks the whole batch up front.
+
+### Added
+
+- Add `ERC3643TokenMock`: a minimal ERC-3643 (T-REX) style token whose compliance interaction mirrors `Token.sol` from the reference implementation, used to test the RuleEngine through the ERC-3643 entry points (`setCompliance` self-binding, `transferred`, `created`, `destroyed`).
+- Add `ERC3643TokenIntegration.t.sol` (11 tests), including a regression guard for the H-1 mint pre-check fail-open and one pinning the requirement that `address(0)` be whitelisted for an ERC-3643 token to mint.
+
+- **Renamed the reference rules in `src/mocks/rules/` with a `Mock` suffix**, so a reader cannot mistake them for the production rules of the same name maintained in [CMTA/Rules](https://github.com/CMTA/Rules): `RuleWhitelist` -> `RuleWhitelistMock`, `RuleConditionalTransferLight` -> `RuleConditionalTransferLightMock`, `RuleMintAllowance` -> `RuleMintAllowanceMock`, `RuleOperationRevert` -> `RuleOperationRevertMock`. Files renamed to match. The abstract bases and invariant-storage contracts they build on are unchanged, as they are not themselves rules.
+
+### Removed
+
+- `RuleEngine_ERC3643Compliance_OperationNotSuccessful`: unreachable after the bind/unbind simplification and referenced nowhere else.
+
+### Documentation
+
+- Document that the ERC-1404 3-argument `canTransfer` / `detectTransferRestriction` path fails open for spender-dependent rules, and that `canTransferFrom` / `detectTransferRestrictionFrom` must be used to pre-check an operation that has an operator.
+- Add the code-quality review in [doc/security/audits/tools/v3.0.0-rc5/CLAUDE_ANALYSIS.md](./doc/security/audits/tools/v3.0.0-rc5/CLAUDE_ANALYSIS.md).
+- Add integration guides in `doc/technical`: [RuleEngine-with-CMTAT.md](./doc/technical/RuleEngine-with-CMTAT.md) and [RuleEngine-with-ERC3643.md](./doc/technical/RuleEngine-with-ERC3643.md), covering entry points, configuration, warnings, limitations and test coverage for each token standard.
+- Add the script review in [doc/security/audits/tools/v3.0.0-rc5/CLAUDE_ANALYSIS_SCRIPT.md](./doc/security/audits/tools/v3.0.0-rc5/CLAUDE_ANALYSIS_SCRIPT.md).
+- Add the v3.0.0-rc5 Slither and Aderyn reports with their assessment feedback, each prefixed with a summary table of findings and dispositions.
+- Add [doc/security/audits/AUDIT_OVERVIEW.md](./doc/security/audits/AUDIT_OVERVIEW.md) indexing every analysis performed, the static-analysis results per tool, and the substantive findings that were fixed.
+
+### Fixed
+
+- `RuleEngineScript.s.sol`: the CMTAT token is now bound to the engine (passed to the constructor). Previously the script produced a deployment in which every transfer, mint and burn reverted with `RuleEngine_ERC3643Compliance_UnauthorizedCaller`, because the token was never bound.
+- `RuleEngineScript.s.sol`: `setRuleEngine` is now called through the typed interface instead of a low-level `.call` guarded by a bare `require(success)`. The previous form returned success when `CMTAT_ADDRESS` held no code, silently producing an unconfigured deployment, and discarded the revert reason on failure.
+- `RuleEngineScript.s.sol`: the demo whitelist is now seeded with the deployer and `address(0)`, so the resulting deployment can transfer, mint and burn as-is.
+- `test/script/RuleEngineScript.t.sol`: asserts the resulting deployment works (engine set, token bound, rule configured, a real mint) instead of only that `run()` does not revert.
+- `doc/script/script_surya_*.sh`: fixed the shebang (`#/bin/bash` -> `#!/bin/bash`), the undefined `$dir` loop variable, `mkdir` without `-p` in the report script, and the output-directory guard in the inheritance script; added `set -euo pipefail` and null-delimited `find` iteration to all three. The loop iterates `find .` rather than an absolute path on purpose: `surya mdreport` embeds the path it is given, so an absolute one would write machine-specific paths into the committed reports under `doc/schema/surya/surya_report`.
+- `package.json`: the `surya:*` and `uml:*` scripts now write beneath `docOut/` (gitignored) instead of the repository root.
+- `doc/script/convert_links_for_pdf.sh`: the default input is now `doc/README.md` (the full documentation) rather than the short root README.
+
+### Dependencies
+
+- Update CMTAT submodule to [v3.3.0-rc3](https://github.com/CMTA/CMTAT/releases/tag/v3.3.0-rc3).
+- Update OpenZeppelin Contracts and OpenZeppelin Contracts Upgradeable submodules to [v5.7.0](https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v5.7.0).
+
 ### v3.0.0-rc4 - 2026-05-22
 
+Commit: `66fcf2aafebd1f9d9de8a81dec92b88da071c9b3`
+
 ### Added
 
 - Add `RuleMintAllowance` mock rule: admin-controlled per-minter mint allowance with `setMintAllowance(address, uint256)`, enforcement in `transferred(spender, from, to, value)` when `from == address(0)`, and `CODE_MINTER_INSUFFICIENT_ALLOWANCE` (code 81).