explorer icon indicating copy to clipboard operation
explorer copied to clipboard

XLS-33 Multi Purpose token

Open shawnxie999 opened this issue 1 year ago • 10 comments

High Level Overview of Change

  • add MPT transactions: MPTokenIssuance, MPTokenIssuanceDestroy, MPTokenAuthorize, MPTokenIssuanceSet
  • add MPT page
  • support search by MPTID
  • updates transactions: Payment, Clawback
  • modified Currency to support MPTID

Context of Change

Spec: https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0033d-multi-purpose-tokens

MPT page image

Ledger view image

MPTokenIssuance image

MPTokenAuthorize image

MPTokenIssuanceSet image

Payment (with scaling MPT amount) image

Clawback image

Type of Change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Refactor (non-breaking change that only restructures code)
  • [ ] Tests (You added tests for code that already exists, or your new feature included in this PR)
  • [ ] Documentation Updates
  • [ ] Translation Updates
  • [ ] Release

TypeScript/Hooks Update

  • [ ] Updated files to React Hooks
  • [ ] Updated files to TypeScript

Before / After

Test Plan

shawnxie999 avatar May 30 '24 14:05 shawnxie999

Noticed 2 MPT Issuance IDs on top of each other in the screenshot for MPT page

pdp2121 avatar Jun 17 '24 20:06 pdp2121

Noticed 2 MPT Issuance IDs on top of each other in the screenshot for MPT page

That's because it's a tooltip for the ID - similar to the NFT page

shawnxie999 avatar Jun 21 '24 19:06 shawnxie999

I'm not sure which way is better: the current way of showing scaled amount, or just showed the amount divided by 10^scale (so that on payment and clawback simple page we dont have to click on the token itself to find out about the scale). Other than that, looks good to me.

pdp2121 avatar Jul 24 '24 16:07 pdp2121

The failed tests can be fixed when merging latest changes from staging

pdp2121 avatar Jul 24 '24 16:07 pdp2121

I'm not sure which way is better: the current way of showing scaled amount, or just showed the amount divided by 10^scale (so that on payment and clawback simple page we dont have to click on the token itself to find out about the scale). Other than that, looks good to me.

Personally, I'd rather see the scaled amount instead of having to do the math myself.

mvadari avatar Jul 24 '24 17:07 mvadari

I'm not sure which way is better: the current way of showing scaled amount, or just showed the amount divided by 10^scale (so that on payment and clawback simple page we dont have to click on the token itself to find out about the scale). Other than that, looks good to me.

Personally, I'd rather see the scaled amount instead of having to do the math myself.

@mvadari @pdp2121 One problem with showing the amount divided by 10^scale that is the information of the AssetScale is stored in the MPTokenIssuance object. This means that whenever MPT amount is display, we would need to make an extra call using the ledger_entry API to fetch the MPTokenIssuance object to get the scale.

shawnxie999 avatar Aug 21 '24 17:08 shawnxie999

This means that whenever MPT amount is display, we would need to make an extra call using the ledger_entry API to fetch the MPTokenIssuance object to get the scale.

I think that's fine.

mvadari avatar Aug 22 '24 14:08 mvadari

This means that whenever MPT amount is display, we would need to make an extra call using the ledger_entry API to fetch the MPTokenIssuance object to get the scale.

I think that's fine.

Yes. I would prefer seeing the scaled amount. We are also showing scaled amount in PriceOracles tx.

pdp2121 avatar Aug 26 '24 21:08 pdp2121

@pdp2121 @mvadari

updated scaling in this commit

it now looks like this image image

shawnxie999 avatar Sep 03 '24 17:09 shawnxie999

LGTM. Just left a comment and may be get the linting fixed

Not quite sure why the lint fails, the ones that are failing are the ones that my local linter told me to fix.

shawnxie999 avatar Sep 05 '24 17:09 shawnxie999