-
Notifications
You must be signed in to change notification settings - Fork 142
Support for MPT-DEX in Explorer #1302
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
base: main
Are you sure you want to change the base?
Changes from 14 commits
0b8367b
cd9ff32
9453ff8
67626a2
13c81a6
54cd50a
5fc004f
5782f14
d31ee18
8ac00cd
ec33f9c
805e630
7498ce0
72a44ea
976236f
a3641a8
a8f1f71
ad688b6
416846f
ab4553c
d14cd92
d749048
599e3bc
c82d0c3
d20a0c4
78c4164
bcf8cff
ddf45fc
4a58b18
67ba204
08e5675
f18ce99
2a15173
b02b7c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,28 @@ import { Account } from '../../../shared/components/Account' | |
| import Currency from '../../../shared/components/Currency' | ||
| import type { MetaRenderFunctionWithTx, MetaNode } from './types' | ||
|
|
||
| const normalize = (value: number | string, currency: string): string => | ||
| currency === 'XRP' ? (Number(value) / XRP_BASE).toString() : String(value) | ||
| const getCurrency = (takerAmount: any): string => { | ||
| if (takerAmount?.mpt_issuance_id) return takerAmount.mpt_issuance_id | ||
| return takerAmount?.currency || 'XRP' | ||
| } | ||
|
|
||
| const getIsMPT = (takerAmount: any): boolean => !!takerAmount?.mpt_issuance_id | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a specific type instead of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| const getIssuer = (takerAmount: any): string | undefined => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a specific type instead of any
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if (takerAmount?.mpt_issuance_id) return undefined | ||
| return takerAmount?.issuer | ||
| } | ||
|
|
||
| const normalize = ( | ||
| value: number | string, | ||
| currency: string, | ||
| isMPT: boolean = false, | ||
| ): string => { | ||
| if (isMPT) return String(value) | ||
| return currency === 'XRP' | ||
| ? (Number(value) / XRP_BASE).toString() | ||
| : String(value) | ||
| } | ||
|
|
||
| const renderChanges = ( | ||
| _t: any, | ||
|
|
@@ -22,14 +42,16 @@ const renderChanges = ( | |
| const meta: JSX.Element[] = [] | ||
| const final = node.FinalFields | ||
| const prev = node?.PreviousFields | ||
| const paysCurrency = final.TakerPays.currency || 'XRP' | ||
| const getsCurrency = final.TakerGets.currency || 'XRP' | ||
| const paysCurrency = getCurrency(final.TakerPays) | ||
| const getsCurrency = getCurrency(final.TakerGets) | ||
| const paysIsMPT = getIsMPT(final.TakerPays) | ||
| const getsIsMPT = getIsMPT(final.TakerGets) | ||
| const finalPays = final.TakerPays.value || final.TakerPays | ||
| const finalGets = final.TakerGets.value || final.TakerGets | ||
| const prevPays = prev?.TakerPays?.value || prev?.TakerPays | ||
| const prevGets = prev?.TakerGets?.value || prev?.TakerGets | ||
| const changePays = normalize(prevPays - finalPays, paysCurrency) | ||
| const changeGets = normalize(prevGets - finalGets, getsCurrency) | ||
| const changePays = normalize(prevPays - finalPays, paysCurrency, paysIsMPT) | ||
| const changeGets = normalize(prevGets - finalGets, getsCurrency, getsIsMPT) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks 67ba204 |
||
|
|
||
| if (prevPays && finalPays) { | ||
| const options = { ...CURRENCY_OPTIONS, currency: paysCurrency } | ||
|
|
@@ -39,7 +61,8 @@ const renderChanges = ( | |
| <b> | ||
| <Currency | ||
| currency={paysCurrency} | ||
| issuer={final.TakerPays.issuer} | ||
| issuer={getIssuer(final.TakerPays)} | ||
| isMPT={paysIsMPT} | ||
| displaySymbol={false} | ||
| /> | ||
| </b>{' '} | ||
|
|
@@ -50,7 +73,7 @@ const renderChanges = ( | |
| <b> | ||
| {{ | ||
| previous: localizeNumber( | ||
| normalize(prevPays, paysCurrency), | ||
| normalize(prevPays, paysCurrency, paysIsMPT), | ||
| language, | ||
| options, | ||
| ), | ||
|
|
@@ -60,7 +83,7 @@ const renderChanges = ( | |
| <b> | ||
| {{ | ||
| final: localizeNumber( | ||
| normalize(finalPays, paysCurrency), | ||
| normalize(finalPays, paysCurrency, paysIsMPT), | ||
| language, | ||
| options, | ||
| ), | ||
|
|
@@ -78,7 +101,8 @@ const renderChanges = ( | |
| <b> | ||
| <Currency | ||
| currency={getsCurrency} | ||
| issuer={final.TakerGets.issuer} | ||
| issuer={getIssuer(final.TakerGets)} | ||
| isMPT={getsIsMPT} | ||
| displaySymbol={false} | ||
| /> | ||
| </b>{' '} | ||
|
|
@@ -89,7 +113,7 @@ const renderChanges = ( | |
| <b> | ||
| {{ | ||
| previous: localizeNumber( | ||
| normalize(prevGets, getsCurrency), | ||
| normalize(prevGets, getsCurrency, getsIsMPT), | ||
| language, | ||
| options, | ||
| ), | ||
|
|
@@ -99,7 +123,7 @@ const renderChanges = ( | |
| <b> | ||
| {{ | ||
| final: localizeNumber( | ||
| normalize(finalGets, getsCurrency), | ||
| normalize(finalGets, getsCurrency, getsIsMPT), | ||
| language, | ||
| options, | ||
| ), | ||
|
|
@@ -123,11 +147,14 @@ const render: MetaRenderFunctionWithTx = ( | |
| ) => { | ||
| const lines: JSX.Element[] = [] | ||
| const fields = node.FinalFields || node.NewFields | ||
| const paysCurrency = fields.TakerPays.currency || 'XRP' | ||
| const getsCurrency = fields.TakerGets.currency || 'XRP' | ||
| const paysCurrency = getCurrency(fields.TakerPays) | ||
| const getsCurrency = getCurrency(fields.TakerGets) | ||
| const paysIsMPT = getIsMPT(fields.TakerPays) | ||
| const getsIsMPT = getIsMPT(fields.TakerGets) | ||
| const takerPaysValue = normalize( | ||
| fields.TakerPays.value || fields.TakerPays, | ||
| paysCurrency, | ||
| paysIsMPT, | ||
| ) | ||
| const invert = | ||
| CURRENCY_ORDER.indexOf(getsCurrency) > CURRENCY_ORDER.indexOf(paysCurrency) | ||
|
|
@@ -194,16 +221,22 @@ const render: MetaRenderFunctionWithTx = ( | |
| components={{ | ||
| Currency: ( | ||
| <Currency | ||
| currency={(invert ? getsCurrency : paysCurrency) || 'XRP'} | ||
| issuer={invert ? tx.TakerGets?.issuer : tx.TakerPays?.issuer} | ||
| currency={invert ? getsCurrency : paysCurrency} | ||
| issuer={ | ||
| invert ? getIssuer(tx.TakerGets) : getIssuer(tx.TakerPays) | ||
| } | ||
| isMPT={invert ? getsIsMPT : paysIsMPT} | ||
| displaySymbol={false} | ||
| shortenIssuer | ||
| /> | ||
| ), | ||
| Currency2: ( | ||
| <Currency | ||
| currency={(invert ? paysCurrency : getsCurrency) || 'XRP'} | ||
| issuer={invert ? tx.TakerPays?.issuer : tx.TakerGets?.issuer} | ||
| currency={invert ? paysCurrency : getsCurrency} | ||
| issuer={ | ||
| invert ? getIssuer(tx.TakerPays) : getIssuer(tx.TakerGets) | ||
| } | ||
| isMPT={invert ? paysIsMPT : getsIsMPT} | ||
| displaySymbol={false} | ||
| shortenIssuer | ||
| /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a specific type for takerAmount instead of
any?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! thanks for the review. I can think of two alternatives for the question of type safety:
Wait for this PR to be merged -- this provides for
MPTAmountas a variant of theAmounttype in xrpl package.Introduce an interface in all these files that simulates the true
Amounttype i.e.Option-1 is more type-safe because 'xrpl' package includes additional validation for the different kinds of Amount -- IOU, MPT or XRP.
What do you suggest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can add the shared interface in option 2 and add a TODO comment to use
xrplpackage when that PR is merged and released so we don't have to waitThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a8f1f71