Feat/add ignore inscriptions UI #6028

kyranjamie · 2024-12-18T18:07:32Z

Try out Leather build 67589f4 — Extension build, Test report, Storybook, Chromatic

This PR adds more granular control for UTXOs. Now we track discarded UTXOs in setting slice, by txid:vout:offset.

A user might discard an one inscription on a UTXO, but not the second one, on the same UTXO. A UTXO is only spendable if all inscriptions on it are discarded.

In the UI, there's a dropdown from which users can "unprotect" inscriptions. This helps users spend funds with trash BRC-20 inscriptions on them.

This feature only works for native segwit inscriptions. Protecting/unprotecting taproot inscriptions does nothing. The UI is functional but not perfect, e.g. how hover events work on the inscription cards.

Next year, I want to jump straight on a refactor of this work. Much code here should live elsewhere, and all UTXOs should be treated equal, regardless of payment type.

camerow · 2024-12-20T20:29:59Z

src/app/features/collectibles/components/bitcoin/high-sat-value-utxo.tsx

+  if (!featureBuilt) return null;
+  return (
+    <Box position="absolute" top="space.01" right="space.01">
+      <BasicTooltip label="This inscription has loads of BTC on it, remove protections? Click">


Possibly specify the amount instead of "loads"?

this is disabled for now, but yeah would be better to specify

src/app/features/discarded-inscriptions/use-inscribed-spendable-utxos.ts

src/app/query/bitcoin/address/utxos-by-address.hooks.ts

camerow · 2024-12-20T20:41:24Z

src/app/ui/components/account/account.card.tsx

+              <styled.span ml="space.01">
+                <BasicTooltip
+                  side="right"
+                  label="Some funds may be unavailable to protect your collectible assets. Disable protection to access your remaining balance."


Maybe a bit more specificity on where: "... You can ensure those fund are available by disabling protection in the collectible menu."

edgarkhanzadian

My biggest concern is the empty strings that we pass around. Other than that, looks awesome! 🚀

edgarkhanzadian · 2024-12-23T12:04:56Z

src/app/common/hooks/balance/use-total-balance.tsx

+      availableBalance,
+      availableUsdBalance: i18nFormatCurrency(
+        availableBalance,
+        availableBalance.amount.isGreaterThanOrEqualTo(100_000) ? 0 : 2


nit: move 100_000 to a constant

edgarkhanzadian · 2024-12-23T12:08:02Z

src/app/features/collectibles/components/bitcoin/high-sat-value-utxo.tsx

+}
+
+export function HighSatValueUtxoWarning({ inscription }: HighSatValueUtxoProps) {
+  if (Number(inscription.value) < 5_000) return null;


This 5_000 constant too. Should this be a part of a wallet config?

edgarkhanzadian · 2024-12-23T12:08:55Z

src/app/features/collectibles/components/bitcoin/high-sat-value-utxo.tsx

+
+import { BasicTooltip } from '@app/ui/components/tooltip/basic-tooltip';
+
+const featureBuilt = false;


seems like a good place for launch darkly

I could delete this as we're not using it, but hoping to add some kind of warning when revisiting this

edgarkhanzadian · 2024-12-23T12:09:57Z

src/app/features/collectibles/components/bitcoin/inscription.tsx

+  const content = useMemo(() => {
+    const sharedProps = { onClickSend: () => openSendInscriptionModal() };
+    switch (inscription.mimeType) {
+      case 'audio':
+        return (
+          <CollectibleAudio
+            icon={<OrdinalAvatarIcon size="lg" />}
+            key={inscription.title}
+            subtitle="Ordinal inscription"
+            title={`# ${inscription.number}`}
+            {...sharedProps}
+          />
+        );
+      case 'html':
+      case 'svg':
+      case 'video':
+      case 'gltf':
+        return (
+          <CollectibleIframe
+            icon={<OrdinalAvatarIcon size="lg" />}
+            key={inscription.title}
+            src={inscription.src}
+            subtitle="Ordinal inscription"
+            title={`# ${inscription.number}`}


I'd suggest making this a separate component that receives inscription instead of creating a component in useMemo

edgarkhanzadian · 2024-12-23T12:11:46Z

src/app/features/discarded-inscriptions/use-inscribed-spendable-utxos.ts

+
+  // Utxos but don't filter the inscribed ones
+  const { data: nativeSegwitUtxos } = useNativeSegwitUtxosByAddress({
+    address: address ?? '',


I'd suggest using undefined or null instead of empty string here. Typescript can help us with null/undefined while it can't do so with empty strings

Totally agree and only did this as I'm trying to get it done quickly before Christmas.

The way I see it, resorting to empty strings here (as done in many other places), is symptomatic of poor patterns around how we pass and handle state.

I've been thinking about this with this work. One solution would be to use feature container components to handle null cases, and set state in contexts state such that it can be consumed without the need to inline null fallbacks in every case.

This is probably a type signature hole useNativeSegwitUtxosByAddress, but I think we should address using placeholder values in the codebase (lots of those) in future by tightening up the contracts.

src/app/features/discarded-inscriptions/use-inscribed-spendable-utxos.ts

edgarkhanzadian · 2024-12-23T12:14:28Z

src/app/pages/home/home.tsx

+  const { totalUsdBalance, availableUsdBalance, isPending, isLoadingAdditionalData } =
+    useAvailableBalance({
+      btcAddress,
+      stxAddress: account?.address || '',


Same here regarding the empty string

src/app/query/bitcoin/address/utxos-by-address.hooks.ts

src/app/common/hooks/use-hover-with-children.ts

tigranpetrossian · 2024-12-23T12:18:41Z

src/app/features/collectibles/components/bitcoin/inscription.tsx

+          </CollectibleOther>
+        );
+      default:
+        return null;


if mimeType exhaustiveness is important here, I added an assertUnreachable to utils package do make sure it errors for uncovered cases.

src/app/features/collectibles/components/bitcoin/inscription.tsx

tigranpetrossian · 2024-12-23T12:25:32Z

src/app/features/discarded-inscriptions/use-inscribed-spendable-utxos.ts

+
+  // Utxos but don't filter the inscribed ones
+  const { data: nativeSegwitUtxos } = useNativeSegwitUtxosByAddress({
+    address: address ?? '',


This is probably a type signature hole useNativeSegwitUtxosByAddress, but I think we should address using placeholder values in the codebase (lots of those) in future by tightening up the contracts.

kyranjamie · 2024-12-23T12:49:53Z

Thanks for your reviews all, unaddressed feedback I will work on when refactoring this in the new year

kyranjamie force-pushed the feat/add-ignore-inscriptions-ui branch 4 times, most recently from 57bb153 to 175048f Compare December 20, 2024 16:39

camerow reviewed Dec 20, 2024

View reviewed changes

src/app/features/discarded-inscriptions/use-inscribed-spendable-utxos.ts Show resolved Hide resolved

camerow reviewed Dec 20, 2024

View reviewed changes

src/app/query/bitcoin/address/utxos-by-address.hooks.ts Outdated Show resolved Hide resolved

camerow reviewed Dec 20, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

kyranjamie force-pushed the feat/add-ignore-inscriptions-ui branch 8 times, most recently from db4f120 to 2164df7 Compare December 23, 2024 11:50

kyranjamie requested review from fbwoolf, alter-eggo, tigranpetrossian and edgarkhanzadian December 23, 2024 11:59

edgarkhanzadian approved these changes Dec 23, 2024

View reviewed changes

kyranjamie force-pushed the feat/add-ignore-inscriptions-ui branch from 2164df7 to d5a30dd Compare December 23, 2024 12:20

tigranpetrossian approved these changes Dec 23, 2024

View reviewed changes

kyranjamie force-pushed the feat/add-ignore-inscriptions-ui branch 2 times, most recently from 9cf5924 to 9f6a04a Compare December 23, 2024 12:27

feat: granular utxo protection feature

67589f4

kyranjamie force-pushed the feat/add-ignore-inscriptions-ui branch from 9f6a04a to 67589f4 Compare December 23, 2024 12:28

kyranjamie merged commit fbb2fed into dev Dec 23, 2024
32 checks passed

kyranjamie deleted the feat/add-ignore-inscriptions-ui branch December 23, 2024 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/add ignore inscriptions UI #6028

Feat/add ignore inscriptions UI #6028

kyranjamie commented Dec 18, 2024 •

edited by leather-bot

Loading

camerow Dec 20, 2024

kyranjamie Dec 23, 2024

camerow Dec 20, 2024

This comment was marked as resolved.

edgarkhanzadian left a comment

edgarkhanzadian Dec 23, 2024

edgarkhanzadian Dec 23, 2024

edgarkhanzadian Dec 23, 2024

kyranjamie Dec 23, 2024

edgarkhanzadian Dec 23, 2024

edgarkhanzadian Dec 23, 2024

kyranjamie Dec 23, 2024

tigranpetrossian Dec 23, 2024

edgarkhanzadian Dec 23, 2024

tigranpetrossian Dec 23, 2024

tigranpetrossian Dec 23, 2024

kyranjamie commented Dec 23, 2024


		import { BasicTooltip } from '@app/ui/components/tooltip/basic-tooltip';

		const featureBuilt = false;

Feat/add ignore inscriptions UI #6028

Feat/add ignore inscriptions UI #6028

Conversation

kyranjamie commented Dec 18, 2024 • edited by leather-bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

edgarkhanzadian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyranjamie commented Dec 23, 2024

kyranjamie commented Dec 18, 2024 •

edited by leather-bot

Loading