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.

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

feat: listen for the transfer success event #1490

Merged
merged 6 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
37 changes: 8 additions & 29 deletions apps/namadillo/src/App/Common/Timeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,25 @@ type TransactionTimelineProps = {
complete?: boolean;
};

type DisabledProps = {
disabled: boolean;
};

const StepConnector = ({ disabled }: DisabledProps): JSX.Element => (
<i
className={clsx(
"h-10 w-px bg-current mb-3",
clsx({ "opacity-20": disabled })
)}
/>
const StepConnector = (): JSX.Element => (
<i className="h-10 w-px bg-current mb-3" />
);

const StepBullet = ({ disabled }: DisabledProps): JSX.Element => (
<i
className={clsx(
"w-4 aspect-square rounded-full bg-current",
clsx({ "opacity-20": disabled })
)}
/>
const StepBullet = (): JSX.Element => (
<i className="w-4 aspect-square rounded-full bg-current" />
);

const StepContent = ({
children,
isCurrentStep,
hasError,
disabled,
}: React.PropsWithChildren & {
isCurrentStep: boolean;
hasError: boolean;
disabled: boolean;
}): JSX.Element => (
<div
className={clsx("text-center", {
"animate-pulse": isCurrentStep && !hasError,
"opacity-20": disabled,
})}
>
{children}
Expand Down Expand Up @@ -206,19 +189,15 @@ export const Timeline = ({
className={twMerge(
clsx(
"flex flex-col gap-1 items-center",
"text-center transition-all duration-150"
"text-center transition-all duration-150",
{ "opacity-20": index > currentStepIndex }
)
)}
>
{index > 0 && (
<StepConnector disabled={index > currentStepIndex} />
)}
{step.bullet && (
<StepBullet disabled={index > currentStepIndex} />
)}
{index > 0 && <StepConnector />}
{step.bullet && <StepBullet />}
<StepContent
isCurrentStep={index === currentStepIndex}
disabled={index > currentStepIndex}
hasError={!!hasError}
>
{step.children}
Expand Down
20 changes: 5 additions & 15 deletions apps/namadillo/src/hooks/useTransactionCallbacks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ export const useTransactionCallback = (): void => {
useTransactionEventListener("Unbond.Success", onBalanceUpdate);
useTransactionEventListener("Withdraw.Success", onBalanceUpdate);
useTransactionEventListener("Redelegate.Success", onBalanceUpdate);
useTransactionEventListener("ClaimRewards.Success", onBalanceUpdate, [
account?.address,
]);
useTransactionEventListener("ClaimRewards.Success", onBalanceUpdate);

useTransactionEventListener("VoteProposal.Success", () => {
shouldUpdateProposal(true);
Expand All @@ -56,18 +54,10 @@ export const useTransactionCallback = (): void => {
"UnshieldingTransfer.Success",
],
(e) => {
e.detail.data.forEach((dataList) => {
dataList.data.forEach((props) => {
const sourceAddress = "source" in props ? props.source : "";
sourceAddress &&
changeTransaction(
e.detail.tx.hash,
{
status: "success",
currentStep: TransferStep.Complete,
},
sourceAddress
);
e.detail.tx.forEach(({ hash }) => {
changeTransaction(hash, {
status: "success",
currentStep: TransferStep.Complete,
});
});
}
Expand Down
15 changes: 6 additions & 9 deletions apps/namadillo/src/hooks/useTransactionNotifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const getTotalAmountFromTransactionList = (txs: TxWithAmount[]): BigNumber =>
}, new BigNumber(0));

const parseTxsData = <T extends TxWithAmount>(
tx: TxProps,
tx: TxProps | TxProps[],
data: T[]
): { id: string; total: BigNumber } => {
const id = createNotificationId(tx);
Expand Down Expand Up @@ -357,11 +357,9 @@ export const useTransactionNotifications = (): void => {
"UnshieldingTransfer.Error",
],
(e) => {
const tx = e.detail.tx;
const data: TxWithAmount[] = e.detail.data[0].data;
const { id } = parseTxsData(tx, data);
const id = createNotificationId(e.detail.tx);
clearPendingNotifications(id);
const storedTx = searchAllStoredTxByHash(tx.hash);
const storedTx = searchAllStoredTxByHash(e.detail.tx[0].hash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check if what you want is not the tx[0].innerTxHashes[0]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Validated. The tx[0].hash is correct. We don't have the innerTxHashes on the event object, and we are creating the stored tx with the txs[0].hash from response, not the txs[0].innerTxHashes[0]

Screenshot 2025-01-14 at 11 50 50

Screenshot 2025-01-14 at 11 51 04

dispatchNotification({
id,
type: "error",
Expand Down Expand Up @@ -391,11 +389,10 @@ export const useTransactionNotifications = (): void => {
"UnshieldingTransfer.Success",
],
(e) => {
const tx = e.detail.tx;
const data: TxWithAmount[] = e.detail.data[0].data;
const { id } = parseTxsData(tx, data);
const id = createNotificationId(e.detail.tx);
clearPendingNotifications(id);
const storedTx = searchAllStoredTxByHash(tx.hash);
clearPendingNotifications(id);
const storedTx = searchAllStoredTxByHash(e.detail.tx[0].hash);
dispatchNotification({
id,
title: "Transfer transaction succeeded",
Expand Down
5 changes: 4 additions & 1 deletion apps/namadillo/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,25 @@ export enum TransferStep {
// Defines the steps in the Namada <> Namada transfer progress for tracking transaction stages.
export const namadaTransferStages = {
TransparentToShielded: [
TransferStep.Sign,
TransferStep.ZkProof,
TransferStep.Sign,
TransferStep.TransparentToShielded,
TransferStep.Complete,
] as const,
ShieldedToTransparent: [
TransferStep.ZkProof,
TransferStep.Sign,
TransferStep.ShieldedToTransparent,
TransferStep.Complete,
] as const,
ShieldedToShielded: [
TransferStep.ZkProof,
TransferStep.Sign,
TransferStep.ShieldedToShielded,
TransferStep.Complete,
] as const,
TransparentToTransparent: [
TransferStep.ZkProof,
TransferStep.Sign,
TransferStep.TransparentToTransparent,
TransferStep.Complete,
Expand Down
2 changes: 1 addition & 1 deletion apps/namadillo/src/types/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export type TransactionEventHandlers = {

export interface EventData<T> extends CustomEvent {
detail: {
tx: TxProps;
tx: TxProps[];
data: T[];
// If event is for PartialSuccess, use the following:
successData?: T[];
Expand Down
41 changes: 28 additions & 13 deletions apps/namadillo/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { ProposalStatus, ProposalTypeString } from "@namada/types";
import { localnetConfigAtom } from "atoms/integrations/atoms";
import BigNumber from "bignumber.js";
import { getDefaultStore } from "jotai";
import { isEqual } from "lodash";
import namadaAssets from "namada-chain-registry/namada/assetlist.json";
import { useEffect } from "react";
import { useEffect, useRef } from "react";

export const proposalStatusToString = (status: ProposalStatus): string => {
const statusText: Record<ProposalStatus, string> = {
Expand Down Expand Up @@ -38,32 +39,46 @@ export const proposalIdToString = (proposalId: bigint): string =>

export const useTransactionEventListener = <T extends keyof WindowEventMap>(
event: T,
handler: (this: Window, ev: WindowEventMap[T]) => void,
deps: React.DependencyList = []
handler: (event: WindowEventMap[T]) => void
): void => {
// `handlerRef` is useful to avoid recreating the listener every time
const handlerRef = useRef(handler);
handlerRef.current = handler;

useEffect(() => {
window.addEventListener(event, handler);
const callback: typeof handler = (event) => handlerRef.current(event);
window.addEventListener(event, callback);
return () => {
window.removeEventListener(event, handler);
window.removeEventListener(event, callback);
};
}, deps);
}, [event]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I lost many hours debugging until realizing this dependencies was the issue 😢

Maybe we should enable the "react-hooks/exhaustive-deps": "warn" and, if you don't want to use, at least you need to add the lint ignore comment

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Daaaamn

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we concatenate this [event] with the former deps array, if it's not empty?

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 don't need the deps anymore, because the function is saved on the ref, so you are free to pass anything, even a new function everytime

};

export const useTransactionEventListListener = <T extends keyof WindowEventMap>(
events: T[],
handler: (this: Window, ev: WindowEventMap[T]) => void,
deps: React.DependencyList = []
handler: (event: WindowEventMap[T]) => void
): void => {
// `eventsRef` and `handlerRef` are useful to avoid recreating the listener every time
const eventsRef = useRef(events);
if (!isEqual(events, eventsRef.current)) {
eventsRef.current = events;
}
const eventsValue = eventsRef.current;

const handlerRef = useRef(handler);
handlerRef.current = handler;

useEffect(() => {
events.forEach((event) => {
window.addEventListener(event, handler);
const callback: typeof handler = (event) => handlerRef.current(event);
eventsValue.forEach((event) => {
window.addEventListener(event, callback);
});
return () => {
events.forEach((event) => {
window.removeEventListener(event, handler);
eventsValue.forEach((event) => {
window.removeEventListener(event, callback);
});
};
}, deps);
}, [eventsValue]);
};

export const sumBigNumberArray = (numbers: BigNumber[]): BigNumber => {
Expand Down
Loading