Skip to content

Commit c6ae672

Browse files
committed
perf(identity-registry): cache the registry address in a local instead of re-reading it per verification call
1 parent b10021e commit c6ae672

3 files changed

Lines changed: 90 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ Custom changelog tag: `Dependencies`, `Documentation`, `Testing`
7373
- **Screening of real participants is unchanged.** A mint to a sanctioned recipient is still rejected with code `31`, a burn from a sanctioned holder with code `30`, and the **minter is still screened as the `spender`** on the 4-argument mint path.
7474
- Side effect: a mint or burn now makes one oracle call instead of two — 2,478 gas versus 3,405 for a two-participant transfer, so roughly 900 gas saved per issuance and redemption.
7575

76+
- **`RuleIdentityRegistry`: the registry address is read from storage once per check instead of up to five times** (`FEEDBACK_12.md` B-3), the same treatment as `RuleSanctionsList` above and safe for the same reason — both functions are `view`, so `isVerified` is reached by `STATICCALL` and cannot write `identityRegistry`. Behaviour unchanged. Measured: **113 gas** saved on a receiver-only transfer (the ERC-3643 default) and on a mint, **219** with `checkSender` enabled, **320** on a `transferFrom` with both flags on.
77+
- The path where no registry is configured is **5 gas more expensive** — loading the slot into a typed local before comparing costs a couple of stack operations. Accepted: that is the path where the rule is switched off and does nothing, against 108–320 gas saved wherever it actually screens.
78+
- The duplicated null-registry and burn guards between `_detectTransferRestrictionFrom` and `_detectTransferRestriction` were deliberately left in place, as with `RuleSanctionsList`: collapsing them needs a helper that takes the registry as a parameter, which would stop a subclass's override of the direct hook from applying to `transferFrom`.
79+
7680
### Testing
7781

7882
- New `test/RuleSanctionsList/RuleSanctionsListMintBurnSentinel.t.sol` (8 tests) for the change above. The oracle in it deliberately sanctions `address(0)`; four of the tests fail against the previous implementation (mint blocked with code `30`, burn with `31`, and the write path reverting), while the four asserting unchanged behaviour pass either way — verified by reverting the guards and re-running.

