You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
|`src/registry/`| Contracts filling a token's **identity registry** slot, not its compliance slot (`IdentityRegistryWhitelist`). Not rules: no `IRule`, never added to a RuleEngine |
@@ -171,5 +171,5 @@ Gotchas worth knowing before you change anything:
171
171
-`RuleChainlinkPoR` reads the feed's `decimals()`**live on every check** and deliberately does NOT cache it. Caching saves ~2,900 gas per mint but lets an aggregator migration that changes decimals mis-scale the reserves by `10 ** delta` with no on-chain signal — in the overstating direction that is unlimited unbacked minting. Both feed calls share the `code.length` guard (Solidity's extcodesize revert on a `try` to a codeless address is uncatchable) and `MAX_FEED_DECIMALS` is re-checked at read time, not just at configuration. Do not "optimise" this back into a cache.
172
172
-`RuleChainlinkPoR` (and `RuleMaxTotalSupply`) protect **one token per instance** with no on-chain guard: they read `totalSupply()` from the configured `tokenContract`, never from the token that triggered the check, and behind a RuleEngine they cannot learn that identity. One instance added to two RuleEngines evaluates both tokens against the first one's supply and feed — silently over-minting or freezing the second. Chainlink's `SecureMintPolicy` blocks this with `onInstall`/`PolicyAlreadyBound`; adding an equivalent here would mean making a stateless validation rule bindable, which is a library-wide decision. Documented, not fixed.
173
173
-`ADDRESS_LIST_ADD_ROLE` / `ADDRESS_LIST_REMOVE_ROLE` live in `RuleAddressSetRolesStorage`, inherited by `RuleAddressSet` (the public layer that enforces them) — **not** by `RuleAddressSetInternal`. Do not move them back into `RuleAddressSetInvariantStorage`: a contract reusing only the internal layer (`IdentityRegistryWhitelist`) would then publish two roles it never checks, and an operator granting one would get no privilege and no signal.
174
-
- `RuleChainlinkPoR` and `RuleMaxTotalSupply` both guard `tokenContract.totalSupply()` with a code-length check plus `try/catch`, returning a restriction code (78 and 51 respectively) rather than reverting, and both validate the token at configuration (non-zero, has code, `totalSupply()` callable). The ERC-1404 views MUST NOT revert — never call `totalSupply()` unguarded on a read path. Neither rule re-checks `code.length` at read time: `try/catch` cannot catch a call to a codeless address (Solidity's extcodesize check reverts uncatchably), but the setters require code and EIP-6780 (Cancun) makes that permanent, so the check would be unreachable. This is a **deployment precondition** (Cancun or later, `foundry.toml` targets `prague`), documented in each rule doc rather than enforced at runtime — do not re-add the guard unless targeting a pre-Cancun chain. `decimals()` stays optional on the token; `totalSupply()` is mandatory. The two rules use *different* constant names for the same idea (`CODE_TOTAL_SUPPLY_UNAVAILABLE` vs `CODE_SUPPLY_ORACLE_UNAVAILABLE`) because `HelperContract` inherits both invariant-storage contracts and identical identifiers would clash.
174
+
- `RuleChainlinkPoR` and `RuleMaxTotalSupply` both guard `tokenContract.totalSupply()` with a code-length check plus `try/catch`, returning a restriction code (78 and 51 respectively) rather than reverting, and both validate the token at configuration (non-zero, has code, `totalSupply()` callable). The shared mechanics — the revert-free read and the configuration probe — live in `TokenSupplyReader`; each rule supplies its own token via the `_supplyToken()` hook (so the base holds **no storage** and cannot reorder any rule's slots) and raises its own named errors. Do not move the `require`s into the base: three distinct configuration failures deliberately keep three distinct per-rule errors. The ERC-1404 views MUST NOT revert — never call `totalSupply()` unguarded on a read path. Neither rule re-checks `code.length` at read time: `try/catch` cannot catch a call to a codeless address (Solidity's extcodesize check reverts uncatchably), but the setters require code and EIP-6780 (Cancun) makes that permanent, so the check would be unreachable. This is a **deployment precondition** (Cancun or later, `foundry.toml` targets `prague`), documented in each rule doc rather than enforced at runtime — do not re-add the guard unless targeting a pre-Cancun chain. `decimals()` stays optional on the token; `totalSupply()` is mandatory. The two rules use *different* constant names for the same idea (`CODE_TOTAL_SUPPLY_UNAVAILABLE` vs `CODE_SUPPLY_ORACLE_UNAVAILABLE`) because `HelperContract` inherits both invariant-storage contracts and identical identifiers would clash.
175
175
-`RuleChainlinkPoR` accepts `tokenDecimals == 0`. Chainlink's `SecureMintPolicy` requires 1–18, but CMTAT equity tokens report 0 decimals, so the lower bound was dropped. Do not re-add it.
-**`RuleMaxTotalSupply` and `RuleChainlinkPoR` share their supply-reading mechanics** (`FEEDBACK_12.md` D-2). New `TokenSupplyReader` holds the revert-free `totalSupply()` read that both rules implemented byte-identically, plus the `try/catch` probe their configuration validators use. Behaviour unchanged; **storage layout identical**, verified per-slot from the compiled artifacts for both rules and both Ownable2Step variants.
53
+
-**The base declares no storage.** Each rule keeps its own `tokenContract` and implements a `_supplyToken()` hook — the template-method pattern already used for `_authorize*`. Declaring the variable in the base would have reordered `RuleChainlinkPoR`'s slots, moving `tokenContract` ahead of `reservesFeed`, for no benefit.
54
+
-**Configuration validation stays per-rule.** Both rules check non-zero / has-code / `totalSupply()`-callable, but each raises its own named error for each failure. Only the probe moved, returning a `bool` the rule turns into its own error; collapsing the three `require`s into one helper would trade three named operator-facing diagnostics for a few saved lines.
55
+
- Marginally **faster**: 12 gas less on both mint read paths (`RuleChainlinkPoR` 5,966 → 5,954, `RuleMaxTotalSupply` 2,460 → 2,448), because the hook inlines and removes an intermediate stack shuffle.
52
56
-**The batch add/remove loops are shared instead of written three times** (`FEEDBACK_12.md` D-1). New `AddressSetBatchLib` holds the two loops that `RuleAddressSetInternal` and `RuleERC2980Internal` (once for its whitelist, once for its frozenlist) each carried their own copy of. Behaviour is unchanged and **storage layout is identical**, verified per-slot from the compiled artifacts across `RuleWhitelist`, `RuleERC2980`, `RuleBlacklist`, `RuleReceiverWhitelist`, `RuleSpenderWhitelist` and `IdentityRegistryWhitelist`.
53
57
- 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.
54
58
-**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.
|`src/registry/`| Contracts filling a token's **identity registry** slot, not its compliance slot (`IdentityRegistryWhitelist`). Not rules: no `IRule`, never added to a RuleEngine |
@@ -171,5 +171,5 @@ Gotchas worth knowing before you change anything:
171
171
-`RuleChainlinkPoR` reads the feed's `decimals()`**live on every check** and deliberately does NOT cache it. Caching saves ~2,900 gas per mint but lets an aggregator migration that changes decimals mis-scale the reserves by `10 ** delta` with no on-chain signal — in the overstating direction that is unlimited unbacked minting. Both feed calls share the `code.length` guard (Solidity's extcodesize revert on a `try` to a codeless address is uncatchable) and `MAX_FEED_DECIMALS` is re-checked at read time, not just at configuration. Do not "optimise" this back into a cache.
172
172
-`RuleChainlinkPoR` (and `RuleMaxTotalSupply`) protect **one token per instance** with no on-chain guard: they read `totalSupply()` from the configured `tokenContract`, never from the token that triggered the check, and behind a RuleEngine they cannot learn that identity. One instance added to two RuleEngines evaluates both tokens against the first one's supply and feed — silently over-minting or freezing the second. Chainlink's `SecureMintPolicy` blocks this with `onInstall`/`PolicyAlreadyBound`; adding an equivalent here would mean making a stateless validation rule bindable, which is a library-wide decision. Documented, not fixed.
173
173
-`ADDRESS_LIST_ADD_ROLE` / `ADDRESS_LIST_REMOVE_ROLE` live in `RuleAddressSetRolesStorage`, inherited by `RuleAddressSet` (the public layer that enforces them) — **not** by `RuleAddressSetInternal`. Do not move them back into `RuleAddressSetInvariantStorage`: a contract reusing only the internal layer (`IdentityRegistryWhitelist`) would then publish two roles it never checks, and an operator granting one would get no privilege and no signal.
174
-
- `RuleChainlinkPoR` and `RuleMaxTotalSupply` both guard `tokenContract.totalSupply()` with a code-length check plus `try/catch`, returning a restriction code (78 and 51 respectively) rather than reverting, and both validate the token at configuration (non-zero, has code, `totalSupply()` callable). The ERC-1404 views MUST NOT revert — never call `totalSupply()` unguarded on a read path. Neither rule re-checks `code.length` at read time: `try/catch` cannot catch a call to a codeless address (Solidity's extcodesize check reverts uncatchably), but the setters require code and EIP-6780 (Cancun) makes that permanent, so the check would be unreachable. This is a **deployment precondition** (Cancun or later, `foundry.toml` targets `prague`), documented in each rule doc rather than enforced at runtime — do not re-add the guard unless targeting a pre-Cancun chain. `decimals()` stays optional on the token; `totalSupply()` is mandatory. The two rules use *different* constant names for the same idea (`CODE_TOTAL_SUPPLY_UNAVAILABLE` vs `CODE_SUPPLY_ORACLE_UNAVAILABLE`) because `HelperContract` inherits both invariant-storage contracts and identical identifiers would clash.
174
+
- `RuleChainlinkPoR` and `RuleMaxTotalSupply` both guard `tokenContract.totalSupply()` with a code-length check plus `try/catch`, returning a restriction code (78 and 51 respectively) rather than reverting, and both validate the token at configuration (non-zero, has code, `totalSupply()` callable). The shared mechanics — the revert-free read and the configuration probe — live in `TokenSupplyReader`; each rule supplies its own token via the `_supplyToken()` hook (so the base holds **no storage** and cannot reorder any rule's slots) and raises its own named errors. Do not move the `require`s into the base: three distinct configuration failures deliberately keep three distinct per-rule errors. The ERC-1404 views MUST NOT revert — never call `totalSupply()` unguarded on a read path. Neither rule re-checks `code.length` at read time: `try/catch` cannot catch a call to a codeless address (Solidity's extcodesize check reverts uncatchably), but the setters require code and EIP-6780 (Cancun) makes that permanent, so the check would be unreachable. This is a **deployment precondition** (Cancun or later, `foundry.toml` targets `prague`), documented in each rule doc rather than enforced at runtime — do not re-add the guard unless targeting a pre-Cancun chain. `decimals()` stays optional on the token; `totalSupply()` is mandatory. The two rules use *different* constant names for the same idea (`CODE_TOTAL_SUPPLY_UNAVAILABLE` vs `CODE_SUPPLY_ORACLE_UNAVAILABLE`) because `HelperContract` inherits both invariant-storage contracts and identical identifiers would clash.
175
175
-`RuleChainlinkPoR` accepts `tokenDecimals == 0`. Chainlink's `SecureMintPolicy` requires 1–18, but CMTAT equity tokens report 0 decimals, so the lower bound was dropped. Do not re-add it.
0 commit comments