-
Notifications
You must be signed in to change notification settings - Fork 191
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: actually make permit2 allowance/sign distinct steps #8509
Conversation
return [HopExecutionState.AwaitingAllowanceApproval].includes(state) | ||
} | ||
|
||
const isInPermit2SignState = (state: HopExecutionState): boolean => { | ||
return [HopExecutionState.AwaitingPermit2Eip712Sign].includes(state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick]
return [HopExecutionState.AwaitingAllowanceApproval].includes(state) | |
} | |
const isInPermit2SignState = (state: HopExecutionState): boolean => { | |
return [HopExecutionState.AwaitingPermit2Eip712Sign].includes(state) | |
return state === HopExecutionState.AwaitingAllowanceApproval | |
} | |
const isInPermit2SignState = (state: HopExecutionState): boolean => { | |
return state = HopExecutionState.AwaitingPermit2Eip712Sign |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧠
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works good, get it in
Description
Follows up on #8508 by akschually making the steps separate.
While 8508 made sure we use nice copies for permit2 contract allowance / permit2 sign, this actually splits the two into separate steps.
Note, this is for new swapper flow only.
Issue (if applicable)
Risk
Low
Testing
Engineering
Test with new swapper flow flag on - effectively the same as #8508, except for the first case, where there should be two explicit steps.
0x trade - No Permit2 allowance granted for asset
0x trade - Infinite Permit2 allowance already granted for asset
Non-0x-trade
Operations
None yet! This only touches the new trade flow under flag.
Screenshots (if applicable)
https://jam.dev/c/22ecd401-665f-40ab-910e-c66e71019df9
https://jam.dev/c/ae7cf74e-90a7-4e97-b929-8e06c8116307
https://jam.dev/c/f1cf7bdf-4f3a-4760-9a3d-d813d175b401