-
Notifications
You must be signed in to change notification settings - Fork 74
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:PoolManager Interface Implementation v3 #102
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -1,35 +1,115 @@ | |||
// SPDX-License-Identifier: GPL-2.0-or-later | |||
// Author: Eli Lee |
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.
// Author: Eli Lee |
代码里面就不体现 Author 了。
|
||
function getPoolInfo( | ||
address token0, | ||
address token1, | ||
uint24 fee | ||
) external view override returns (PoolInfo memory poolInfo) {} | ||
) external view override returns (PoolInfo memory poolInfo) { |
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.
getPoolInfo 第三个参数 fee 应该要改为 index,后面调整过设计,每个代币对下面可以有多个 pool,通过 index 来索引,每个 pool 的 fee 和价格上下限都可以不一样
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.
#105 可以先看看这个 PR,先修复了初始化代码中之前漏掉的几个问题
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.
getPoolInfo 第三个参数 fee 应该要改为 index,后面调整过设计,每个代币对下面可以有多个 pool,通过 index 来索引,每个 pool 的 fee 和价格上下限都可以不一样
我觉得这里想通过代码索引到池子得话这个可以通过初始化池子得方式将代币和池子关联索引。这样是否更合理一点
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.
#107 可以看一下这个,我重新梳理了一下,PoolManager 直接继承 Factory,接口重新设计了一下。
|
||
contract PoolManager is IPoolManager { | ||
address public factory; |
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.
用 address 应该是没问题的,我看了 v2 https://github.com/Uniswap/v2-periphery/blob/master/contracts/UniswapV2Router01.sol#L12C39-L12C46 和 v3 https://github.com/Uniswap/v3-periphery/blob/main/contracts/base/PeripheryImmutableState.sol#L10 的类似的场景,都是存的 address。
不过还加了 immutable
,我们也可以加上,应该是可以有一些优化节约点 gas。
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.
直接存地址的方式会更节省gas但是差别不是很大,存实例的话会更加可读一点。存入实例也会多做一步静态检查。两种方式都可以。
|
||
function getPoolInfo( | ||
address token0, | ||
address token1, | ||
uint24 fee | ||
) external view override returns (PoolInfo memory poolInfo) {} | ||
) external view override returns (PoolInfo memory poolInfo) { | ||
address poolAddress = IFactory(factory).getPool(token0, token1, fee); |
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.
最新的代码 getPool 的第三个参数是 index 了。
} | ||
|
||
// 创建 pool | ||
pool = IFactory(factory).createPool(params.token0, params.token1, params.fee); |
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.
同上,fee 应该要改为 index
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.
fee 应该还是需要的,每个 Pool 都有自己的 fee,只是看是否需要加 index。
现在的逻辑是这样:每个代币对下面会有多个 Pool,每个 Pool 都有各自的 fee 和 tickLower 和 tickUpper,在 PoolManager 里面同一个代币对下面的 Pool 会放到一个数组中,所以也可以通过 index 索引到。
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.
初始化池子时候获取池子方法(getPool)也需要传入index去获得这里的token0、tokne1、index如果通过这三个参数作为参数获取池子判断池子是否存在会变得麻烦,没有通过token0、tokne1、fee 能保证池子会有唯一性。
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.
token0、tokne1、fee、tickLower、tickUpper,这五个才能保证池子唯一
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.
看了一下,应该用 getPools 来拿到一个代币对下面所有的池子来判断
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.
Factory 的接口应该是有点问题的,这个 createPool 和 getPool 参数应该都要改为 token0、token1、fee、tickLower、tickUpper,这个五个值确认一个池子
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.
我觉得池子的唯一性用token0、tokne1、fee这三个参数就可以决定。 tickLower、tickUpper只是定义了可提供流动性的头寸不应该作为池子唯一性的判断条件。
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.
我们现在的设计是把每个池子都单独定义了 tickLower、tickUpper,这个两个和 fee 一样是池子的属性,和 V3 的逻辑不太一样。可以参考 https://github.com/WTFAcademy/WTF-Dapp/blob/main/P003_OverallDesign/readme.md 这里的说明
IPool(pool).initialize(params.sqrtPriceX96, params.tickLower, params.tickUpper); | ||
|
||
// 添加 PoolKey 到 poolKeys 数组 | ||
PoolKey memory poolKey = PoolKey({ |
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.
这个 memory 是必须的吗?
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.
结构体必须指定类型不然编译器会报错:
TypeError: Data location must be "storage", "memory" or "calldata" for variable, but none was given.
--> PoolManager.sol:94:9:
|
94 | PoolKey key = PoolKey({
| ^^^^^^^^^^^
}); | ||
poolKeys.push(poolKey); | ||
|
||
// 检查并添加代币到 tokens 数组 |
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.
有没有必要搞一个 tokenExists 来装满检测 token 是否存在?可以探讨一下
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.
嗯,这个还好。其实最好的办法是这些信息应该是存服务端就好了,Uniswap 其实就是这么做的,这个合约其实不是必须的。写具体课程内容的时候要提醒一下,避免误导开发者。非必要的内容完全 Web2 的中心化服务器存储就行。
最新的代码在 #114 |
基于 pr #97 做了部分修改: