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

🧹 (Dust to OSMO): Feature implementation #3913

Open
wants to merge 15 commits into
base: stage
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
39 changes: 38 additions & 1 deletion packages/trpc/src/assets.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PricePretty } from "@keplr-wallet/unit";
import { Dec, PricePretty } from "@keplr-wallet/unit";
import {
AssetFilterSchema,
AvailableRangeValues,
Expand Down Expand Up @@ -408,6 +408,43 @@ export const assetsRouter = createTRPCRouter({
limit,
})
),
getUserDustAssets: publicProcedure
.input(
UserOsmoAddressSchema.merge(
z.object({
dustThreshold: z.custom<Dec>((val) => {
if (val instanceof Dec) return true;
try {
new Dec(val as string | number);
return true;
} catch {
return false;
}
}, "Invalid Dec value"),
})
)
)
.query(async ({ input: { userOsmoAddress, dustThreshold }, ctx }) => {
const assets = await mapGetAssetsWithUserBalances({
...ctx,
userOsmoAddress,
// TODO(Greg): check if this filtering assumption is correct
onlyVerified: true,
includePreview: false,
});

const assetsWithAmountAndUsdValue = assets.filter(
(
asset
): asset is Omit<typeof asset, "amount" | "usdValue"> &
Required<Pick<typeof asset, "amount" | "usdValue">> =>
asset.amount !== undefined && asset.usdValue !== undefined
);

return assetsWithAmountAndUsdValue.filter((asset) =>
asset.usdValue.toDec()?.lte(dustThreshold)
);
Comment on lines +444 to +446
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 5, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for asset price fetching.

The filter assumes usdValue is always valid when defined. Consider adding error handling for potential failures in price fetching.

       return assetsWithAmountAndUsdValue.filter((asset) =>
-        asset.usdValue.toDec()?.lte(dustThreshold)
+        asset.usdValue.toDec().lte(dustThreshold)
       );

Also, consider adding a try-catch block to handle potential price fetching errors:

+      const filteredAssets = [];
+      for (const asset of assetsWithAmountAndUsdValue) {
+        try {
+          if (asset.usdValue.toDec().lte(dustThreshold)) {
+            filteredAssets.push(asset);
+          }
+        } catch (error) {
+          console.error(`Failed to process asset ${asset.coinDenom}:`, error);
+          continue;
+        }
+      }
+      return filteredAssets;

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Contributor Author

@greg-nagy greg-nagy Nov 5, 2024

Choose a reason for hiding this comment

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

The filter assumes usdValue is always valid when defined. Consider adding error handling for potential failures in price fetching.

There is a filter above this that makes sure this is true. (The filter doesn't check the type assumption that the values are indeed CoinPretty or PricePretty)

Also, consider adding a try-catch block to handle potential price fetching errors:

The errors are ok to flow through the client.

Copy link
Contributor

Choose a reason for hiding this comment

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

@greg-nagy, thank you for the clarification. I understand that the filter above ensures usdValue is valid when defined, and that it's acceptable for errors to flow through to the client in this context. I'll keep this in mind for future reviews.


✏️ Learnings added
Learnt from: greg-nagy
PR: osmosis-labs/osmosis-frontend#3913
File: packages/trpc/src/assets.ts:444-446
Timestamp: 2024-11-05T08:46:24.588Z
Learning: In `packages/trpc/src/assets.ts`, within the `getUserDustAssets` procedure, a prior filter ensures that `usdValue` is defined and valid, so additional error handling for `usdValue` may not be necessary in subsequent operations.

Learnt from: greg-nagy
PR: osmosis-labs/osmosis-frontend#3913
File: packages/trpc/src/assets.ts:444-446
Timestamp: 2024-11-05T08:46:24.588Z
Learning: In the codebase, it's acceptable for errors to propagate to the client from the `getUserDustAssets` procedure in `packages/trpc/src/assets.ts`, so adding try-catch blocks for handling potential price fetching errors is not required.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}),
getUserAssetsTotal: publicProcedure
.input(UserOsmoAddressSchema.required())
.query(({ input, ctx }) => getUserAssetsTotal({ ...ctx, ...input })),
Expand Down
3 changes: 2 additions & 1 deletion packages/types/src/feature-flags-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ export type AvailableFlags =
| "sqsActiveOrders"
| "alloyedAssets"
| "bridgeDepositAddress"
| "nomicWithdrawAmount";
| "nomicWithdrawAmount"
| "dustToOsmo";
5 changes: 4 additions & 1 deletion packages/web/.env
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,7 @@ TWITTER_API_URL=https://api.twitter.com/
BLOCKAID_BASE_URL=http://api.blockaid.io:80
# BLOCKAID_API_KEY=

NEXT_PUBLIC_SPEND_LIMIT_CONTRACT_ADDRESS=osmo10xqv8rlpkflywm92k5wdmplzy7khtasl9c2c08psmvlu543k724sy94k74
NEXT_PUBLIC_SPEND_LIMIT_CONTRACT_ADDRESS=osmo10xqv8rlpkflywm92k5wdmplzy7khtasl9c2c08psmvlu543k724sy94k74

# Feature flag for dust to osmo
# NEXT_PUBLIC_SHOW_DUST_TO_OSMO=true
236 changes: 236 additions & 0 deletions packages/web/components/complex/dust-to-osmo/dust-to-osmo.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
import { Dec } from "@keplr-wallet/unit";
import { useMutation, UseMutationResult } from "@tanstack/react-query";
import { useCallback, useEffect, useMemo, useState } from "react";

import { FallbackImg, Icon } from "~/components/assets";
import { Button } from "~/components/ui/button";
import { DefaultSlippage } from "~/config/swap";
import { useTranslation, useWalletSelect } from "~/hooks";
import { useSwap } from "~/hooks/use-swap";
import { useStore } from "~/stores";
import { HideDustUserSetting } from "~/stores/user-settings";
import { api } from "~/utils/trpc";

const maxSlippage = new Dec(DefaultSlippage);

export function DustToOsmo({ onComplete }: { onComplete?: () => void }) {
const { t } = useTranslation();

const { accountStore } = useStore();
const account = accountStore.getWallet(accountStore.osmosisChainId);
const { isLoading: isLoadingWallet } = useWalletSelect();

const [convertDust, setConvertDust] = useState(false);

const {
data: dustAssets,
isSuccess: isSuccessDust,
isFetched: isFetchedDust,
refetch: refetchDust,
} = api.edge.assets.getUserDustAssets.useQuery(
{
userOsmoAddress: account?.address ?? "",
// TODO(Greg): the dust threshold has changed to 0.02.
// Is it ok to use this or should I use the 0.01 which was specified?
dustThreshold: HideDustUserSetting.DUST_THRESHOLD,
},
{
enabled: !isLoadingWallet && Boolean(account?.address) && convertDust,
cacheTime: 0,
staleTime: 0,
refetchOnWindowFocus: false,
refetchOnMount: false,
refetchOnReconnect: false,
}
);

const handleConvertDustComplete = useCallback(() => {
setConvertDust(false);
// Refetch to ensure the dust assets are up to date
refetchDust();
onComplete && onComplete();
}, [onComplete, refetchDust]);

useEffect(() => {
// Stop loading if there are no dust assets
if (convertDust && isFetchedDust && dustAssets?.length === 0) {
// Prevent a flash of the button if the dust assets are fetched quickly
const timer = setTimeout(() => {
setConvertDust(false);
}, 500);

return () => clearTimeout(timer);
}
}, [convertDust, dustAssets, isFetchedDust]);

return (
<>
<Button
className="gap-2 !border !border-osmoverse-700 !py-2 !px-4 !text-wosmongton-200"
variant="outline"
size="lg-full"
isLoading={convertDust}
onClick={() => setConvertDust(true)}
>
<Icon id="dust-broom" className="h-6 w-6" />
{t("dustToOsmo.mainButton")}
</Button>
{convertDust && isSuccessDust && dustAssets.length > 0 && (
<SequentialSwapExecutor
dustAssets={dustAssets}
onComplete={handleConvertDustComplete}
/>
)}
</>
);
}

function SequentialSwapExecutor({
dustAssets,
onComplete,
}: {
dustAssets: {
coinDenom: string;
amount: { toDec: () => { toString: () => string } };
}[];
onComplete: () => void;
}) {
const [swapTxStatuses, setSwapTxStatuses] = useState<{
[denom: string]: {
status: UseMutationResult["status"];
denom: string;
};
}>({});

const hasAllSwapsFinished =
Object.keys(swapTxStatuses).length === dustAssets.length &&
Object.values(swapTxStatuses).every(
({ status }) => status === "success" || status === "error"
);

const handleSwapTxStatusChange = useCallback(
(denom: string, status: UseMutationResult["status"]) =>
setSwapTxStatuses((prev) => ({ ...prev, [denom]: { status, denom } })),
[]
);

const activeSwapDenom = useMemo(() => {
const runningSwapDenom = Object.values(swapTxStatuses).find(
(s) => s.status === "loading"
)?.denom;

if (runningSwapDenom) {
return runningSwapDenom;
}

const nextDenom = Object.values(swapTxStatuses).find(
(s) => s.status === "idle"
)?.denom;

return nextDenom ?? "";
}, [swapTxStatuses]);

useEffect(() => {
// Notify the parent that we're done
if (hasAllSwapsFinished) {
onComplete();
}
}, [hasAllSwapsFinished, onComplete]);

return (
<>
{dustAssets.map((dustAsset) => (
<SwapHandler
key={dustAsset.coinDenom}
fromDenom={dustAsset.coinDenom}
amount={dustAsset.amount.toDec().toString()}
onSendSwapTxStatusChange={handleSwapTxStatusChange}
shouldExecuteSwap={activeSwapDenom === dustAsset.coinDenom}
/>
))}
</>
);
}

function SwapHandler({
fromDenom,
amount,
shouldExecuteSwap = false,
onSendSwapTxStatusChange,
}: {
fromDenom: string;
amount: string;
shouldExecuteSwap: boolean;
onSendSwapTxStatusChange: (
denom: string,
status: UseMutationResult["status"]
) => void;
}) {
const {
fromAsset,
toAsset,
inAmountInput: { setAmount: setInAmount },
isReadyToSwap,
sendTradeTokenInTx,
} = useSwap({
initialFromDenom: fromDenom,
initialToDenom: "OSMO",
useQueryParams: false,
maxSlippage,
quoteType: "out-given-in",
// Swap needs a bit of time to stabilize
// otherwise account seq errors can happen
inputDebounceMs: 100,
});

useEffect(() => {
setInAmount(amount);
}, [setInAmount, amount]);

const {
mutate: sendSwapTx,
isIdle: isSendSwapTxIdle,
status: sendSwapTxStatus,
} = useMutation(sendTradeTokenInTx);

useEffect(() => {
if (shouldExecuteSwap && isReadyToSwap && isSendSwapTxIdle) {
sendSwapTx();
}
}, [shouldExecuteSwap, isReadyToSwap, isSendSwapTxIdle, sendSwapTx]);

useEffect(() => {
onSendSwapTxStatusChange(fromDenom, sendSwapTxStatus);
}, [fromDenom, sendSwapTxStatus, onSendSwapTxStatusChange]);

return (
<>
{shouldExecuteSwap &&
fromAsset?.amount?.currency.coinImageUrl &&
toAsset?.amount?.currency.coinImageUrl && (
<div className="flex lg:hidden items-center justify-end transition-colors duration-200 ease-in-out">
<FallbackImg
alt={fromAsset?.amount?.denom}
src={fromAsset?.amount?.currency.coinImageUrl}
fallbacksrc="/icons/question-mark.svg"
height={32}
width={32}
/>
<Icon
id="arrow-right"
width={16}
height={16}
className="my-[8px] mx-[4px] text-osmoverse-500"
/>
<FallbackImg
alt={toAsset?.amount?.denom}
src={toAsset?.amount?.currency.coinImageUrl}
fallbacksrc="/icons/question-mark.svg"
height={32}
width={32}
/>
</div>
)}
</>
);
}
8 changes: 6 additions & 2 deletions packages/web/components/table/portfolio-asset-balances.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
useState,
} from "react";

import { DustToOsmo } from "~/components/complex/dust-to-osmo/dust-to-osmo";
import { AssetCell } from "~/components/table/cells/asset";
import { SpriteIconId } from "~/config";
import {
Expand Down Expand Up @@ -74,6 +75,7 @@ export const PortfolioAssetBalancesTable: FunctionComponent<{
const { width, isMobile } = useWindowSize();
const router = useRouter();
const { t } = useTranslation();
const featureFlags = useFeatureFlags();

// #region search
const [searchQuery, setSearchQuery] = useState<Search | undefined>();
Expand Down Expand Up @@ -124,6 +126,7 @@ export const PortfolioAssetBalancesTable: FunctionComponent<{
isPreviousData,
isFetchingNextPage,
fetchNextPage,
refetch,
} = api.edge.assets.getUserBridgeAssets.useInfiniteQuery(
{
userOsmoAddress: account?.address,
Expand Down Expand Up @@ -438,10 +441,11 @@ export const PortfolioAssetBalancesTable: FunctionComponent<{
</tbody>
</table>
{assetsData.length > 0 && (
<div className="flex items-center justify-end gap-4 py-2 px-4">
<div className="flex gap-4 py-2 px-4">
{featureFlags.dustToOsmo && <DustToOsmo onComplete={refetch} />}
<Button
onClick={() => setHideDust(!hideDust)}
className="gap-2 !border !border-osmoverse-700 !py-2 !px-4 !text-wosmongton-200"
className="ml-auto gap-2 !border !border-osmoverse-700 !py-2 !px-4 !text-wosmongton-200"
variant="outline"
size="lg-full"
>
Expand Down
4 changes: 4 additions & 0 deletions packages/web/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,7 @@ export const GITHUB_API_TOKEN = process.env.GITHUB_API_TOKEN;

export const SPEND_LIMIT_CONTRACT_ADDRESS =
process.env.NEXT_PUBLIC_SPEND_LIMIT_CONTRACT_ADDRESS;

// TODO(Greg): add this flag to LaunchDarkly and drop the .env config
export const SHOW_DUST_TO_OSMO =
process.env.NEXT_PUBLIC_SHOW_DUST_TO_OSMO === "true";
3 changes: 3 additions & 0 deletions packages/web/hooks/use-feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AvailableFlags } from "@osmosis-labs/types";
import { useFlags, useLDClient } from "launchdarkly-react-client-sdk";
import { useEffect, useState } from "react";

import { SHOW_DUST_TO_OSMO } from "~/config";
import { useWindowSize } from "~/hooks";

const defaultFlags: Record<AvailableFlags, boolean> = {
Expand Down Expand Up @@ -32,6 +33,8 @@ const defaultFlags: Record<AvailableFlags, boolean> = {
alloyedAssets: false,
bridgeDepositAddress: false,
nomicWithdrawAmount: false,
// TODO(Greg): add this flag to LaunchDarkly and drop the .env config
dustToOsmo: SHOW_DUST_TO_OSMO,
};

export function useFeatureFlags() {
Expand Down
Loading