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

Namadillo: IBC withdrawal improvements #1446

Merged
merged 4 commits into from
Dec 26, 2024
Merged

Namadillo: IBC withdrawal improvements #1446

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a029d0
feat: refactoring ibc transfer
pedrorezende Dec 17, 2024
0dd5f6d
feat: ibc improvements wip
pedrorezende Dec 24, 2024
8c3e314
feat: ibc withdraw working sometimes
pedrorezende Dec 24, 2024
44217eb
feat: some refactor improvements
pedrorezende Dec 26, 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 apps/namadillo/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"@chain-registry/client": "^1.53.5",
"@cosmjs/encoding": "^0.32.3",
"@keplr-wallet/types": "^0.12.136",
"@namada/indexer-client": "1.0.2",
"@namada/indexer-client": "1.1.4",
"@tailwindcss/container-queries": "^0.1.1",
"@tanstack/query-core": "^5.40.0",
"@tanstack/react-query": "^5.40.0",
Expand Down
6 changes: 5 additions & 1 deletion apps/namadillo/src/App/Common/Timeline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ type TransactionTimelineProps = {
steps: TransactionStep[];
currentStepIndex: number;
hasError?: boolean;
complete?: boolean;
};

type DisabledProps = {
Expand Down Expand Up @@ -56,6 +57,7 @@ export const Timeline = ({
steps,
currentStepIndex,
hasError,
complete,
}: TransactionTimelineProps): JSX.Element => {
return (
<div>
Expand All @@ -78,7 +80,9 @@ export const Timeline = ({
<StepBullet disabled={index > currentStepIndex} />
)}
<StepContent
isNextStep={index === currentStepIndex + 1 && !hasError}
isNextStep={
index === currentStepIndex && !hasError && !complete
}
disabled={index > currentStepIndex}
hasError={Boolean(hasError)}
>
Expand Down
2 changes: 0 additions & 2 deletions apps/namadillo/src/App/Ibc/IbcShieldAll.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { KeplrWalletManager } from "integrations/Keplr";
import { useState } from "react";
import { ShieldAllIntro } from "./ShieldAllIntro";
import { ShieldAllPanel } from "./ShieldAllPanel";
import { ShieldAllProgress } from "./ShieldAllProgress";

const keplr = new KeplrWalletManager();

Expand Down Expand Up @@ -48,7 +47,6 @@ export const IbcShieldAll: React.FC = () => {
onShieldAll={onShieldAll}
/>
)}
{isSuccess && <ShieldAllProgress />}
</Panel>
);
};
204 changes: 66 additions & 138 deletions apps/namadillo/src/App/Ibc/IbcTransfer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Chain } from "@chain-registry/types";
import { Window as KeplrWindow } from "@keplr-wallet/types";
import { AccountType } from "@namada/types";
import { mapUndefined } from "@namada/utils";
import { TransferTransactionTimeline } from "App/Transactions/TransferTransactionTimeline";
Expand All @@ -11,17 +10,15 @@ import { allDefaultAccountsAtom } from "atoms/accounts";
import {
assetBalanceAtomFamily,
availableChainsAtom,
chainRegistryAtom,
ibcChannelsFamily,
ibcTransferAtom,
} from "atoms/integrations";
import BigNumber from "bignumber.js";
import clsx from "clsx";
import { useIbcTransaction } from "hooks/useIbcTransaction";
import { useTransactionActions } from "hooks/useTransactionActions";
import { useWalletManager } from "hooks/useWalletManager";
import { wallets } from "integrations";
import { KeplrWalletManager } from "integrations/Keplr";
import { getIbcGasConfig } from "integrations/utils";
import { useAtomValue } from "jotai";
import { useEffect, useMemo, useState } from "react";
import namadaChain from "registry/namada.json";
Expand All @@ -31,65 +28,63 @@ import { IbcTopHeader } from "./IbcTopHeader";
const keplr = new KeplrWalletManager();
const defaultChainId = "cosmoshub-4";

