-
Notifications
You must be signed in to change notification settings - Fork 216
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
remove NFT protocols for V4 router #348
Conversation
contracts/UniversalRouter.sol
Outdated
@@ -79,6 +60,6 @@ contract UniversalRouter is IUniversalRouter, Dispatcher, RewardsCollector { | |||
return command & Commands.FLAG_ALLOW_REVERT == 0; | |||
} | |||
|
|||
/// @notice To receive ETH from WETH and NFT protocols | |||
/// @notice To receive ETH from WETH protocols |
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.
I think we can finally add this in now?
require(msg.sender == WETH9, 'Not WETH9');
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.
that's been a huge thorn in our side
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.
we'll probably need to check that it's either WETH or v4 PoolManager
expect(miladyBalanceAfter.sub(miladyBalanceBefore)).to.eq(numMiladys) | ||
}) | ||
}) | ||
it('reverts if no commands are allowed to revert') |
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.
unrun test cases?
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.
yeeah need to completely rewrite them as they were all written for NFTs
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.
can we add that receive() external payable {}
change???
* solc upgrade to 0.8.26 * use mainnet permit2 work started * fix uniswap tests * Remove block from resetFork * Refactor to fetch fee tiers * remove NFT protocols for V4 router (#348) * first pass * fix forge builds * fix reentrancy test * Add check to receive
* update hardhat * remove symlinks by using hardhat-foundry use hardhat-foundry package to use foundry remappings to compile. Symlinks are no longer necessary After upgrade HH412 error was thrown caused by the existing symlinks ref: https://hardhat.org/hardhat-runner/docs/errors#HH412 NomicFoundation/hardhat#3623 * update compiler version to ^0.8.24 supporting the cancun upgrades * fix ci * fix lock file * Update yarn.lock * regenerate gas snapshots * install foundry in ci * Use mainnet permit2 (#347) * solc upgrade to 0.8.26 * use mainnet permit2 work started * fix uniswap tests * Remove block from resetFork * Refactor to fetch fee tiers * remove NFT protocols for V4 router (#348) * first pass * fix forge builds * fix reentrancy test * Add check to receive * remove .only rip * add todo for tests that need wrtiting * update readme and planner --------- Co-authored-by: Alice <[email protected]> Co-authored-by: Alice Henshaw <[email protected]>
Remove NFT protocols to prepare for a new version that supports uniswap v2, v3 and v4