balancer / balancer-v3-monorepo

GNU General Public License v3.0
21 stars 1 forks source link

Deterministic pool addresses #60

Open jubeira opened 1 year ago

jubeira commented 1 year ago

Pool factories should use CREATE2, so that their address can be calculated before its creation.

The salt should probably include the chain ID and a nonce somewhere to make unique addresses. E.g. salt = hash(chainId, nonce++);.

jubeira commented 7 months ago

In addition, we'll want canonical core pool addresses to be determined from the token addresses on-chain. Get from token A, B —> pool address.

TBD: what happens if the canonical pool is deprecated and has to be migrated?

0xng commented 3 months ago

I see the current implementation of BasePoolFactory uses CREATE3 and takes into account both msg.sender and chainid, is the idea of moving to Create2 as the issue suggests something to be done, or did you guys settle for CREATE3 internally?

If this still needs to be done, could you tell me what you consider to be a canonical core pool so I can begin implementing this?

wei3erHase commented 3 months ago

is nonce really necessary? it will make the pool address non deterministic (if frontrunned the pool ends up in a different address)

jubeira commented 3 months ago

I see the current implementation of BasePoolFactory uses CREATE3 and takes into account both msg.sender and chainid, is the idea of moving to Create2 as the issue suggests something to be done, or did you guys settle for CREATE3 internally?

Create3 should be fine; I think we didn't know about it when we first opened the ticket.

If this still needs to be done, could you tell me what you consider to be a canonical core pool so I can begin implementing this?

Roughly speaking, the idea was to have 'canonical' factories that would produce e.g. 80/20 that would concentrate the liquidity for those tokens. Then, knowing just the token addresses you'd be able to query onchain what that pool is. For that to happen you'd probably need the factory address, which can recompute the address given to a certain pool based on its tokens. But again, we can probably say something like "for these two tokens, this is the canonical pool for a given factory"; the question is how to address pool migrations.

is nonce really necessary? it will make the pool address non deterministic (if frontrunned the pool ends up in a different address)

The question is how to address deprecations. We might need some sort of registry to query which is the current active factory, but it's not 100% clear to me. I'll circle back on this.

jubeira commented 3 months ago

Ok, let's forget about the nonce. Canonical pool factories should be e.g. 80/20 or 50/50 weighted pools, and should only produce a pool for a given pair / weight combination. Then, you should be able to ask the factory what the pool is for a given token pair. This would be the requirement.

In principle it shouldn't make sense to create two pools from the same factory with same tokens / weights. In the case of a migration, the factory would be deprecated and a new one would be created. External actors would still need to know which is the current active pool factory, but in principle that's fine.

If you think I'm missing anything, please let me know.

0xng commented 3 months ago

Perfect! Started thinking about it and some questions came to mind.

EndymionJkb commented 3 months ago

Yes; users normally need to sort the tokens beforehand. Our factories that take individual tokens vs. arrays do this automatically: e.g., 80/20 and buffer pools.

jubeira commented 3 months ago

This one is also to double-check. For the weighted pools with many tokens and weights the tokens won't be enough to compute the address given that there can exist pools with the same tokens but different weight permutations, so adding the weights into the mix will probably be necessary if I'm not missing anything. Is this correct?

Let's scope this feature to the simple cases for now: 80/20s and 50/50s. In that case there are only 2 combinations for a given token pair in the former, and only 1 possible case in the latter. But yeah, in the generic sense you could throw in the weights and solve the problem.

I find the tidiest way is to add the state variable and getter function straight into the <...>Factory contract instead of the BasePoolFactory. Do you agree with this, or should I find a way to standardize everything into a single function used in the 'BasePoolFactory`?

That sounds fine to me. The feature doesn't need to be 100% perfect to start with; we can have a base implementation and then iterate on it if we find itches to scratch :)

jubeira commented 1 month ago

We now have one more dimension to this problem, which is pools with hooks. In the current architecture, you can plug a hook to any pool, changing its behavior; it's no longer limited by the pool factory.