Skip to content

test flow() atomicity on later-transfer failure#445

Merged
thedavidmeister merged 2 commits intomainfrom
2026-05-05-issue-318-flow-atomicity-test
May 8, 2026
Merged

test flow() atomicity on later-transfer failure#445
thedavidmeister merged 2 commits intomainfrom
2026-05-05-issue-318-flow-atomicity-test

Conversation

@thedavidmeister
Copy link
Copy Markdown
Contributor

@thedavidmeister thedavidmeister commented May 5, 2026

Summary

Pins the documented atomicity invariant on IFlowV5.flow(): when a later transfer in the same flow reverts, the entire flow reverts and no earlier transfer's effects persist.

The test mixes a valid ERC20 transfer with an ERC1155 transfer that fails the from-allowed check. The outer revert reason MUST be the inner failure's selector — that proves the inner revert was not caught and squashed by a wrapper.

EVM transaction-level rollback handles the actual state revert; this test exists to lock the revert-propagation invariant against a future change that wraps inner calls in try/catch.

Closes #318.

Test plan

  • testFlowAtomicRollbackOnLaterTransferFailure — 100 fuzz runs

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added test coverage to verify atomic rollback behavior in flow transfers, ensuring earlier successful transfers are reverted when subsequent transfers fail.

Pins the documented atomicity invariant on `IFlowV5.flow()`: when a
later transfer in the same flow reverts, the entire flow reverts and
no earlier transfer's effects persist. The test mixes a valid ERC20
transfer with an ERC1155 transfer that fails the from-allowed check;
the outer revert reason MUST be the inner failure's selector, which
proves the inner revert was not caught and squashed.

EVM transaction-level rollback handles the actual state revert; the
test exists to lock the revert-propagation invariant against a future
change that wraps inner calls in try/catch.

Closes #318.

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

coderabbitai Bot commented May 5, 2026

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: edaa42db-f162-46ee-b0db-c53eda17211b

📥 Commits

Reviewing files that changed from the base of the PR and between f434ae7 and 880275b.

📒 Files selected for processing (1)
  • test/src/concrete/Flow.transfer.t.sol

Walkthrough

This PR adds a new fuzz test testFlowAtomicRollbackOnLaterTransferFailure to the Flow transfer test suite. The test verifies that IFlowV5.flow() atomically processes transfers: when a later transfer fails, the entire batch reverts and earlier successful transfers are rolled back without leaving observable state changes.

Changes

Atomic Rollback Test

Layer / File(s) Summary
Test Implementation
test/src/concrete/Flow.transfer.t.sol
Fuzz test that constructs a flow with a mocked-to-succeed ERC20 transfer and a failing ERC1155 transfer (disallowed sender), mocks the ERC20 call, and asserts the entire flow() call reverts with UnsupportedERC1155Flow.selector, confirming atomicity and rollback.

Possibly related issues

🎯 2 (Simple) | ⏱️ ~8 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-05-05-issue-318-flow-atomicity-test

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 7, 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.

…flow-atomicity-test

# Conflicts:
#	test/src/concrete/Flow.transfer.t.sol
@thedavidmeister thedavidmeister merged commit 7b167f3 into main May 8, 2026
2 of 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.

[A22-3] [LOW] IFlowV5.flow() atomicity invariant has no dedicated test

1 participant