Another round of fixes

- Transaction success is now checked through a shared helper that validates hash, gas usage, and response payloads, not hash presence alone. - Node settings error helper renamed to match its broader scope. - Balance refresh now owns the loading flag so nested balance and vesting fetches do not race each other. - Unbond modal removes the non-null assertion on compounded rewards.
2026-06-08 19:06:13 +02:00
parent 7374ceae6f
commit 13d48b4bb6
9 changed files with 171 additions and 33 deletions
@@ -25,6 +25,7 @@ interface Props {
 export const UnbondModal = ({ node, onConfirm, onClose, onError }: Props) => {
  const { fee, isFeeLoading, getFee, feeError } = useGetFee();
  const unbondReturn = formatOperatorUnbondReturn(node);
+  const compoundedRewards = unbondReturn.hasCompoundedRewards ? unbondReturn.operatorRewards : null;

  useEffect(() => {
    if (feeError) {
@@ -66,14 +67,10 @@ export const UnbondModal = ({ node, onConfirm, onClose, onError }: Props) => {
          Could not calculate exact return - check your wallet balance after unbonding.
        </Alert>
      )}
-      {unbondReturn.hasCompoundedRewards ? (
+      {compoundedRewards ? (
        <>
          <ModalListItem label="Original pledge" value={formatCoinDisplay(unbondReturn.pledge)} divider />
-          <ModalListItem
-            label="Compounded operator rewards"
-            value={formatCoinDisplay(unbondReturn.operatorRewards!)}
-            divider
-          />
+          <ModalListItem label="Compounded operator rewards" value={formatCoinDisplay(compoundedRewards)} divider />
          <ModalListItem label="Total returned to your account" value={formatCoinDisplay(unbondReturn.total)} divider />
          <Typography fontSize="small" sx={{ mb: 1 }}>
            Delegator stake is returned to delegators separately and is not included in this total.
@@ -0,0 +1,13 @@
+import { runBalanceRefreshWithoutNestedLoading } from './balanceRefreshOrchestration';
+
+describe('runBalanceRefreshWithoutNestedLoading', () => {
+  it('delegates loading ownership to the caller by disabling nested loading toggles', async () => {
+    const fetchBalance = jest.fn(async () => undefined);
+    const fetchTokenAllocation = jest.fn(async () => undefined);
+
+    await runBalanceRefreshWithoutNestedLoading(fetchBalance, fetchTokenAllocation);
+
+    expect(fetchBalance).toHaveBeenCalledWith(false);
+    expect(fetchTokenAllocation).toHaveBeenCalledWith(false, false);
+  });
+});
@@ -0,0 +1,7 @@
+export async function runBalanceRefreshWithoutNestedLoading(
+  fetchBalance: (manageLoading?: boolean) => Promise<void>,
+  fetchTokenAllocation: (isBackgroundPoll?: boolean, manageLoading?: boolean) => Promise<void>,
+): Promise<void> {
+  await fetchBalance(false);
+  await fetchTokenAllocation(false, false);
+}
@@ -13,6 +13,7 @@ import {
  userBalance,
 } from '../requests';
 import { Console } from '../utils/console';
+import { runBalanceRefreshWithoutNestedLoading } from './balanceRefreshOrchestration';

 type TTokenAllocation = {
  [key in
@@ -98,16 +99,22 @@ export const useGetBalance = (clientDetails?: Account): TUseuserBalance => {
    setVestingAccountInfo(vestingAccountDetail);
  };

-  const fetchTokenAllocation = async (isBackgroundPoll = false) => {
-    setIsLoading(true);
+  const fetchTokenAllocation = async (isBackgroundPoll = false, manageLoading = true) => {
+    if (manageLoading) {
+      setIsLoading(true);
+    }
    if (!clientDetails?.client_address) {
-      setIsLoading(false);
+      if (manageLoading) {
+        setIsLoading(false);
+      }
      return;
    }

    if (vestingAccountStatusRef.current === 'absent') {
      if (isBackgroundPoll) {
-        setIsLoading(false);
+        if (manageLoading) {
+          setIsLoading(false);
+        }
        return;
      }
      vestingAccountStatusRef.current = 'unknown';
@@ -204,12 +211,16 @@ export const useGetBalance = (clientDetails?: Account): TUseuserBalance => {
      clearVestingUiState();
      Console.error(e as string);
    } finally {
-      setIsLoading(false);
+      if (manageLoading) {
+        setIsLoading(false);
+      }
    }
  };

-  const fetchBalance = useCallback(async () => {
-    setIsLoading(true);
+  const fetchBalance = useCallback(async (manageLoading = true) => {
+    if (manageLoading) {
+      setIsLoading(true);
+    }
    setError(undefined);
    try {
      const bal = await userBalance();
@@ -217,7 +228,9 @@ export const useGetBalance = (clientDetails?: Account): TUseuserBalance => {
    } catch (err) {
      setError(err as string);
    } finally {
-      setIsLoading(false);
+      if (manageLoading) {
+        setIsLoading(false);
+      }
    }
  }, []);

@@ -236,9 +249,10 @@ export const useGetBalance = (clientDetails?: Account): TUseuserBalance => {
      clearAll();
      return;
    }
+    setIsLoading(true);
+    setError(undefined);
    try {
-      await fetchBalance();
-      await fetchTokenAllocation();
+      await runBalanceRefreshWithoutNestedLoading(fetchBalance, fetchTokenAllocation);
    } finally {
      setIsLoading(false);
    }
@@ -249,8 +263,6 @@ export const useGetBalance = (clientDetails?: Account): TUseuserBalance => {
      clearAll();
      return;
    }
-    setIsLoading(true);
-    setError(undefined);
    refreshBalances().catch((e) => Console.error(String(e)));
  }, [clientDetails?.client_address]);

@@ -16,7 +16,7 @@ import { useBondingContext, AppContext } from 'src/context';
 import { TBondedNymNode } from 'src/requests/nymNodeDetails';
 import { settingsValidationSchema } from 'src/components/Bonding/forms/nym-node/settingsValidationSchema';
 import { simulateUpdateNymNodeConfig } from 'src/requests';
-import { getHostnameUpdateErrorMessage } from 'src/utils/hostnameUpdateError';
+import { getNodeSettingsUpdateErrorMessage } from 'src/utils/nodeSettingsUpdateError';

 export const GeneralNymNodeSettings = ({ bondedNode }: { bondedNode: TBondedNymNode }) => {
  const [openConfirmationModal, setOpenConfirmationModal] = useState<boolean>(false);
@@ -57,7 +57,7 @@ export const GeneralNymNodeSettings = ({ bondedNode }: { bondedNode: TBondedNymN

    try {
      const tx = await updateNymNodeConfig(configUpdate, fee);
-      const errorMessage = getHostnameUpdateErrorMessage(tx);
+      const errorMessage = getNodeSettingsUpdateErrorMessage(tx);
      if (errorMessage) {
        setSubmitError(errorMessage);
        resetFeeState();
@@ -68,7 +68,7 @@ export const GeneralNymNodeSettings = ({ bondedNode }: { bondedNode: TBondedNymN
      setOpenConfirmationModal(true);
    } catch (error) {
      Console.error(error);
-      setSubmitError(getHostnameUpdateErrorMessage(undefined, String(error)));
+      setSubmitError(getNodeSettingsUpdateErrorMessage(undefined, String(error)));
      resetFeeState();
    }
  };
@@ -1,12 +1,12 @@
-import { getHostnameUpdateErrorMessage } from './hostnameUpdateError';
+import { getNodeSettingsUpdateErrorMessage } from './nodeSettingsUpdateError';

-describe('getHostnameUpdateErrorMessage', () => {
+describe('getNodeSettingsUpdateErrorMessage', () => {
  it('returns undefined when the transaction succeeded', () => {
    expect(
-      getHostnameUpdateErrorMessage({
+      getNodeSettingsUpdateErrorMessage({
        transaction_hash: 'abc123',
-        logs_json: '',
-        msg_responses_json: '',
+        logs_json: '[]',
+        msg_responses_json: '[]',
        gas_info: {
          gas_wanted: { gas_units: BigInt(1) },
          gas_used: { gas_units: BigInt(1) },
@@ -17,23 +17,23 @@ describe('getHostnameUpdateErrorMessage', () => {
  });

  it('returns context error when provided', () => {
-    expect(getHostnameUpdateErrorMessage(undefined, 'an error occurred: insufficient funds')).toBe(
+    expect(getNodeSettingsUpdateErrorMessage(undefined, 'an error occurred: insufficient funds')).toBe(
      'an error occurred: insufficient funds',
    );
  });

  it('returns a generic message when the update failed without context error', () => {
-    expect(getHostnameUpdateErrorMessage(undefined)).toBe(
+    expect(getNodeSettingsUpdateErrorMessage(undefined)).toBe(
      'Unable to update node settings. Check your balance and try again.',
    );
  });

  it('returns an error message when transaction_hash is an empty string', () => {
    expect(
-      getHostnameUpdateErrorMessage({
+      getNodeSettingsUpdateErrorMessage({
        transaction_hash: '',
-        logs_json: '',
-        msg_responses_json: '',
+        logs_json: '[]',
+        msg_responses_json: '[]',
        gas_info: {
          gas_wanted: { gas_units: BigInt(1) },
          gas_used: { gas_units: BigInt(1) },
@@ -42,4 +42,19 @@ describe('getHostnameUpdateErrorMessage', () => {
      }),
    ).toBe('Unable to update node settings. Check your balance and try again.');
  });
+
+  it('returns an error when gas_used is zero despite a non-empty hash', () => {
+    expect(
+      getNodeSettingsUpdateErrorMessage({
+        transaction_hash: 'abc123',
+        logs_json: '[]',
+        msg_responses_json: '[]',
+        gas_info: {
+          gas_wanted: { gas_units: BigInt(1) },
+          gas_used: { gas_units: BigInt(0) },
+        },
+        fee: { amount: '1', denom: 'nym' },
+      }),
+    ).toBe('Unable to update node settings. Check your balance and try again.');
+  });
 });
@@ -1,10 +1,11 @@
 import { TransactionExecuteResult } from '@nymproject/types';
+import { isTransactionExecuteSuccessful } from './transactionExecuteSuccess';

-export const getHostnameUpdateErrorMessage = (
+export const getNodeSettingsUpdateErrorMessage = (
  tx: TransactionExecuteResult | undefined,
  contextError?: string,
 ): string | undefined => {
-  if (tx?.transaction_hash && tx.transaction_hash.length > 0) {
+  if (isTransactionExecuteSuccessful(tx)) {
    return undefined;
  }
  if (contextError) {
@@ -0,0 +1,53 @@
+import { TransactionExecuteResult } from '@nymproject/types';
+import { isTransactionExecuteSuccessful } from './transactionExecuteSuccess';
+
+const baseTx = {
+  transaction_hash: 'abc123',
+  logs_json: '[]',
+  msg_responses_json: '[]',
+  gas_info: {
+    gas_wanted: { gas_units: BigInt(100) },
+    gas_used: { gas_units: BigInt(50) },
+  },
+  fee: { amount: '1', denom: 'nym' },
+} as TransactionExecuteResult;
+
+describe('isTransactionExecuteSuccessful', () => {
+  it('returns true for a structurally valid execution result', () => {
+    expect(isTransactionExecuteSuccessful(baseTx)).toBe(true);
+  });
+
+  it('returns true when log and response JSON are empty strings', () => {
+    expect(
+      isTransactionExecuteSuccessful({
+        ...baseTx,
+        logs_json: '',
+        msg_responses_json: '',
+      }),
+    ).toBe(true);
+  });
+
+  it('returns false when transaction_hash is empty', () => {
+    expect(isTransactionExecuteSuccessful({ ...baseTx, transaction_hash: '' })).toBe(false);
+  });
+
+  it('returns false when gas_used is zero', () => {
+    expect(
+      isTransactionExecuteSuccessful({
+        ...baseTx,
+        gas_info: {
+          gas_wanted: { gas_units: BigInt(100) },
+          gas_used: { gas_units: BigInt(0) },
+        },
+      }),
+    ).toBe(false);
+  });
+
+  it('returns false when msg_responses_json is not valid JSON', () => {
+    expect(isTransactionExecuteSuccessful({ ...baseTx, msg_responses_json: 'not-json' })).toBe(false);
+  });
+
+  it('returns false when logs_json is not a JSON array', () => {
+    expect(isTransactionExecuteSuccessful({ ...baseTx, logs_json: '{"error":true}' })).toBe(false);
+  });
+});
@@ -0,0 +1,40 @@
+import { TransactionExecuteResult } from '@nymproject/types';
+
+const parseJsonArray = (raw: string): unknown[] | null => {
+  const trimmed = raw.trim();
+  if (!trimmed) {
+    return [];
+  }
+  try {
+    const parsed = JSON.parse(trimmed);
+    return Array.isArray(parsed) ? parsed : null;
+  } catch {
+    return null;
+  }
+};
+
+/**
+ * Rust IPC only returns `TransactionExecuteResult` after DeliverTx succeeds (code 0).
+ * This helper centralizes the TS-side interpretation: non-empty hash plus structurally
+ * valid execution metadata, not hash presence alone.
+ */
+export const isTransactionExecuteSuccessful = (tx: TransactionExecuteResult | undefined): boolean => {
+  if (!tx?.transaction_hash || tx.transaction_hash.length === 0) {
+    return false;
+  }
+
+  const gasUsed = tx.gas_info?.gas_used?.gas_units;
+  if (gasUsed === undefined || gasUsed <= 0n) {
+    return false;
+  }
+
+  if (parseJsonArray(tx.msg_responses_json) === null) {
+    return false;
+  }
+
+  if (parseJsonArray(tx.logs_json) === null) {
+    return false;
+  }
+
+  return true;
+};