Skip to content

test token-revert bubble-up paths in LibFlow transfers#450

Merged
thedavidmeister merged 3 commits into
mainfrom
2026-05-05-issue-331-token-revert-bubble-tests
May 9, 2026
Merged

test token-revert bubble-up paths in LibFlow transfers#450
thedavidmeister merged 3 commits into
mainfrom
2026-05-05-issue-331-token-revert-bubble-tests

Conversation

@thedavidmeister
Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister commented May 5, 2026

Summary

Three new fuzz tests, one per token standard, pinning that an underlying token revert propagates out of flow() rather than being caught and swallowed by SafeERC20 or the bare ERC721/ERC1155 interface calls in LibFlow.

  • testFlowERC20TokenRevertBubblesUpTOKEN_A.transferFrom reverts.
  • testFlowERC721TokenRevertBubblesUpTOKEN_B.safeTransferFrom reverts.
  • testFlowERC1155TokenRevertBubblesUpTOKEN_C.safeTransferFrom reverts.

Closes #331.

Test plan

  • all 3 tests, 100 fuzz runs each

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added three new fuzz tests to verify token-level revert handling for ERC20, ERC721, and ERC1155 token transfers.

Review Change Stack

Three new fuzz tests, one per token standard:

- testFlowERC20TokenRevertBubblesUp — TOKEN_A.transferFrom reverts
- testFlowERC721TokenRevertBubblesUp — TOKEN_B.safeTransferFrom reverts
- testFlowERC1155TokenRevertBubblesUp — TOKEN_C.safeTransferFrom reverts

Each pins that the underlying token revert propagates out of flow()
rather than being caught and swallowed by SafeERC20 / the interface
calls in LibFlow.

Closes #331.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 290080a6-606d-4a25-9386-2a444cc062ce

📥 Commits

Reviewing files that changed from the base of the PR and between 2e4a645 and 2921b9d.

📒 Files selected for processing (2)
  • test/src/concrete/Flow.construction.t.sol
  • test/src/concrete/Flow.transfer.t.sol
💤 Files with no reviewable changes (1)
  • test/src/concrete/Flow.construction.t.sol

Walkthrough

This PR adds three new fuzz tests to FlowTransferTest that verify token-level reverts properly bubble up through flow operations for ERC20, ERC721, and ERC1155 transfers. Each test mocks the token contract to revert and asserts the revert data is propagated. A formatting adjustment is also applied to the Flow construction tests.

Changes

Token Revert Bubble-Up Coverage

Layer / File(s) Summary
Formatting Cleanup
test/src/concrete/Flow.construction.t.sol
Non-functional line-layout adjustment in testFlowConstructionRevertsOnNonZeroFlowInputs.
Token Revert Bubble-Up Tests
test/src/concrete/Flow.transfer.t.sol
Three parallel fuzz tests mock token contract reverts (ERC20 transferFrom, ERC721 safeTransferFrom(from,to,tokenId), ERC1155 safeTransferFrom) and verify flow.flow() reverts with the same revert bytes. Each test uses interpreterEval2MockCall to set up stack evaluation and vm.mockCallRevert to simulate token failures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main change: adding tests for token-revert bubble-up paths in LibFlow transfers.
Linked Issues check ✅ Passed All three fuzz tests (ERC20, ERC721, ERC1155) are implemented as proposed in #331, mocking token reverts and verifying flow() propagates them correctly.
Out of Scope Changes check ✅ Passed Changes are limited to adding three new test functions and minor formatting; all changes directly address #331 requirements with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-05-05-issue-331-token-revert-bubble-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thedavidmeister
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

thedavidmeister and others added 2 commits May 9, 2026 15:14
…token-revert-bubble-tests

# Conflicts:
#	test/src/concrete/Flow.transfer.t.sol
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thedavidmeister thedavidmeister merged commit 0ad8f77 into main May 9, 2026
4 checks passed
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.

[A23-4] [MEDIUM] LibFlow transfers: token-revert bubble-up paths uncovered

1 participant