refactor: add TrailsBalanceInjector delegatecall support for injectAndCall (previously sweepAndCall)
#40
Conversation
…l patterns Add injectSweepAndCall (regular calls with transferFrom) and injectAndCall (delegatecalls reading address(this) balance) while keeping shared logic DRY in _executeCall. This enables both backward-compatible external calls and native Sequence wallet delegatecall integration.
miguelmota
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the TrailsBalanceInjector contract to support both regular calls and delegatecalls from Sequence wallets. The refactoring separates the injection logic into two distinct functions with different behavior patterns.
Key changes:
- Adds delegatecall support for Sequence wallets through new
injectAndCallandhandleSequenceDelegateCallfunctions - Renames
sweepAndCalltoinjectSweepAndCallfor clarity and consistency - Implements conditional placeholder replacement logic to handle cases where replacement should be skipped
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/TrailsBalanceInjector.sol | Adds IDelegatedExtension interface, new delegatecall functions, refactors existing logic into internal helpers, and implements conditional placeholder replacement |
| test/TrailsBalanceInjector.t.sol | Updates test function names, adds MockWallet for delegatecall testing, and adds comprehensive test coverage for new delegatecall functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
miguelmota
changed the title
injectAndCall (previously sweepAndCall)
Updated the ETH balance retrieval logic to always use address(this).balance, ensuring the contract has ETH available before execution. Removed unnecessary checks for msg.value and clarified the require statement for better error handling.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Always use address(this).balance, regardless of call type. | ||
| // The contract must have ETH available before calling this function. | ||
| callerBalance = address(this).balance; | ||
| require(callerBalance > 0, "No ETH available in contract"); |
There was a problem hiding this comment.
The error message 'No ETH available in contract' is inconsistent with other ETH-related error messages in the codebase. Consider using 'No ETH sent' to maintain consistency with the existing error message pattern.
| require(callerBalance > 0, "No ETH available in contract"); | |
| require(callerBalance > 0, "No ETH sent"); |
|
Thank you for this! Makes a lot of sense especially the native transfer on the destination intent call w/ the |
Agreed with this; let's ship the existing contracts w/ the current intentmachine optimizations pushed to production and take it from there 🙌 |
|
Thank you @shunkakinoki! Will contracts be redeployed once this is merged? |
Previously TrailsBalanceInjector was called as separate contract call and while it worked for ERC20 sweep and call, it did not work for native sweep and call because there's no such thing as
transferFromwith native token. The changes in PR are to make it work withDelegateCall: truefrom meta tx so it's called in context of intent wallet so it can properlycallwithvaluefor native token destination call, and properly inject (replace placeholder) andcallif it's ERC20 token destination call.Dependents
https://github.com/0xsequence/trails-api/pull/6
There's a lot of overlap with
TrailsTokenSweeper, maybe it makes sense to combine these two contracts at some point or move the shared functionality out to a shared contract to dry things up