Refactor/fit periphery #9
Conversation
.gitmodules
Outdated
| [submodule "lib/forge-std"] | ||
| path = lib/forge-std | ||
| url = https://github.com/foundry-rs/forge-std |
There was a problem hiding this comment.
Do you think it might be a good idea if we keep this forge-std dependency?
1/ what we are doing means that if we somehow need to update forge-std to get newer feature, we are forced to rely on dependency -- where we might not want to touch v4-core and v4-periphery since the code has frozen
2/ and so far i think forge-std have not caused us any conflicts yet
3/ we do this for oz to prevent conflicts due to breaking changes
There was a problem hiding this comment.
same for forge-gas-snapshot probably
|
should fix #8 |
| @@ -11,12 +11,7 @@ optimizer_runs = 1000 | |||
| via_ir = true | |||
| evm_version = 'cancun' | |||
| ffi = true | |||
| fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}] | |||
There was a problem hiding this comment.
I've put the remapping settings into remappings.txt
There was a problem hiding this comment.
can also consider using UniversalRouter here if u guys prefer
There was a problem hiding this comment.
yeh.. ideally it should be MockUniversalRouter i guess.. better to educate just universal router instead of universal router + swap router
(though not urgent)
There was a problem hiding this comment.
cool, will handle it in separate PR. i've added an issue to track this.
| BinPositionManager(_vault, _binPoolManager, _permit2) | ||
| {} | ||
|
|
||
| function addLiquidity(IBinPositionManager.BinAddLiquidityParams calldata params) |
There was a problem hiding this comment.
interesting.. i wonder if we should ask the hook dev to use this kind of contract
context: they can't use universal to swap as universal router have re-entrancy guard
| // Vm.Log[] memory entries = vm.getRecordedLogs(); | ||
| // // find IBinFungibleToken.TransferBatch | ||
| // for (uint256 i = 0; i < entries.length; i++) { | ||
| // if (entries[i].topics[0] == keccak256("TransferBatch(address,address,address,uint256[],uint256[])")) { | ||
| // (tokenIds, liquidityMinted) = abi.decode(entries[i].data, (uint256[], uint256[])); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
should we delete this or its purposely meant to be commented?
Some necessary updates to fit the v4-periphery