Skip to content

Uniswap like SC #260

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

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open

Uniswap like SC #260

wants to merge 21 commits into from

Conversation

ozsefw
Copy link

@ozsefw ozsefw commented Jan 3, 2025

Implement a uniswap like LP in qubic

In the SC, users can create pools with two kind of asset, native qu and asset (like erc20)
users is able to add liqudity and remove liqudity from the pool,
the apis can be used in the contracts are following:

CreatePool

create a liqudity pool which can hold both native qu and asset

AddLiqudity, RemoveLiqudity

users can add/remove liqudity to/from the pool

SwapExactQuForAsset, SwapQuForExactAsset, SwapExactAssetForQu, SwapAssetForExactQu

users can swap in the pool by AMM with differnt requirement

SwapFee

query the protocol swap fee

GetPoolBasicState

get the pool state, such as totoal liqudity and reserve amount of native qu and asset

GetLiqudityOf (Not avaialbe currently)

QuoteExactQuInput, QuoteExactQuOutput, QuoteExactAssetInput, QuoteExactAssetOutput

get the amountIn/amountOut before swap

@Franziska-Mueller
Copy link
Collaborator

First of all, thank you for your contribution!
Please make sure to follow our guidelines for smart contract development: https://github.com/qubic/core/blob/main/doc/contracts.md
Currently, your PR does not conform with them (e.g. no changes to other files in src allowed)

Copy link
Contributor

@krypdkat krypdkat left a comment

Choose a reason for hiding this comment

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

Will add more comments later

Copy link
Contributor

@philippwerner philippwerner left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I have some comments now and will continue reviewing later.

2. use qx.issueAssetFee and qx.transferFee
3. enable code for liqudity for individual
4. test for token issuance, pool creatation, add/remove liqudity, 4 swap interface
Copy link
Contributor

@krypdkat krypdkat left a comment

Choose a reason for hiding this comment

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

I have some comments about coding style and syntax, the rest look good to me. We can proceed proposal->ipo->going live after you addressing all comments.

Disclaimer: Ensuring correctness of the code and security of the funds in the contract is the responsibility of the team developing this contract. I (as a core dev) don’t have enough time for understanding all details and pentesting the SC. It's not the core devs responsibility to make sure this SC 100% bug-free but we do our best to give you the advices to avoid bugs based on our experience.
And my last advice is triple-checking everything before going live.

Copy link
Contributor

@philippwerner philippwerner left a comment

Choose a reason for hiding this comment

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

Some comments. The ones about compliance with licenses are very important IMO. I recommend to talk with @J0ET0M , because they may require some high-level decisions to be made.

@philippwerner philippwerner changed the base branch from main to develop March 3, 2025 18:10
@philippwerner
Copy link
Contributor

Please don't rebase and force push, but merge the current develop of the official repo into your branch in the future.

ozsefw added 5 commits March 5, 2025 12:49
2. remove dependency of <utilith> in uint128.h
3. add prefix for test in uint128
4. fix some misuse of NULL
2. fix some merge issue
@ozsefw
Copy link
Author

ozsefw commented Mar 6, 2025

Please don't rebase and force push, but merge the current develop of the official repo into your branch in the future.

Thanks for your feedback on the code review, I've just update the code, mainly the following items:

  1. Add the information about the license for uin128, I'm wondering if my solution is ok
  2. Remove the dependence in uint128.h
  3. Merge current develop into this branch

@krypdkat
Copy link
Contributor

krypdkat commented Mar 8, 2025

@J0ET0M can you take a look at license thing for uint128

Copy link
Contributor

@philippwerner philippwerner left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments. I have two minor requests additional. After addressing them, you can proceed with the proposal.

Please note the disclaimer written by dkat above. We recommend triple-checking everything before going live, because ensuring correctness of the code and security of the funds in the contract is the responsibility of the team developing this contract (NOT of the core devs reviewing and providing feedback).

@philippwerner
Copy link
Contributor

Please note that the syntax of how contract procedures and functions are defined will change with the next release. See #361 for details.

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.

5 participants