feat: (WIP) Simplified PropellerRouter Sketch #127
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Executors/Verifiers - There will be no more built-in executors (such as USV2 and V3), not built-in callback verifiers. - Removed swap callback methods. Should we still include a generic callback in the fallback() method? ActionTypes - No more TRANSFER, PAY, or SWAP action types. I don't see these being used in any way after we simplify the UniswapV3 Executor. I've also removed the _getPayer() method because of this. Fees - No fee taking, right? TODO - ETH wrapping and unwrapping (see permissioned frontent contract for implementation) - Some imports missing (Efficient ERC20, PackedSwapStructs, PrefixLengthEncodedByteArray) - Remove SetApprovals if using Permit2
tamaralipows
commented
Dec 19, 2024
| address internal _currentSender; | ||
| } | ||
|
|
||
| contract SwapContext is SwapContextStorage { |
There was a problem hiding this comment.
Don't fully understand this.
There was a problem hiding this comment.
Thank you so much @tamaralipows 🙏🏼 This is looking greeeaatt! awesome push!
left a million comments for future us.
No fee taking, right?
that is a great question. no idea
| * @author PropellerHeads Devs | ||
| * @dev Allows a contract to tokens for trading on third party contracts. | ||
| */ | ||
| contract ApprovalManagement is IBatchSwapRouterV1Structs { |
There was a problem hiding this comment.
let's leave this for now until we get feedback from the users. then we can check how to use permit2
| 0xd8aa0f3194971a2a116679f7c2090f6939c8d4e01a2a8d7e41d55e5351469e63; | ||
|
|
||
| constructor() { | ||
| _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); |
There was a problem hiding this comment.
from the audit: we should have multiple roles
| _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); | ||
| } | ||
|
|
||
| function _executeSwap( |
There was a problem hiding this comment.
you know I'm gonna ask for these functions to be in order of usage eheh
Comment on lines
126
to
129
| // Idea: On v2, reserve 14 bytes for calculatedAmount and replace them here | ||
| // to save some quotes, if these 14 bytes are all zero the swap call won't | ||
| // recalculate the quote else, it will simply execute with the calculatedAmount | ||
| // that was passed along. |
There was a problem hiding this comment.
I don't understand this idea 🤔 but I think I don't understand the problem well enough either 🙃
There was a problem hiding this comment.
legacy code (we can either do this from scratch or keep this)
added 5 commits
December 20, 2024 14:08
- Didn't realize the internal _singleSwap method was being used for the other single swaps
- All methods are now checked by default. Merge checked and unchecked internal methods for simplicity. - Also remove the remaining _executeAction which was only used for batch execution, as well as the ActionType.
- The internal was useful in the case where we would want to reuse the methods for several different routers (i.e. Fusion, UniswapX, Cowswap, and Frontend). Now - this is not the case for the forseeable future. So, let's keep this as simple as possible and avoid premature overgeneralization.
accidental delete
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is mostly based off of
BatchSwapRouterV1. Why?exactOutinSwapRouterMethodsV1Executors/Verifiers
Fees
TODO
- MAKE CHANGES FROM MAX'S AUDIT