Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add foreign assets support on asset hubs, such as MYTH on PAH #1577

Merged
merged 16 commits into from
Oct 6, 2024
Merged

feat: add foreign assets support on asset hubs, such as MYTH on PAH #1577

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
22c373b
feat: add foreign assets support on asset hubs, such as MYTH on PAH
Nick-1979 Oct 3, 2024
0f81f37
refactor: handle type issues
Nick-1979 Oct 5, 2024
fa547aa
fix: asset id type issues
Nick-1979 Oct 5, 2024
97a26b8
refactor: set different asset ids for native assets on asset hubs vs.…
Nick-1979 Oct 6, 2024
57b3a36
Merge branch 'main' into addMythAsForeignAsset
Nick-1979 Oct 6, 2024
e224376
chore: adjust spacings
Nick-1979 Oct 6, 2024
98e971c
chore: update news
Nick-1979 Oct 6, 2024
bc5e750
refactor: handle asset id type
Nick-1979 Oct 6, 2024
cbe417c
chore: adjust decimal point for crypto as currency
Nick-1979 Oct 6, 2024
e13d209
docs: update news
Nick-1979 Oct 6, 2024
70eca03
refactor: use destruct
Nick-1979 Oct 6, 2024
cf88550
fix: handle saved asset id
Nick-1979 Oct 6, 2024
c26d3d4
chore: update comment
Nick-1979 Oct 6, 2024
1ad4c50
refactor: add try catch
Nick-1979 Oct 6, 2024
4b37252
refactor: memorize parsedAssetId
Nick-1979 Oct 6, 2024
b628f90
chore: remove unused
Nick-1979 Oct 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"@polkadot/types-support": "^11.2.1",
"@polkadot/util": "^12.6.2",
"@polkadot/util-crypto": "^12.6.2",
"@polkagate/apps-config": "^0.140.3",
"@polkagate/apps-config": "^0.140.5",
"@substrate/connect": "^0.7.32",
"@vaadin/icons": "^23.2.3",
"babel-plugin-transform-import-meta": "^2.1.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/extension-polkagate/src/fullscreen/accountDetails/components/AssetSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface Props {
onChange: (value: number | string) => void;
label: string;
style: SxProps<Theme> | undefined;
assetId: number | undefined;
assetId: number | string | undefined;
Nick-1979 marked this conversation as resolved.
Show resolved Hide resolved
setAssetId: React.Dispatch<React.SetStateAction<number | undefined>>
}

Expand Down
2 changes: 1 addition & 1 deletion packages/extension-polkagate/src/fullscreen/accountDetails/components/CommonTasks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { popupNumbers } from '..';

