Skip to content

[AMM pool] Changed approvals + locking logic #1771

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

Merged
merged 8 commits into from
Sep 21, 2020

Conversation

Brechtpd
Copy link
Contributor

  • Removed queue, pool transactions can be done in any order (but exits need to be processed).
  • All tokens deposited to the pool contract are locked by default. Users can withdraw immediately with the help of a signature of the operator. They can also do an Ethereum transaction themselves to unlock the funds with a delay.
  • Onchain join/exit only to be used for censorship resistance/onchain composability, otherwise signatures are better.
  • Transferring the liquidity tokens can now easily be supported (just have to make it an ERC20 token and everything will work as expected). Deposit/Withdraw can be used to transfer them in and out.

This is the basic idea, still room for improvement and probably some bugs but will wait on feedback if this setup is okay.

Copy link
Contributor

@dong77 dong77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tokens deposited to the pool contract are locked by default. Users can withdraw immediately with the help of a signature of the operator. They can also do an Ethereum transaction themselves to unlock the funds with a delay.

Can we make join part of the deposit operation so users don't have to do it using another tx? Or just add another parameter to enable auto join/exit.

for (uint i = 0; i < tokens.length; i++) {
address token = tokens[i].addr;
for (uint i = 0; i < tokens.length + 1; i++) {
address token = (i < tokens.length) ? tokens[i].addr : address(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if token == address(this), then the safeTransferFromAndVerify will fail, won't it?

@@ -204,17 +211,18 @@ contract AmmPool is IBlockReceiver, IAgent {
}
}

/// @param amounts The amounts to deposit + the amount of pool tokens to deposit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand: why do we support depositing pool tokens (liquidity tokens)?

@dong77
Copy link
Contributor

dong77 commented Sep 19, 2020

Deposit/Withdraw can be used to transfer them in and out.
I'd suggest we don't do it in this PR.

@dong77
Copy link
Contributor

dong77 commented Sep 19, 2020

Brecht, please go ahead with this implementation.

@Brechtpd Brechtpd marked this pull request as ready for review September 21, 2020 17:02
@Brechtpd Brechtpd merged commit 49dccd2 into protocol36-ammpool Sep 21, 2020
@Brechtpd Brechtpd deleted the protocol36-ammpool-rmqueue branch September 21, 2020 17:03
Brechtpd added a commit that referenced this pull request Sep 22, 2020
* [protocol 3.6] AMM pool testing

* [protocol 3.6] More AMM pool testing

* [protocol 3.6] Improved AMM pool + testing

* [protocol 3.6] Small optimizations

* [protocol 3.6] Misc improvements/fixes AMM pool

* unify submitBlocksCompressedWithCallbacks (#1761)

* unify submitBlocksCompressedWithCallbacks

* unify submitBlocksCompressedWithCallbacks

* Amm approve erc20 on setup (#1762)

* approve upon setup

* approve upon setup

* allow withdraw all tokens (#1763)

* make agent explicitly inherit IAgent (#1765)

* [protocol 3.6] Small fixes

* [protocol 3.6] uint96 safe math (#1766)

* [AMM Pool] Rename variables and extract methods (#1772)

* rename variables

* extract methods

* extract methods

* [AMM Pool] Some small improvements (#1773)

* add changes based on my questions

* more

Co-authored-by: wangdong <wangdong@zhongan.io>
Co-authored-by: Brechtpd <Brechtp.Devos@gmail.com>

* [AMM pool] Changed approvals + locking logic (#1771)

* [protocol 3.6] Changed approvals + locking logic

* [protocol 3.6] LP tokens

* [protocol 3.6] Use minPoolAmountOut for joins

* [protocol 3.6] AMM pool signature assisted L1 withdrawals + fixes

* [protocol 3.6] Small improvements/fixes

Co-authored-by: Daniel Wang <daniel@loopring.org>
Co-authored-by: wangdong <wangdong@zhongan.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants