Skip to content

fix: consume allowance on transferFrom when caller == from#87

Merged
amiecorso merged 3 commits into
mainfrom
fix/mockb20-transferfrom-self-allowance
May 28, 2026
Merged

fix: consume allowance on transferFrom when caller == from#87
amiecorso merged 3 commits into
mainfrom
fix/mockb20-transferfrom-self-allowance

Conversation

@stevieraykatz
Copy link
Copy Markdown
Member

Summary

MockB20.transferFrom (and transferFromWithMemo) gated _consumeAllowance behind msg.sender != from, so an owner-as-caller transferFrom(self, to, amt) silently moved tokens without burning any approval. OZ ERC20 and the Rust precompile both carve no exception for msg.sender == from, so this is a real divergence — flagged by Andreas in #proj-b20.

Fix

_consumeAllowance now fires unconditionally outside the factory bootstrap window. The executor-policy check is still gated on msg.sender != from, since the sender-policy already covers from inside _transfer and self-callers aren't a distinct executor — this is the one carve-out the mock intentionally keeps.

if (!_isPrivileged()) {
    _consumeAllowance(from, msg.sender, amount);
    if (msg.sender != from) {
        // executor policy check
    }
}

Tests

Regression tests added to both transferFrom.t.sol and transferFromWithMemo.t.sol (all fuzz, per repo convention):

  • test_transferFrom_revert_selfCaller_insufficientAllowance — owner-as-caller with no self-approval reverts InsufficientAllowance
  • test_transferFrom_success_selfCaller_decreasesAllowance — owner-as-caller consumes allowances[from][from] slot
  • test_transferFrom_success_selfCaller_skipsExecutorPolicy — pins the intentional carve-out: executor policy is still bypassed when msg.sender == from
  • test_transferFromWithMemo_revert_selfCaller_insufficientAllowance / …_success_selfCaller_decreasesAllowance — same for the memo variant

Test plan

  • forge test --match-path "test/unit/B20/erc20/transferFrom.t.sol" — 14/14 passing
  • forge test --match-path "test/unit/B20/**/*.t.sol" — 191/191 passing
  • forge test (full suite) — 504/504 passing

stevieraykatz and others added 2 commits May 27, 2026 15:46
MockB20.transferFrom and transferFromWithMemo gated _consumeAllowance
behind `msg.sender != from`, silently skipping allowance burn when the
owner was the caller. OZ ERC20 and the Rust precompile both carve no
such exception. Lift _consumeAllowance to fire unconditionally outside
the factory bootstrap window; keep the executor-policy check gated on
`msg.sender != from` (sender-policy already covers `from`).

Regression tests pin the corrected behavior on both transferFrom and
transferFromWithMemo: self-caller now reverts InsufficientAllowance
without a self-approval and consumes the slot when one exists.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@stevieraykatz stevieraykatz requested a review from amiecorso May 27, 2026 23:01
@linear
Copy link
Copy Markdown

linear Bot commented May 28, 2026

BOP-173

@amiecorso amiecorso merged commit 9218b0b into main May 28, 2026
6 checks passed
@amiecorso amiecorso deleted the fix/mockb20-transferfrom-self-allowance branch May 28, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants