feat(policy): enforce TRANSFER_EXECUTOR_POLICY on every transfer path - #224
Open
rayyan224 wants to merge 1 commit into
Open
feat(policy): enforce TRANSFER_EXECUTOR_POLICY on every transfer path#224rayyan224 wants to merge 1 commit into
rayyan224 wants to merge 1 commit into
Conversation
Interface Coverage✅ All interface functions have test coverage. |
📊 Forge Coverage (
|
| File | Lines | Stmts | Branches | Funcs |
|---|---|---|---|---|
| 🟡 B20FactoryLib.sol | 97.70% | 98.00% | 100.00% | 95.00% |
| 🔴 test/lib/ForceFeeder.sol | 0.00% | 0.00% | 100.00% | 0.00% |
| 🔴 test/lib/PrecompileProbe.sol | 0.00% | 0.00% | 0.00% | 0.00% |
| 🟢 MockActivationRegistry.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockActivationRegistryStorage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockB20.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockB20Asset.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟡 MockB20Factory.sol | 98.96% | 99.10% | 100.00% | 100.00% |
| 🟢 MockB20Stablecoin.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟢 MockB20Storage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| 🟡 MockPolicyRegistry.sol | 100.00% | 99.54% | 97.67% | 100.00% |
| 🟢 MockPolicyRegistryStorage.sol | 100.00% | 100.00% | 100.00% | 100.00% |
| Total | 97.05% | 97.49% | 98.12% | 97.00% |
Full report: download artifact. To browse locally: make coverage (runs forge coverage + genhtml + opens the HTML report).
|
rayyan224
force-pushed
the
feat/policy-transfer-executor-enforcement
branch
from
September 10, 2026 20:11
8b64ef2 to
2c63f86
Compare
Centralize the executor gate in `_transfer` so TRANSFER_EXECUTOR_POLICY is checked against `msg.sender` on all four transfer entrypoints (`transfer`, `transferFrom`, `transferWithMemo`, `transferFromWithMemo`), including when `msg.sender == from`. Previously it ran only on the delegated `transferFrom` paths and only when `msg.sender != from`. This closes two bypasses that made an executor allowlist unenforceable: direct `transfer` was never gated, and a self-`transferFrom` skipped the check. The executor scope is now a true "who may initiate a transfer" gate. Behavioral only — no new selectors, events, errors, or storage. An unset executor slot stays always-allow, so tokens that never configured the policy are unaffected. The factory bootstrap bypass and allowance accounting are unchanged. - MockB20: move executor check into `_transfer` (first, before sender/receiver); drop the duplicated body checks and the `msg.sender == from` carve-out - tests: executor cases on transfer + memo, EXECUTOR woven into the revert-order suites, and the old self-caller skip test inverted to pin the closed loophole - interface/comments: IB20 natspec, mock/storage comments Co-Authored-By: Claude <noreply@anthropic.com>
rayyan224
force-pushed
the
feat/policy-transfer-executor-enforcement
branch
from
September 10, 2026 20:28
2c63f86 to
6a55bf8
Compare
rayyan224
commented
Sep 10, 2026
| // is still not decremented. The executor policy is enforced centrally | ||
| // in `_transfer` (on `msg.sender`), which honors the bootstrap bypass. | ||
| _consumeAllowance(from, msg.sender, amount); | ||
| if (!_isPrivileged() && msg.sender != from) { |
Collaborator
Author
There was a problem hiding this comment.
Removed here, becaues repulled in the helper no need for an extra SLOAD
rayyan224
marked this pull request as ready for review
September 10, 2026 20:29
rayyan224
requested review from
amiecorso,
eric-ships,
ilikesymmetry and
stevieraykatz
as code owners
September 10, 2026 20:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
TRANSFER_EXECUTOR_POLICYapply to every transfer path. The executor gate now checksmsg.senderontransfer,transferFrom,transferWithMemo, andtransferFromWithMemo— including whenmsg.sender == from. Previously it ran only on the delegatedtransferFrompaths, and only whenmsg.sender != from.This targets the Q4 "Denim" candidate "Apply transfer executor policy on normal transfer" (P2) — letting issuers use an executor allowlist to restrict who may initiate a transfer (e.g. only an approved settlement contract).
Why
The old behavior left the executor scope unenforceable as an initiator gate, via two bypasses:
transferwas never gated — the initiator ismsg.sender(==from), and the check ran only insidetransferFrom.transferFromskipped the check —msg.sender == frombypassed it, so a non-allowlisted holder could routetransferFrom(self, to, amount)to move tokens anyway.Centralizing the check in
_transferonmsg.senderand removing themsg.sender == fromcarve-out closes both.Approach
test/lib/mocks/MockB20.sol): executor check moved into_transfer(first, before sender/receiver, under the existing_isPrivileged()bootstrap bypass); duplicated body checks and themsg.sender == fromcarve-out removed. Allowance is still consumed in thetransferFrom*bodies first, so revert order is unchanged.transfer.t.sol+ memo parity; EXECUTOR woven intotransfer_revertOrder.t.sol(C(7,2)=21 pairs) and the memo sequential order test; the oldtransferFromself-caller skip test inverted intotest_transferFrom_revert_selfCaller_executorPolicyForbidsto pin the closed loophole.IB20.solnatspec for the executor scope + thetransfer/transferFromrevert lists; stale "delegated-only" comments in the mock/storage/revert-order headers.Compatibility
Purely behavioral — no new selectors, events, errors, or storage. An unset executor slot stays always-allow, so tokens that never configured the policy are unaffected. Factory bootstrap bypass and allowance accounting are unchanged.
Breaking only for a token that has set a restrictive
TRANSFER_EXECUTOR_POLICYand relies on holders moving their own tokens viatransfer/ self-transferFrom— those holders must now be authorized as initiators.Testing
forge test— 746 passed, 0 failed, 4 skipped (pre-existing mock-only privileged skips).🤖 Generated with Claude Code