interface Props {
address: string | undefined;
assetId: number | undefined;
assetId: number | string | undefined;
balance: BalancesInfo | FetchedBalance | undefined;
genesisHash: string | null | undefined;
setDisplayPopup: React.Dispatch<React.SetStateAction<number | undefined>>;
Expand Down
2 changes: 1 addition & 1 deletion packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export default function AccountDetails (): React.ReactElement {

const mayBeAssetIdSelectedInHomePage = assetId !== undefined ? assetId : parseInt(paramAssetId);

if (mayBeAssetIdSelectedInHomePage >= 0 && accountAssets) {
if (mayBeAssetIdSelectedInHomePage as number >= 0 && accountAssets) {
Nick-1979 marked this conversation as resolved.
Show resolved Hide resolved
const found = accountAssets.find(({ assetId, genesisHash: _genesisHash }) => assetId === mayBeAssetIdSelectedInHomePage && genesisHash === _genesisHash);

found && setSelectedAsset(found);
Expand Down
77 changes: 47 additions & 30 deletions packages/extension-polkagate/src/fullscreen/sendFund/InputPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import type { IconProp } from '@fortawesome/fontawesome-svg-core';
import type { SubmittableExtrinsicFunction } from '@polkadot/api/types';
import type { Balance } from '@polkadot/types/interfaces';
import type { HexString } from '@polkadot/util/types';
import type { BalancesInfo, DropdownOption, TransferType } from '../../util/types';
import type { Inputs } from '.';

Expand All @@ -22,15 +23,15 @@ import { useTranslation } from '../../components/translate';
import { useInfo, useTeleport } from '../../hooks';
import { getValue } from '../../popup/account/util';
import { ASSET_HUBS } from '../../util/constants';
import { amountToHuman, amountToMachine } from '../../util/utils';
import { amountToHuman, amountToMachine, decodeMultiLocation } from '../../util/utils';
import { openOrFocusTab } from '../accountDetails/components/CommonTasks';
import { toTitleCase } from '../governance/utils/util';
import { STEPS } from '../stake/pool/stake';

interface Props {
address: string;
balances: BalancesInfo | undefined;
assetId: number | undefined;
assetId: string | undefined;
inputs: Inputs | undefined;
setStep: React.Dispatch<React.SetStateAction<number>>;
setInputs: React.Dispatch<React.SetStateAction<Inputs | undefined>>;
Expand Down Expand Up @@ -99,6 +100,14 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
const { api, chain, formatted } = useInfo(address);
const teleportState = useTeleport(address);

const isForeignAsset = assetId && assetId.startsWith('0x');
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Simplify property access with optional chaining

In the assignment of isForeignAsset, you can use optional chaining to simplify the code and handle cases where assetId is undefined:

const isForeignAsset = assetId?.startsWith('0x');

This makes the code more concise and reduces the risk of errors.

Apply this diff:

-const isForeignAsset = assetId && assetId.startsWith('0x');
+const isForeignAsset = assetId?.startsWith('0x');
🧰 Tools
🪛 Biome

[error] 103-103: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


const parsedAssetId = assetId === undefined || assetId === 'undefined'
? undefined
: isForeignAsset
? decodeMultiLocation(assetId as HexString)
: parseInt(assetId);

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid checking for 'undefined' as a string

In the parsedAssetId assignment, the code checks if assetId equals 'undefined' as a string:

const parsedAssetId = assetId === undefined || assetId === 'undefined'
    ? undefined
    : isForeignAsset
        ? decodeMultiLocation(assetId as HexString)
        : parseInt(assetId);

Having 'undefined' as a string suggests that somewhere assetId may be set to the string 'undefined' instead of being undefined. This could lead to unexpected behavior. It's recommended to ensure assetId is either undefined or a valid string representing an asset ID.

Apply this diff to remove the string comparison and handle the potential issue at the source where assetId may be set to 'undefined':

-const parsedAssetId = assetId === undefined || assetId === 'undefined'
+const parsedAssetId = assetId === undefined
    ? undefined
    : isForeignAsset
        ? decodeMultiLocation(assetId as HexString)
        : parseInt(assetId);

const [amount, setAmount] = useState<string>(inputs?.amount || '0');
const [estimatedFee, setEstimatedFee] = useState<Balance>();
const [estimatedCrossChainFee, setEstimatedCrossChainFee] = useState<Balance>();
Expand Down Expand Up @@ -159,26 +168,34 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
return undefined;
}

const module = assetId !== undefined
? isAssethub(chain.genesisHash)
? 'assets'
: api.tx?.['currencies']
? 'currencies'
: 'tokens'
: 'balances';
try {
const module = assetId !== undefined
? isAssethub(chain.genesisHash)
? isForeignAsset
? 'foreignAssets'
: 'assets'
: api.tx?.['currencies']
? 'currencies'
: 'tokens'
: 'balances';
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor nested ternary operators for readability

The nested ternary operators used to determine the module variable could be simplified for better readability:

const module = assetId !== undefined
    ? isAssethub(chain.genesisHash)
        ? isForeignAsset
            ? 'foreignAssets'
            : 'assets'
        : api.tx?.['currencies']
            ? 'currencies'
            : 'tokens'
    : 'balances';

Consider refactoring using if-else statements or a more readable approach to enhance code clarity.

Suggested refactor:

let module: string;

if (assetId !== undefined) {
    if (isAssethub(chain.genesisHash)) {
        module = isForeignAsset ? 'foreignAssets' : 'assets';
    } else if (api.tx?.['currencies']) {
        module = 'currencies';
    } else {
        module = 'tokens';
    }
} else {
    module = 'balances';
}


if (['currencies', 'tokens'].includes(module)) {
return api.tx[module]['transfer'];
}

if (['currencies', 'tokens'].includes(module)) {
return api.tx[module]['transfer'];
}
return api.tx?.[module] && (
transferType === 'Normal'
? api.tx[module]['transferKeepAlive']
: assetId !== undefined
? api.tx[module]['transfer']
: api.tx[module]['transferAll']
);
} catch (e) {
console.log('Something wrong while making on chain call!', e);
Comment on lines +194 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling in the try-catch block

In the catch block, consider providing more informative error messages or handling specific exceptions to aid in debugging:

} catch (e) {
    console.error('Error determining on-chain call module:', e);
    return undefined;
}

Apply this diff:

-} catch (e) {
-  console.log('Something wrong while making on chain call!', e);
-  return undefined;
+} catch (e) {
+  console.error('Error determining on-chain call module:', e);
+  return undefined;
}


return api.tx?.[module] && (
transferType === 'Normal'
? api.tx[module]['transferKeepAlive']
: assetId !== undefined
? api.tx[module]['transfer']
: api.tx[module]['transferAll']
);
}, [api, assetId, chain, transferType]);
return undefined;
}
}, [api, assetId, chain, isForeignAsset, transferType]);
Nick-1979 marked this conversation as resolved.
Show resolved Hide resolved

