[DO NOT MERGE] Refactor TrailsRouter to integrate Sequence V3 CallsPayload format #82
Conversation
shunkakinoki
commented
Nov 27, 2025
shunkakinoki
commented
- Introduced a new Guest module interface for forwarding CallsPayload.
- Updated execute and pullAmountAndExecute functions to accept Sequence V3 CallsPayload format, replacing previous Multicall3 integration.
- Enhanced error handling with new error types for invalid payload formats and call execution failures.
- Updated tests to reflect changes in call execution and removed obsolete Multicall3 references.
- Adjusted documentation to clarify the new functionality and usage of the Guest module.
- Introduced a new Guest module interface for forwarding CallsPayload. - Updated execute and pullAmountAndExecute functions to accept Sequence V3 CallsPayload format, replacing previous Multicall3 integration. - Enhanced error handling with new error types for invalid payload formats and call execution failures. - Updated tests to reflect changes in call execution and removed obsolete Multicall3 references. - Adjusted documentation to clarify the new functionality and usage of the Guest module.
… recent performance optimizations and adjustments in test cases. Modified gas usage metrics for various tests to ensure accuracy and consistency following refactoring and enhancements in the contract logic.
…deployment address for Sequence V3 integration.
- Updated TrailsRouter to initialize GUEST_MODULE through the constructor instead of a hardcoded value. - Modified deployment scripts to pass the Guest module address during deployment. - Adjusted tests to reflect the new initialization method for the TrailsRouter contract.
…or improved code readability.
…quence V3 CallsPayload format - Updated TrailsRouter to replace Multicall3 references with Sequence V3 CallsPayload for batch call execution. - Adjusted related documentation in CODE4ARENA.md and README.md to reflect the new CallsPayload functionality. - Removed the IMulticall3 interface and associated mock implementations to streamline the codebase. - Enhanced test cases to utilize the new CallsPayload format, ensuring comprehensive coverage and accuracy in call execution. - Updated diagrams in FLOW.md to illustrate the new call routing process through the Guest module.
- Introduced a new error type, BehaviorOnErrorMustBeRevert, to enforce that all calls in the Sequence V3 CallsPayload must have behaviorOnError set to REVERT_ON_ERROR. - Implemented the _validateV3CallsPayload function to decode and validate the payload format, ensuring atomicity in batch call execution. - Updated execute function to utilize the new validation logic, enhancing error handling and payload integrity.
…ter and TrailsRouterShim - Adjusted gas usage metrics for various tests to reflect recent performance changes. - Updated expected router address calculations to include the guest module address in the deployment scripts, ensuring consistency with the new initialization method. - Enhanced comments for clarity regarding the expected router address and its relation to the deployment script's initCode.
- Removed unnecessary whitespace in TrailsRouter and TrailsRouterShim test files to enhance code clarity. - Adjusted formatting in test cases to maintain uniformity across the codebase.
|
I think the changes superficially look fine, but I do not think it is a good idea to merge this at this stage. We have seen vulnerabilities involving I agree that it is a good idea to re-use the guest contracts instead of depending on As a general rule, it is risky to make big changes after audits, because audits build on top of assumptions, and when you change one of the assumptions you have to re-reason the whole assessment; you don't know the true impact of a small-looking change. Unless there is a good reason to do this change now, I propose we leave this for Trails v1.1 or v2. The changes themselves look good, I can't see any immediate issue with them. |
|
Thank you for the above! Completely agreed on the above viewpoint; not a wise decision to merge at this point in time, 100% agreed. Will keep this as a reference draft as well as the backend corresponding PR https://github.com/0xsequence/trails-api/pull/190 for refernece and keep it as draft for the time being) |
|
We should reconsider the whole flow between the intent address and Trails contracts before including this change in v1.1 / v2. I believe it's possible to remove For times where fallback logic that's required over multiple calls, we can use nested payload calls to The |
pkieltyka
changed the title
pkieltyka
changed the title
