Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add SwapRouter02ExecutorWithPermit to support 2612 style permit tokens #187

Closed
wants to merge 6 commits into from

Conversation

zhongeric
Copy link
Collaborator

Add SwapRouter02ExecutorWithPermit, an extension of the current sample executor which introduces two new entrypoints for filing orders: executeWithPermit and executeBatchWithPermit.

This allows fillers to fill orders sent by users who have not yet made the necessary approvals to Permit2 for the input tokens in their order. The executor will simply confirm the 2612 permit before calling the respective execute function on the reactor.

Comment on lines +24 to +27
ISwapRouter02 public immutable swapRouter02;
address public immutable whitelistedCaller;
IReactor public immutable reactor;
WETH public immutable weth;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making these public so swapRouter02ExecutorWithPermit can inherit

function executeBatchWithPermit(
SignedOrder[] calldata orders,
bytes calldata callbackData,
bytes[] calldata permitData
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing array of permitData for executeBatch and single for execute single doesn't seem like a perfect matchup - like in the batch of orders they might not all need permits, so just cuz its a list of orders doesnt mean its necessarily a list of permits.. likewise maybe for executesingle we might want to send multiple permits for some reason?

i think we should just decouple it and have a multicall and permit function so the permits can be layered on granularly

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should make a base contract that other executors can inherit from that has this shared logic? wdyt

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooo good point about the multiple orders not all needing permits, it would be weird to have the inputs be different lengths...

how would you layer permits on granularly? are you thinking of bundling the orders together with any applicable permits like [ (order, permits[]), (order, permits[]), ...] ?

@zhongeric
Copy link
Collaborator Author

Closing this in favor of #202

@zhongeric zhongeric closed this Sep 14, 2023
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