const call = useMemo((): SubmittableExtrinsicFunction<'promise'> | undefined => {
if (!api) {
Expand Down Expand Up @@ -221,11 +238,11 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
const _params = assetId !== undefined
? ['currencies', 'tokens'].includes(onChainCall.section)
? [formatted, balances.currencyId, amount]
: [assetId, formatted, amount]
: [parsedAssetId, formatted, amount]
: [formatted, amount];

onChainCall(..._params).paymentInfo(formatted).then((i) => setFeeCall(i?.partialFee)).catch(console.error);
}, [api, formatted, balances, onChainCall, assetId]);
}, [api, formatted, balances, onChainCall, assetId, parsedAssetId]);

const crossChainParams = useMemo(() => {
if (!api || !balances || !teleportState || isCrossChain === false || (recipientParaId === INVALID_PARA_ID && !teleportState?.isParaTeleport) || Number(amount) === 0) {
Expand Down Expand Up @@ -288,7 +305,7 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
: assetId !== undefined
? ['currencies', 'tokens'].includes(onChainCall?.section || '')
? [recipientAddress, balances.currencyId, amountAsBN] // this is for transferring on mutliasset chains
: [assetId, recipientAddress, amountAsBN] // this is for transferring on asset hubs
: [parsedAssetId, recipientAddress, amountAsBN] // this is for transferring on asset hubs
: transferType === 'All'
? [recipientAddress, false] // transferAll with keepalive = false
: [recipientAddress, amountAsBN]) as unknown[],
Expand All @@ -297,7 +314,7 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
recipientGenesisHashOrParaId: recipientChainGenesisHash,
totalFee: estimatedFee ? estimatedFee.add(estimatedCrossChainFee || BN_ZERO) : undefined
});
}, [amountAsBN, estimatedFee, estimatedCrossChainFee, setInputs, call, recipientAddress, isCrossChain, crossChainParams, assetId, formatted, amount, recipientChainName, recipientChainGenesisHash, transferType, onChainCall?.section, balances]);
}, [amountAsBN, estimatedFee, estimatedCrossChainFee, setInputs, call, parsedAssetId, recipientAddress, isCrossChain, crossChainParams, assetId, formatted, amount, recipientChainName, recipientChainGenesisHash, transferType, onChainCall?.section, balances]);

useEffect(() => {
if (!api || !transferableBalance) {
Expand Down Expand Up @@ -339,21 +356,21 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
}, [call, formatted, isCrossChain, crossChainParams]);

const setWholeAmount = useCallback(() => {
if (!api || !transferableBalance || !maxFee || !balances || !ED) {
if (!transferableBalance || !maxFee || !balances) {
return;
}

setTransferType('All');

const _isAvailableZero = transferableBalance.isZero();
const isAvailableZero = transferableBalance.isZero();

const _maxFee = assetId === undefined ? maxFee : BN_ZERO;

const _canNotTransfer = _isAvailableZero || _maxFee.gte(transferableBalance);
const allAmount = _canNotTransfer ? '0' : amountToHuman(transferableBalance.sub(_maxFee).toString(), balances.decimal);
const canNotTransfer = isAvailableZero || _maxFee.gte(transferableBalance);
const allAmount = canNotTransfer ? '0' : amountToHuman(transferableBalance.sub(_maxFee).toString(), balances.decimal);
Comment on lines +370 to +371
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential negative values in amount calculation

In the setWholeAmount function, when calculating allAmount, ensure that subtracting _maxFee from transferableBalance does not result in negative values, which could cause issues when converting to a human-readable amount.

Consider adding a check to ensure the result is not negative:

const balanceAfterFee = transferableBalance.sub(_maxFee);
const allAmount = canNotTransfer || balanceAfterFee.isNeg()
    ? '0'
    : amountToHuman(balanceAfterFee.toString(), balances.decimal);


setAmount(allAmount);
}, [api, assetId, balances, ED, maxFee, transferableBalance]);
}, [assetId, balances, maxFee, transferableBalance]);