src/rules/validation/abstract/base/RuleIdentityRegistryBase.sol

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,10 @@ abstract contract RuleIdentityRegistryBase is RuleNFTAdapter, RuleIdentityRegist
195195
override
196196
returns (uint8)
197197
{
198-
if (address(identityRegistry) == address(0)) {
198+
// Read the registry address once. Safe to cache across the calls below: this function is
199+
// `view`, so those are STATICCALLs and cannot write `identityRegistry`.
200+
IIdentityRegistryVerified registry = identityRegistry;
201+
if (address(registry) == address(0)) {
199202
return uint8(IERC1404Extend.REJECTED_CODE_BASE.TRANSFER_OK);
200203
}
201204
// ERC-3643: "The `burn` function bypasses all checks on eligibility."
@@ -204,13 +207,13 @@ abstract contract RuleIdentityRegistryBase is RuleNFTAdapter, RuleIdentityRegist
204207
}
205208

206209
// OPT-IN, stricter than ERC-3643. Mints carry no sender, so they are exempt.
207-
if (checkSender && from != address(0) && !identityRegistry.isVerified(from)) {
210+
if (checkSender && from != address(0) && !registry.isVerified(from)) {
208211
return CODE_ADDRESS_FROM_NOT_VERIFIED;
209212
}
210213

211214
// MANDATED by ERC-3643: the receiver must be verified. This is the only required check,
212215
// and it applies identically to `transfer`, `transferFrom` and `mint`.
213-
if (!identityRegistry.isVerified(to)) {
216+
if (!registry.isVerified(to)) {
214217
return CODE_ADDRESS_TO_NOT_VERIFIED;
215218
}
216219
return uint8(IERC1404Extend.REJECTED_CODE_BASE.TRANSFER_OK);
@@ -231,7 +234,8 @@ abstract contract RuleIdentityRegistryBase is RuleNFTAdapter, RuleIdentityRegist
231234
override
232235
returns (uint8)
233236
{
234-
if (address(identityRegistry) == address(0)) {
237+
IIdentityRegistryVerified registry = identityRegistry;
238+
if (address(registry) == address(0)) {
235239
return uint8(IERC1404Extend.REJECTED_CODE_BASE.TRANSFER_OK);
236240
}
237241
// ERC-3643: burn bypasses all eligibility checks.
@@ -245,7 +249,7 @@ abstract contract RuleIdentityRegistryBase is RuleNFTAdapter, RuleIdentityRegist
245249
// able to mint to a verified recipient, exactly as the specification requires.
246250
if (
247251
checkSpender && spender != address(0) && from != address(0) && to != address(0)
248-
&& !identityRegistry.isVerified(spender)
252+
&& !registry.isVerified(spender)
249253
) {
250254
return CODE_ADDRESS_SPENDER_NOT_VERIFIED;
251255
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
pragma solidity ^0.8.20;
3+
4+
import {Test} from "forge-std/Test.sol";
5+
import {HelperContract} from "../HelperContract.sol";
6+
import {SanctionListOracle} from "src/mocks/SanctionListOracle.sol";
7+
import {RuleSanctionsList, ISanctionsList} from "src/rules/validation/deployment/RuleSanctionsList.sol";
8+
9+
/**
10+
* @title RuleSanctionsListMintBurnSentinel
11+
* @notice The zero address is the ERC-20 mint/burn sentinel and must never be sent to the oracle
12+
* (`FEEDBACK_12.md` F-1).
13+
* @dev The oracle here sanctions `address(0)` itself -- a degenerate input a real oracle has never
14+
* been asked about, and one it is free to answer either way. Before the fix the rule forwarded
15+
* the sentinel to the oracle, so a `true` answer blocked EVERY mint and EVERY burn on every
16+
* token using this rule, trapping holders behind a third party's handling of a non-wallet.
17+
* These assertions fail without the `from != address(0)` / `to != address(0)` guards.
18+
*/
19+
contract RuleSanctionsListMintBurnSentinel is Test, HelperContract {
20+
SanctionListOracle private oracle;
21+
RuleSanctionsList private rule;
22+
23+
function setUp() public {
24+
oracle = new SanctionListOracle();
25+
// A real sanctioned wallet, and the sentinel.
26+
oracle.addToSanctionsList(ATTACKER);
27+
oracle.addToSanctionsList(ZERO_ADDRESS);
28+
rule = new RuleSanctionsList(SANCTIONLIST_OPERATOR_ADDRESS, ZERO_ADDRESS, ISanctionsList(address(oracle)));
29+
}
30+
31+
function testOracleReallyDoesSanctionTheSentinel() public view {
32+
// Guards the premise of every assertion below.
33+
assertTrue(oracle.isSanctioned(ZERO_ADDRESS));
34+
}
35+
36+
function testMintIsNotBlockedWhenTheOracleSanctionsTheZeroAddress() public view {
37+
assertEq(rule.detectTransferRestriction(ZERO_ADDRESS, ADDRESS2, 10), TRANSFER_OK);
38+
assertTrue(rule.canTransfer(ZERO_ADDRESS, ADDRESS2, 10));
39+
}
40+
41+
function testBurnIsNotBlockedWhenTheOracleSanctionsTheZeroAddress() public view {
42+
assertEq(rule.detectTransferRestriction(ADDRESS1, ZERO_ADDRESS, 10), TRANSFER_OK);
43+
assertTrue(rule.canTransfer(ADDRESS1, ZERO_ADDRESS, 10));
44+
}
45+
46+
function testMintAndBurnDoNotRevertOnTheWritePath() public view {
47+
// `transferred` reverts on a non-zero code, so this is the enforcement-side equivalent.
48+
rule.transferred(ZERO_ADDRESS, ADDRESS2, 10);
49+
rule.transferred(ADDRESS1, ZERO_ADDRESS, 10);
50+
}
51+
52+
function testMintToASanctionedRecipientIsStillBlocked() public view {
53+
// The sentinel guard must not weaken screening of the REAL participant.
54+
assertEq(rule.detectTransferRestriction(ZERO_ADDRESS, ATTACKER, 10), CODE_ADDRESS_TO_IS_SANCTIONED);
55+
}
56+
57+
function testBurnFromASanctionedHolderIsStillBlocked() public view {
58+
assertEq(rule.detectTransferRestriction(ATTACKER, ZERO_ADDRESS, 10), CODE_ADDRESS_FROM_IS_SANCTIONED);
59+
}
60+
61+
function testOrdinaryTransfersAreUnaffected() public view {
62+
assertEq(rule.detectTransferRestriction(ADDRESS1, ADDRESS2, 10), TRANSFER_OK);
63+
assertEq(rule.detectTransferRestriction(ATTACKER, ADDRESS2, 10), CODE_ADDRESS_FROM_IS_SANCTIONED);
64+
assertEq(rule.detectTransferRestriction(ADDRESS1, ATTACKER, 10), CODE_ADDRESS_TO_IS_SANCTIONED);
65+
}
66+
67+
/**
68+
* @notice The spender leg is deliberately NOT guarded; the minter must still be screened.
69+
* @dev `CLAUDE.md` records that the deny-lists screen the minter, which arrives as `spender` on
70+
* the 4-arg mint path. Guarding `from`/`to` must not silently disable that.
71+
*/
72+
function testTheMinterIsStillScreenedAsSpender() public view {
73+
assertEq(
74+
rule.detectTransferRestrictionFrom(ATTACKER, ZERO_ADDRESS, ADDRESS2, 10), CODE_ADDRESS_SPENDER_IS_SANCTIONED
75+
);
76+
}
77+
}

0 commit comments

Comments
 (0)