Skip to content

full support for all amm transactions #503

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 8 commits into
base: main
Choose a base branch
from

Conversation

pandablue0809
Copy link
Member

@pandablue0809 pandablue0809 requested a review from ihomp July 30, 2025 20:56
@ihomp
Copy link
Member

ihomp commented Aug 4, 2025

Are the links for all the tx types so we can test? @Anna15170221 @pandablue0809?

@ihomp
Copy link
Member

ihomp commented Aug 4, 2025

http://localhost:3000/en/tx/9CA9CDEBCBE61C19E6C81A186BFB55174798BBE989CB14EBE70F72F3CD4B8B16

It's not super clear here
That user created an AMM with asset 1 and asset 2 and the user received back:
0.316227766016838 BTH/XRP LP.

When TX is fully supported, we remove the "Affected accounts."
This means we need to ensure the user can see how much LP was received and what AMM address was created.

http://localhost:3000/en/tx/AA6883AF2858B09FDFAC3D55BDAF8797D91D114DCBF5D111FE95773F2B184D08

here the issue is not recognised for BTH, but we have a service name
And we don't need asset1 and asset2 , as there are no amounts
it's just a pair
Liquidity pool: XRP/BTH (Bithomp)

I need examples of all the other transaction types to check.

Copy link
Member

@ihomp ihomp left a comment

Choose a reason for hiding this comment

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

we need some improvements

@ihomp
Copy link
Member

ihomp commented Aug 4, 2025

AMM create:
http://localhost:3000/en/tx/C40A50326804C6F913C9C0025D180B76D7F38CE2A18865A05098AE522DC0DAA9

  • I've added comments; land et's call Asset -> Asset 1

AMM Vote
http://localhost:3000/en/tx/B888C6FA3D28D6861BB7DF6D4D4049BE656DCBF43290A9A972BE2B3F9E40EDDA
I've already added comments

AMM Deposit
http://localhost:3000/en/tx/468C14A0A238831E86E34395AFB1C084FD6C3FC3130DDE168C2F6D34BE242CDA
Asset, Asset2, Amount, Amount 2 - looks confusing
It must be just Asset 1 and Asset 2, but each asset should have currency in it.

  • doesn't show how much LP was received.. and LP should show the pair, like "XRP / USDC (Circle)"

AMM Withdraw http://localhost:3000/en/tx/3B80A23644F338D72AEAD923C116BB66A0C302C60A50DE9EA4DB5F0469DA84B1
same as for teh Deposit

AMM Bid
http://localhost:3000/en/tx/BDD2E8993D8EE8E9544C13A73238CD225BFA4756B7BC9D00307EF08B99E17EC3
same as in Vote, we need to show the Pool Pair as "YAWAY (rDivh3...6yAWAY) / XRP " instead of asset/asset 2
if bid max and bid min are the same, may be we can just say BID? and that is the bid that was specified in transaction.. and we nbeed to show teh actual bid which was taken.. it is in balance chnages which is actually the same.

AMM Delete
http://localhost:3000/en/tx/A5CE8BDD1C4DE58A03B50CDD5EB0F964FAF57A103F5B6D08BF905DED260EEE33
this is a failed tx @Anna15170221 , do we have one with success?

  • we need to show Pool pair instead of asset, asset2

AMM Clawback
DEVNET NETWORK
http://localhost:3000/en/tx/FAACBEE4FAB86887FB89DC362DF237A1C2081AB68910EB59AF1EDC5BA8A4FFAF
its hard to understand what is happened in here - it should be more clear as well.. what is the pool, what assets were clawed.

@Anna15170221
Copy link
Contributor

@ihomp
I couldn't find successful AMMDelete

@ihomp
Copy link
Member

ihomp commented Aug 7, 2025

we can wait for @ildaruz to add ammChanges - that would make calculations easier.. but in general:

  1. We need to show how much was deposited and how much was received.
    http://localhost:3000/en/tx/9CA9CDEBCBE61C19E6C81A186BFB55174798BBE989CB14EBE70F72F3CD4B8B16
    http://localhost:3000/en/tx/C40A50326804C6F913C9C0025D180B76D7F38CE2A18865A05098AE522DC0DAA9

Deposited: 0.01 XRP and 0.00001 BTH (Bithomp).
Received: 0.316227766016838 LP XRP/BTH (Bithomp)

  1. Liquidity pool should be identified
    http://localhost:3000/en/tx/AA6883AF2858B09FDFAC3D55BDAF8797D91D114DCBF5D111FE95773F2B184D08
    http://localhost:3000/en/tx/B888C6FA3D28D6861BB7DF6D4D4049BE656DCBF43290A9A972BE2B3F9E40EDDA

Liquidity pool: XRP / BTH (Bithomp)

http://localhost:3000/en/tx/468C14A0A238831E86E34395AFB1C084FD6C3FC3130DDE168C2F6D34BE242CDA
why do we need LP Token Balance here?

we need to show:
Deposited: 0.101711 XRP and 0.3 USDC (Circle)
Received: 173.2388841 XRP / USDC (Circle)

Specified Max Assets for Deposit: 0.3 XRP and 0.3 USDC (Circle)

0.101711 XRP is calculated from balance changes and the fee.

AMM Withdraw http://localhost:3000/en/tx/3B80A23644F338D72AEAD923C116BB66A0C302C60A50DE9EA4DB5F0469DA84B1
we need to show how much it was specified in transaction
we need to show how much was actually paid and how much was actually withdrawn (excluding the fee, if paid by the receiver)

AMM Bid
http://localhost:3000/en/tx/BDD2E8993D8EE8E9544C13A73238CD225BFA4756B7BC9D00307EF08B99E17EC3
same as in Vote, we need to show the Pool Pair as "YAWAY (rDivh3...6yAWAY) / XRP " instead of asset/asset 2
if bid max and bid min are the same, may be we can just say BID? and that is the bid that was specified in transaction.. and we nbeed to show teh actual bid which was taken.. it is in balance chnages which is actually the same.

AMM Delete
http://localhost:3000/en/tx/A5CE8BDD1C4DE58A03B50CDD5EB0F964FAF57A103F5B6D08BF905DED260EEE33
this is a failed tx @Anna15170221 , do we have one with success?

we need to show Pool pair instead of asset, asset2
AMM Clawback
DEVNET NETWORK
http://localhost:3000/en/tx/FAACBEE4FAB86887FB89DC362DF237A1C2081AB68910EB59AF1EDC5BA8A4FFAF
its hard to understand what is happened in here - it should be more clear as well.. what is the pool, what assets were clawed.

@ihomp
Copy link
Member

ihomp commented Aug 14, 2025

@pandablue0809 we already have ammChanges in the output, check if you can show them to finish this PR.

@pandablue0809 pandablue0809 requested a review from ihomp August 14, 2025 14:22
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.

3 participants