const _onChangeAmount = useCallback((value: string) => {
if (!balances) {
Expand Down Expand Up @@ -403,7 +420,7 @@ export default function InputPage ({ address, assetId, balances, inputs, setInpu
<ShowBalance api={api} balance={estimatedFee} />
</Grid>
<Grid alignItems='center' container item justifyContent='space-between' sx={{ height: '25px', width: '57.5%' }}>
<Infotip2 showInfoMark text={t('Existential Deposit: {{ED}}', { replace: { ED: api?.createType('Balance', ED)?.toHuman() } })}>
<Infotip2 showInfoMark text={t('Existential Deposit: {{ED}}', { replace: { ED: `${amountToHuman(balances?.ED, balances?.decimal)} ${balances?.token}` } })}>
<Typography fontSize='14px' fontWeight={400}>
{t('Transferable amount')}
</Typography>
Expand Down
6 changes: 3 additions & 3 deletions packages/extension-polkagate/src/fullscreen/sendFund/Review.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Divider, Grid, Typography, useTheme } from '@mui/material';
import React, { useCallback, useMemo, useState } from 'react';

import { ChainLogo, Identity, Motion, ShowBalance, SignArea2, WrongPasswordAlert } from '../../components';
import { useApi, useChain } from '../../hooks';
import { useInfo } from '../../hooks';
import useTranslation from '../../hooks/useTranslation';
import { ThroughProxy } from '../../partials';
import { PROXY_TYPE } from '../../util/constants';
Expand All @@ -30,10 +30,10 @@ interface Props {

export default function Review ({ address, balances, inputs, setRefresh, setStep, setTxInfo, step }: Props): React.ReactElement {
const { t } = useTranslation();
const api = useApi(address);
const chain = useChain(address);
const theme = useTheme();

const { api, chain } = useInfo(address);

const [isPasswordError, setIsPasswordError] = useState<boolean>(false);
const [selectedProxy, setSelectedProxy] = useState<Proxy | undefined>();

Expand Down
5 changes: 2 additions & 3 deletions packages/extension-polkagate/src/fullscreen/sendFund/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ export default function SendFund (): React.ReactElement {
const ref = useRef(chain);
const history = useHistory();

const parsedAssetId = assetId === undefined || assetId === 'undefined' ? undefined : parseInt(assetId);
const [refresh, setRefresh] = useState<boolean>(false);
const balances = useBalances(address, refresh, setRefresh, undefined, parsedAssetId);
const balances = useBalances(address, refresh, setRefresh, undefined, assetId);

const [step, setStep] = useState<StepsType>(STEPS.INDEX);
const [inputs, setInputs] = useState<Inputs>();
Expand Down Expand Up @@ -106,7 +105,7 @@ export default function SendFund (): React.ReactElement {
{(step === STEPS.INDEX) &&
<InputPage
address={address}
assetId={parsedAssetId}
assetId={assetId}
balances={balances}
inputs={inputs}
setInputs={setInputs}
Expand Down
4 changes: 4 additions & 0 deletions packages/extension-polkagate/src/hooks/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ export { default as useAssetsBalances } from './useAssetsBalances';
export { default as useAuction } from './useAuction';
export { default as useAvailableToSoloStake } from './useAvailableToSoloStake';
export { default as useBalances } from './useBalances';
export { default as useBalancesOnAssethub } from './useBalancesOnAssethub';
export { default as useBalancesOnMultiAssetChain } from './useBalancesOnMultiAssetChain';
export { default as useBalancesOnSingleAssetChain } from './useBalancesOnSingleAssetChain';
export { default as useBlockInterval } from './useBlockInterval';
export { default as useCanPayFee } from './useCanPayFee';
export { default as useCanPayFeeAndDeposit } from './useCanPayFeeAndDeposit';
Expand Down Expand Up @@ -75,6 +78,7 @@ export { default as usePendingRewards } from './usePendingRewards';
export { default as usePendingRewards2 } from './usePendingRewards2';
export { default as usePeopleChain } from './usePeopleChain';
export { default as usePool } from './usePool';
export { default as usePoolBalances } from './usePoolBalances';
export { default as usePoolConsts } from './usePoolConsts';
export { usePoolMembers } from './usePoolMembers';
export { default as usePools } from './usePools';
Expand Down
2 changes: 1 addition & 1 deletion packages/extension-polkagate/src/hooks/useAssetsBalances.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const BN_MEMBERS = [
];

export interface FetchedBalance {
assetId?: number,
assetId?: number | string,
availableBalance: BN,
balanceDetails?: any,
totalBalance: BN,
Expand Down
Loading
Loading