export const IbcTransfer: React.FC = () => {
const chainRegistry = useAtomValue(chainRegistryAtom);
export const IbcTransfer = (): JSX.Element => {
// Global & Atom states
const availableChains = useAtomValue(availableChainsAtom);
const [shielded, setShielded] = useState<boolean>(true);
const [selectedAssetAddress, setSelectedAssetAddress] = useState<Address>();
const [amount, setAmount] = useState<BigNumber | undefined>();
const [generalErrorMessage, setGeneralErrorMessage] = useState("");
const [sourceChannel, setSourceChannel] = useState("");
const [destinationChannel, setDestinationChannel] = useState("");

const performIbcTransfer = useAtomValue(ibcTransferAtom);
const defaultAccounts = useAtomValue(allDefaultAccountsAtom);

// Wallet & Registry
const {
registry,
walletAddress: sourceAddress,
connectToChainId,
chainId,
} = useWalletManager(keplr);

const [transaction, setTransaction] =
useState<PartialTransferTransactionData>();

const {
transactions: myTransactions,
findByHash,
storeTransaction,
} = useTransactionActions();
// IBC Channels & Balances
const { data: ibcChannels } = useAtomValue(
ibcChannelsFamily(registry?.chain.chain_name)
);

const { data: availableAssets, isLoading: isLoadingBalances } = useAtomValue(
const { data: userAssets, isLoading: isLoadingBalances } = useAtomValue(
assetBalanceAtomFamily({
chain: registry?.chain,
walletAddress: sourceAddress,
})
);

// Local State
const [shielded, setShielded] = useState<boolean>(true);
const [selectedAssetAddress, setSelectedAssetAddress] = useState<Address>();
const [amount, setAmount] = useState<BigNumber | undefined>();
const [generalErrorMessage, setGeneralErrorMessage] = useState("");
const [sourceChannel, setSourceChannel] = useState("");
const [destinationChannel, setDestinationChannel] = useState("");
const [transaction, setTransaction] =
useState<PartialTransferTransactionData>();

// Derived data
const availableAmount = mapUndefined(
(address) => availableAssets?.[address]?.amount,
(address) => userAssets?.[address]?.amount,
selectedAssetAddress
);

const gasConfig = useMemo(() => {
if (typeof registry !== "undefined") {
return getIbcGasConfig(registry);
}
return undefined;
}, [registry]);
const selectedAsset = mapUndefined(
(address) => userAssets?.[address],
selectedAssetAddress
);

useEffect(() => {
setSelectedAssetAddress(undefined);
}, [registry]);
// Manage the history of transactions
const { findByHash, storeTransaction } = useTransactionActions();

useEffect(() => {
if (transaction?.hash) {
const tx = findByHash(transaction.hash);
if (tx) {
setTransaction(tx);
}
}
}, [myTransactions]);
// Utils for IBC transfers
const { transferToNamada, gasConfig, transferStatus } = useIbcTransaction({
registry,
sourceAddress,
sourceChannel,
destinationChannel,
shielded,
selectedAsset,
});

const namadaAddress = useMemo(() => {
return (
Expand All @@ -99,10 +94,21 @@ export const IbcTransfer: React.FC = () => {
);
}, [defaultAccounts, shielded]);

const { data: ibcChannels } = useAtomValue(
ibcChannelsFamily(registry?.chain.chain_name)
);
const requiresIbcChannels =
!ibcChannels?.cosmosChannelId ||
(shielded && !ibcChannels?.namadaChannelId);

useEffect(() => setSelectedAssetAddress(undefined), [registry]);

// Update transaction if its hash is known and it exists in stored transactions
useEffect(() => {
if (transaction?.hash) {
const tx = findByHash(transaction.hash);
tx && setTransaction(tx);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking loud, how about if we create a new route/page that handle this tx progress? Then, this useEffect executes an route redirect

Sometimes I had a not so good UX when I'm on the progress page, then I click on menu Ibc button to execute a second Ibc and it doesn't navigates. Same if I click on the back button of the browser

Maybe we should just go directly to the History single page, like /transaction/{hash}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We already have this page. However, if the transaction fails for some reason, we're returning to the previous screen, so the user can change the params. I think it would be too complicate to handle this case if we're redirecting the users to a new page directly. Any ideas?

}
}, [transaction?.hash, findByHash]);

// Set source and destination channels based on IBC channels data
useEffect(() => {
setSourceChannel(ibcChannels?.cosmosChannelId || "");
setDestinationChannel(ibcChannels?.namadaChannelId || "");
Expand All @@ -114,112 +120,34 @@ export const IbcTransfer: React.FC = () => {
}: OnSubmitTransferParams): Promise<void> => {
try {
setGeneralErrorMessage("");

if (typeof sourceAddress === "undefined") {
throw new Error("Source address is not defined");
}

if (!chainId) {
throw new Error("Chain ID is undefined");
}

const selectedAsset = mapUndefined(
(address) => availableAssets?.[address],
selectedAssetAddress
);

if (!selectedAsset) {
throw new Error("No asset is selected");
}

if (!registry) {
throw new Error("Invalid chain");
}

if (!sourceChannel) {
throw new Error("Invalid IBC source channel");
}

if (shielded && !destinationChannel) {
throw new Error("Invalid IBC destination channel");
}

if (typeof gasConfig === "undefined") {
throw new Error("No transaction fee is set");
}

const baseKeplr = (window as KeplrWindow).keplr;

if (typeof baseKeplr === "undefined") {
throw new Error("No Keplr instance");
}

// Set Keplr option to allow Namadillo to set the transaction fee
const savedKeplrOptions = baseKeplr.defaultOptions;
baseKeplr.defaultOptions = {
sign: {
preferNoSetFee: true,
},
};

try {
setTransaction({
type: shielded ? "IbcToShielded" : "IbcToTransparent",
asset: selectedAsset.asset,
chainId,
currentStep: TransferStep.Sign,
});

const tx = await performIbcTransfer.mutateAsync({
chain: registry.chain,
transferParams: {
signer: await keplr.getSigner(chainId),
chainId,
sourceAddress,
destinationAddress,
amount: displayAmount,
asset: selectedAsset,
gasConfig,
sourceChannelId: sourceChannel.trim(),
...(shielded ?
{
isShielded: true,
destinationChannelId: destinationChannel.trim(),
}
: {
isShielded: false,
}),
},
});
setTransaction(tx);
storeTransaction(tx);
} finally {
// Restore Keplr options to avoid mutating state
baseKeplr.defaultOptions = savedKeplrOptions;
}
const txPromise = transferToNamada(destinationAddress, displayAmount);
setTransaction({
type: shielded ? "IbcToShielded" : "IbcToTransparent",
asset: selectedAsset!.asset,
chainId: registry!.chain.chain_id,
currentStep: TransferStep.Sign,
});
const result = await txPromise;
setTransaction(result);
storeTransaction(result);
} catch (err) {
setGeneralErrorMessage(err + "");
setTransaction(undefined);
}
};

const onChangeWallet = (): void => {
if (chainId && chainId in chainRegistry) {
connectToChainId(chainId);
if (registry) {
connectToChainId(registry.chain.chain_id);
return;
}

connectToChainId(defaultChainId);
};

const onChangeChain = (chain: Chain): void => {
connectToChainId(chain.chain_id);
};

const requiresIbcChannels =
!ibcChannels?.cosmosChannelId ||
(shielded && !ibcChannels?.namadaChannelId);

return (
<>
<div className="relative min-h-[600px]">
Expand All @@ -232,12 +160,12 @@ export const IbcTransfer: React.FC = () => {
<TransferModule
source={{
isLoadingAssets: isLoadingBalances,
availableAssets,
availableAssets: userAssets,
selectedAssetAddress,
availableAmount,
availableChains,
onChangeChain,
chain: mapUndefined((id) => chainRegistry[id]?.chain, chainId),
chain: registry?.chain,
availableWallets: [wallets.keplr],
wallet: wallets.keplr,
walletAddress: sourceAddress,
Expand All @@ -255,7 +183,7 @@ export const IbcTransfer: React.FC = () => {
onChangeShielded: setShielded,
}}
gasConfig={gasConfig}
isSubmitting={performIbcTransfer.isPending}
isSubmitting={transferStatus === "pending"}
isIbcTransfer={true}
requiresIbcChannels={requiresIbcChannels}
ibcOptions={{
Expand Down
Loading
Loading