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

fix: fee issue utilizing fee hook #1679

Merged
merged 3 commits into from
Dec 2, 2024
Merged

fix: fee issue utilizing fee hook #1679

merged 3 commits into from
Dec 2, 2024

Conversation

Nick-1979
Copy link
Member

@Nick-1979 Nick-1979 commented Nov 30, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a streamlined fee estimation process using the useEstimatedFee hook across various components.
    • Enhanced type safety and clarity by updating type imports and state management.
  • Bug Fixes

    • Improved error handling in fee calculations and transaction processes.
  • Documentation

    • Updated function signatures and props interfaces for better clarity and consistency.
  • Refactor

    • Removed redundant state management and effects related to fee estimation, simplifying component logic.
  • Style

    • Minor formatting adjustments for consistency in function signatures and code structure.

Copy link
Contributor

coderabbitai bot commented Nov 30, 2024

Walkthrough

The pull request includes extensive modifications across multiple components, primarily focusing on the integration of the useEstimatedFee hook to streamline the estimated fee calculation process. This change replaces various state management approaches with direct hook usage, enhancing type safety and simplifying the code structure. The updates affect components related to governance, staking, and account management, ensuring consistent handling of transaction fees and improving overall clarity in the codebase.

Changes

File Path Change Summary
packages/extension-polkagate/src/fullscreen/accountDetails/unlock/Review.tsx Adjusted imports; replaced estimatedFee state with useEstimatedFee hook for fee calculation.
packages/extension-polkagate/src/fullscreen/governance/delegate/index.tsx Updated fee handling to wrap partialFee in api.createType('Balance', ...) for type consistency.
`packages/extension-polkagate/src/fullscreen/governance/delegate/modify/ModifyDelegate.tsx Enhanced type safety with new type imports; updated fee calculation logic to use api.createType for estimatedFee.
`packages/extension-polkagate/src/fullscreen/governance/delegate/remove/RemoveDelegate.tsx Updated partialFee handling to ensure it is typed as Balance.
`packages/extension-polkagate/src/fullscreen/governance/post/castVote/Cast.tsx Modified fee calculation to wrap partialFee in api.createType('Balance', ...).
`packages/extension-polkagate/src/fullscreen/governance/post/castVote/index.tsx Removed estimatedFee state; replaced with useEstimatedFee hook for fee calculation.
`packages/extension-polkagate/src/fullscreen/governance/post/decisionDeposit/index.tsx Replaced estimatedFee state with useEstimatedFee hook; adjusted SignArea2 component's address prop.
`packages/extension-polkagate/src/fullscreen/manageProxies/Review.tsx Removed @ts-nocheck; replaced estimatedFee state with useEstimatedFee hook; updated API method access with optional chaining.
`packages/extension-polkagate/src/fullscreen/socialRecovery/Review.tsx Removed useEffect; replaced estimatedFee with useEstimatedFee hook; updated transaction handling logic.
`packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/SetState.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/manageValidators/Review.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/fullscreen/stake/solo/commonTasks/manageValidators/Review.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/fullscreen/stake/solo/fastUnstake/index.tsx Removed Balance import; replaced estimatedFee state with useEstimatedFee hook.
`packages/extension-polkagate/src/fullscreen/stake/solo/pending/index.tsx Changed ExpandedRewards from interface to tuple type; updated eraIndex type from number to string.
`packages/extension-polkagate/src/fullscreen/stake/solo/restake/index.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
packages/extension-polkagate/src/hooks/useIdentity.ts Updated formatted parameter type to include null.
`packages/extension-polkagate/src/popup/account/unlock/Review.tsx Removed Chain and Balance imports; replaced estimatedFee state with useEstimatedFee hook.
`packages/extension-polkagate/src/popup/manageProxies/Review.tsx Removed @ts-nocheck; replaced estimatedFee state with useEstimatedFee hook.
`packages/extension-polkagate/src/popup/send/Review.tsx Removed Review component entirely; simplified To component.
`packages/extension-polkagate/src/popup/send/partial/SendTxDetail.tsx Component removed.
`packages/extension-polkagate/src/popup/staking/partial/SelectValidatorsReview.tsx Removed Balance and BN_ONE imports; replaced estimatedFee state with useEstimatedFee hook.
`packages/extension-polkagate/src/popup/staking/pool/myPool/SetState.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/popup/staking/pool/nominations/remove/index.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/popup/staking/pool/redeem/index.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/popup/staking/pool/rewards/Stake.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/popup/staking/pool/rewards/Withdraw.tsx Replaced estimatedFee state with useEstimatedFee hook; removed previous fee calculation logic.
`packages/extension-polkagate/src/popup/staking/pool/stake/bondExtra/Review.tsx Added new type imports; refined totalStaked calculation logic.
`packages/extension-polkagate/src/popup/staking/pool/stake/createPool/index.tsx Enhanced type safety for estimatedFee and estimatedMaxFee.
`packages/extension-polkagate/src/popup/staking/pool/unstake/Review.tsx Added BN type; updated Props interface with staked and total properties.
`packages/extension-polkagate/src/popup/staking/pool/unstake/index.tsx Updated estimatedFee handling and type assertions; refined alert handling logic.

Possibly related PRs

Suggested labels

change requested

Suggested reviewers

  • AMIRKHANEF

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 6b58042 and a24f3e8.

📒 Files selected for processing (2)
  • packages/extension-polkagate/src/fullscreen/accountDetails/unlock/Review.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/account/unlock/Review.tsx (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/extension-polkagate/src/fullscreen/accountDetails/unlock/Review.tsx
  • packages/extension-polkagate/src/popup/account/unlock/Review.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 19

🧹 Outside diff range and nitpick comments (92)
packages/extension-polkagate/src/popup/send/Review.tsx (5)

14-14: Remove unnecessary empty import from React

The import statement includes an empty destructuring { }, which is unnecessary and can be removed to clean up the code.

Apply this diff to fix the import:

-import React, { } from 'react';
+import React from 'react';

18-18: Improve readability by formatting the Props interface

Formatting the Props interface over multiple lines enhances readability and maintainability.

Apply this diff:

-interface Props { chain: Chain | null, identiconSize?: number, mb?: number, noDivider?: boolean, pt1?: number, pt2?: number, fontSize1?: number, label: string, formatted: string | undefined }
+interface Props {
+  chain: Chain | null;
+  identiconSize?: number;
+  mb?: number;
+  noDivider?: boolean;
+  pt1?: number;
+  pt2?: number;
+  fontSize1?: number;
+  label: string;
+  formatted: string | undefined;
+}

20-20: Consider renaming the To component for clarity

The component name To may not clearly convey its purpose. Renaming it to something more descriptive, like RecipientDetails, can improve code readability.

Apply this diff to rename the component:

-export function To ({ chain, fontSize1 = 28, formatted, identiconSize = 31, label, mb = 10, noDivider, pt1 = 0, pt2 = 5 }: Props): React.ReactElement<Props> {
+export function RecipientDetails ({ chain, fontSize1 = 28, formatted, identiconSize = 31, label, mb = 10, noDivider, pt1 = 0, pt2 = 5 }: Props): React.ReactElement<Props> {

Remember to update any references to this component accordingly.


Line range hint 24-29: Avoid casting chain to any in the Identity component

Casting chain to any can undermine TypeScript's type safety. Ensure that the Identity component accepts the correct type for chain.

Consider updating the Identity component's type definitions to accept Chain | null or adjust the type as needed. Apply this diff:

<Identity
-  chain={chain as any}
+  chain={chain}
  formatted={formatted}
  identiconSize={identiconSize}
  showSocial={false}
  style={{ fontSize: `${fontSize1}px` }}
/>

Verify that the Identity component's chain prop is correctly typed to prevent the need for casting.


Line range hint 22-29: Ensure consistent styling and responsive design

The inline styles and fixed values for dimensions might not be responsive on different screen sizes. Consider using theme spacing and responsive units.

Example adjustment:

<Grid alignItems='center' container direction='column' justifyContent='center' sx={{
-  fontWeight: 300,
-  letterSpacing: '-0.015em'
+  fontWeight: 300,
+  letterSpacing: '-0.015em',
+  mx: 'auto',
+  width: '100%'
}}>

This ensures the component scales appropriately across various screen sizes.

packages/extension-polkagate/src/hooks/useIdentity.ts (3)

Line range hint 4-4: Remove @ts-nocheck and fix type issues properly

Using @ts-nocheck disables all type checking for the file, which could hide potential issues. Consider:

  1. Properly typing the API responses instead of using as any
  2. Using proper type guards instead of type assertions
  3. Enabling strict type checking to catch potential issues early

Line range hint 89-89: Improve error handling beyond console.log

The current error handling pattern of .catch(console.error) is insufficient for a production environment:

  1. Users aren't notified when identity fetching fails
  2. No error state is maintained in the hook
  3. No retry mechanism for transient API failures

Consider implementing proper error handling with user feedback and recovery mechanisms.

- }).catch(console.error);
+ }).catch((error: Error) => {
+   console.error('Failed to fetch identity:', error);
+   setError(error);
+   // Optionally: implement retry logic or user notification
+ });

Also applies to: 97-97


Line range hint 45-48: Strengthen null handling in getSubIdentityOf

While the function checks for !formatted, there are several improvements that could be made to make null handling more robust:

  1. Consider using nullish coalescing operator (??) for defaults
  2. Add explicit type guards for API responses
  3. Document the expected behavior when receiving null values
 const getSubIdentityOf = useCallback(async (): Promise<SubIdentity | undefined> => {
-  if (!api?.query?.['identity']?.['superOf'] || !formatted) {
+  if (!api?.query?.identity?.superOf) {
+    console.debug('Identity API not available');
+    return;
+  }
+  if (formatted == null) {
+    console.debug('No address provided');
     return;
   }
packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/manageValidators/Review.tsx (2)

44-44: Consider adding error handling for fee estimation failures.

While the useEstimatedFee hook simplifies the code, it's important to handle potential estimation failures to ensure a robust user experience.

Consider adding error handling:

-  const estimatedFee = useEstimatedFee(address, call, params);
+  const { estimatedFee, error: feeError } = useEstimatedFee(address, call, params);
+  
+  useEffect(() => {
+    if (feeError) {
+      console.error('Fee estimation failed:', feeError);
+      // Handle the error appropriately, e.g., show an error message to the user
+    }
+  }, [feeError]);

Line range hint 46-51: Improve fee handling in extraInfo.

The current fee conversion might not handle BigNumber values correctly and the fallback to '0' could be misleading.

Consider this safer approach:

  const extraInfo = useMemo(() => ({
    action: 'Pool Staking',
-   fee: String(estimatedFee || 0),
+   fee: estimatedFee ? estimatedFee.toString() : undefined,
    subAction: 'Select Validator',
    validatorsCount: selectedValidators?.length
  }), [estimatedFee, selectedValidators?.length]);

This change:

  1. Properly handles BigNumber values
  2. Makes it clear when fee estimation is unavailable instead of showing '0'
packages/extension-polkagate/src/fullscreen/stake/solo/commonTasks/manageValidators/Review.tsx (1)

46-46: Consider adding type annotation for better type safety.

The fee estimation logic has been nicely simplified using the custom hook. However, consider adding an explicit type annotation to make the return type more obvious.

-  const estimatedFee = useEstimatedFee(address, call, params);
+  const estimatedFee: Balance | undefined = useEstimatedFee(address, call, params);
packages/extension-polkagate/src/popup/staking/solo/restake/index.tsx (2)

53-54: Consider adding explicit error handling for undefined API

While optional chaining safely handles undefined api, consider adding explicit error handling to provide better user feedback when the API is not available.

-  const rebonded = api?.tx['staking']['rebond'];
+  const rebonded = api?.tx['staking']['rebond'];
+  useEffect(() => {
+    if (!api) {
+      setAlert(t('API is not available. Please try again later.'));
+    }
+  }, [api, t]);

Line range hint 1-199: Consider implementing comprehensive error boundaries

While the migration to useEstimatedFee hook improves maintainability, this component handles critical staking functionality and would benefit from a more robust error handling strategy.

Consider implementing:

  1. React Error Boundary to catch and handle rendering errors
  2. Comprehensive loading states during fee estimation and transaction preparation
  3. Clear user feedback for all possible error scenarios

Would you like me to provide an example implementation of these improvements?

packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/SetState.tsx (2)

Line range hint 62-89: Consider extracting complex logic into helper functions

The useEffect contains complex conditional logic that could be more maintainable if broken down into smaller, focused functions.

Consider this refactoring:

+ const shouldChillPool = (pool, state, formatted) => {
+   return state === 'Destroying' &&
+     pool.stashIdAccount?.nominators?.length > 0 &&
+     [String(pool.bondedPool?.roles.root), String(pool.bondedPool?.roles.nominator)]
+       .includes(String(formatted));
+ };
+
+ const getTransactionParams = (api, poolId, state, shouldChill) => {
+   if (shouldChill) {
+     return {
+       call: api.tx.utility.batchAll,
+       params: [[api.tx.nominationPools.chill(poolId), 
+                 api.tx.nominationPools.setState(poolId, state)]]
+     };
+   }
+   return {
+     call: api.tx.nominationPools.setState,
+     params: [poolId, state]
+   };
+ };

  useEffect(() => {
    if (!api) {
      return;
    }

-   const batchAll = api?.tx['utility']['batchAll'];
-   const chilled = api?.tx['nominationPools']['chill'];
-   const poolSetState = api?.tx['nominationPools']['setState'];
    const poolId = pool.poolId.toString();

-   const mayNeedChill =
-     state === 'Destroying' &&
-     pool.stashIdAccount?.nominators &&
-     pool.stashIdAccount.nominators.length > 0 &&
-     [String(pool.bondedPool?.roles.root), String(pool.bondedPool?.roles.nominator)].includes(String(formatted));
+   const mayNeedChill = shouldChillPool(pool, state, formatted);
-   const call = mayNeedChill ? batchAll : poolSetState;
-   const params = mayNeedChill ? [[chilled(poolId), poolSetState(poolId, state)]] : [poolId, state];
+   const { call, params } = getTransactionParams(api, poolId, state, mayNeedChill);

    setInputs({
      call,
      extraInfo,
      params
    });
  }, [api, extraInfo, formatted, pool.bondedPool?.roles.nominator, pool.bondedPool?.roles.root, pool.poolId, pool.stashIdAccount?.nominators, state]);

Line range hint 51-56: Add null check for estimatedFee in extraInfo

The estimatedFee is used directly in extraInfo without proper null checking, which could lead to display issues if fee estimation fails.

Consider this safer approach:

  const extraInfo = useMemo(() => ({
    action: 'Pool Staking',
-   fee: String(estimatedFee || 0),
+   fee: estimatedFee ? String(estimatedFee) : undefined,
    helperText,
    pool,
    subAction: state === 'Destroying' ? 'Destroy Pool' : state === 'Open' ? 'Unblock Pool' : 'Block Pool'
  }), [estimatedFee, helperText, state, pool]);
packages/extension-polkagate/src/popup/staking/solo/nominations/remove/index.tsx (2)

21-21: LGTM: Improved fee estimation approach

The switch to useEstimatedFee hook from manual fee calculation is a good architectural improvement that:

  • Centralizes fee estimation logic
  • Reduces component complexity
  • Makes the code more maintainable

Also applies to: 54-54


Line range hint 147-156: Consider handling loading state for fee estimation

While the fee display works correctly, consider showing a loading indicator while the fee is being estimated to improve user experience.

 <Grid alignItems='center' container item justifyContent='center' pt='10px'>
   <Grid item>
     {t('Fee')}:
   </Grid>
   <Grid item sx={{ pl: '5px' }}>
-    <ShowValue height={16} value={estimatedFee?.toHuman()} />
+    {estimatedFee === undefined ? (
+      <ShowValue height={16} value="Calculating..." />
+    ) : (
+      <ShowValue height={16} value={estimatedFee.toHuman()} />
+    )}
   </Grid>
 </Grid>
🧰 Tools
🪛 Biome (1.9.4)

[error] 13-13: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/rewards/Stake.tsx (1)

14-14: Consider renaming the Proxy type import to avoid shadowing

The imported Proxy type shadows the global Proxy property, which could lead to confusion. Consider renaming it to something more specific like ProxyConfig or ProxyDefinition.

-import type { Proxy, ProxyItem, TxInfo } from '../../../../util/types';
+import type { Proxy as ProxyConfig, ProxyItem, TxInfo } from '../../../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/nominations/remove/index.tsx (3)

Line range hint 81-116: Enhance error handling and fee fallback logic

The current implementation has two areas for improvement:

  1. The fee fallback chain fee || String(estimatedFee || 0) could be more explicit
  2. The catch block could provide more specific error handling

Consider this improvement:

- fee: fee || String(estimatedFee || 0),
+ fee: fee ?? String(estimatedFee ?? 0),

- } catch (e) {
-   console.log('error:', e);
-   setIsPasswordError(true);
+ } catch (error) {
+   console.error('Failed to remove validators:', error);
+   if (error.message.includes('password')) {
+     setIsPasswordError(true);
+   } else {
+     // Handle other types of errors appropriately
+     onAction('/error');
+   }
🧰 Tools
🪛 Biome (1.9.4)

[error] 13-13: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 164-171: Consider enhancing transaction progress feedback

The WaitScreen could provide more detailed feedback about the transaction status to improve user experience.

Consider adding transaction steps feedback:

 <WaitScreen
   show={showWaitScreen}
   title={title}
+  steps={[
+    { label: t('Preparing transaction'), status: 'current' },
+    { label: t('Broadcasting'), status: 'pending' },
+    { label: t('Confirming'), status: 'pending' }
+  ]}
 />
🧰 Tools
🪛 Biome (1.9.4)

[error] 13-13: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 38-195: Consider breaking down the component for better maintainability

The component handles multiple responsibilities including:

  • Transaction management
  • Proxy handling
  • Password validation
  • UI state management

This complexity makes it harder to maintain and test.

Consider:

  1. Extract proxy-related logic into a custom hook
  2. Create a separate transaction handler component
  3. Move password validation to a dedicated service
  4. Use TypeScript's strict type checking instead of type assertions

Example for proxy hook extraction:

function useProxyManagement(api: ApiPromise, formatted: string) {
  const [selectedProxy, setSelectedProxy] = useState<Proxy>();
  const [proxyItems, setProxyItems] = useState<ProxyItem[]>();
  const proxies = useProxies(api, formatted);

  useEffect(() => {
    const fetchedProxyItems = proxies?.map((p: Proxy) => ({
      proxy: p,
      status: 'current'
    })) as ProxyItem[];
    setProxyItems(fetchedProxyItems);
  }, [proxies]);

  return {
    selectedProxy,
    setSelectedProxy,
    proxyItems
  };
}
packages/extension-polkagate/src/popup/staking/pool/rewards/Withdraw.tsx (2)

12-14: Consider renaming the Proxy type import

The imported Proxy type shadows the global Proxy object. While this might be intentional given the domain context, it could lead to confusion.

Consider renaming it to something more specific like PolkadotProxy or ChainProxy to avoid shadowing.

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 41-41: Consider breaking down the component for better maintainability

While the implementation is correct, the component handles multiple responsibilities (fee calculation, proxy handling, password management, transaction submission). Consider extracting some of these into separate components or custom hooks for better maintainability.

For example:

  • Extract password handling into a custom hook
  • Move proxy selection logic to a separate component
  • Create a dedicated transaction submission hook
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/restake/Review.tsx (2)

13-17: Consider renaming the Proxy type import to avoid shadowing

The imported Proxy type could potentially shadow the global Proxy object. Consider renaming it to something more specific like StakingProxy or WalletProxy to avoid confusion.

-import type { Proxy, ProxyItem, TxInfo } from '../../../../util/types';
+import type { Proxy as StakingProxy, ProxyItem, TxInfo } from '../../../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 83-106: Enhance error handling in unstake function

The current error handling only logs to console, which might not be sufficient for a production environment. Consider:

  1. Adding specific error types/messages for different failure scenarios
  2. Implementing proper error recovery
  3. Showing appropriate user feedback
 } catch (e) {
-  console.log('error:', e);
+  console.error('Unstaking failed:', e);
+  setShowWaitScreen(false);
+  if (e.message.includes('password')) {
     setIsPasswordError(true);
+  } else {
+    // Handle other error types appropriately
+    onAction('/error', { error: e.message });
+  }
 }
🧰 Tools
🪛 Biome (1.9.4)

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/fastUnstake/Review.tsx (4)

12-14: Consider renaming the shadowed Proxy type

The Proxy type name shadows the global Proxy property, which could lead to confusion. Consider renaming it to something more specific like ProxyConfig or StakingProxy.

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


61-63: Consider memoizing the transaction object

The tx() call in useEstimatedFee could trigger unnecessary re-renders. Consider memoizing it with useMemo:

- const tx = api.tx['fastUnstake']['registerFastUnstake'];
+ const tx = useMemo(() => api.tx['fastUnstake']['registerFastUnstake'], [api]);
  const estimatedFee = useEstimatedFee(address, tx());

Line range hint 95-96: Improve fee fallback logic clarity

The current fee fallback chain fee || String(estimatedFee || 0) could be made more explicit:

- fee: fee || String(estimatedFee || 0),
+ fee: fee ?? String(estimatedFee ?? '0'),

Using the nullish coalescing operator (??) makes it clearer that we're checking for undefined/null values.

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 108-111: Enhance error handling specificity

The current error handling only logs the error and sets a password error flag. Consider adding more specific error handling:

    } catch (e) {
-     console.log('error:', e);
+     console.error('Fast unstake submission failed:', e);
+     if (e instanceof Error && e.message.includes('password')) {
        setIsPasswordError(true);
+     } else {
+       // Handle other types of errors (e.g., network, insufficient funds)
+       onAction('/error', { message: e.message });
+     }
    }
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/stake/solo/restake/index.tsx (2)

Line range hint 89-99: Replace console.log with proper error handling

Consider replacing the console.log with proper error handling or user feedback when the amount exceeds decimal places.

 if (value.length > decimal - 1) {
-  console.log(`The amount digits is more than decimal:${decimal}`);
+  setAlert(t('Amount exceeds maximum allowed decimal places'));
   return;
 }

Line range hint 77-84: Consider adding minimum amount validation

The validation only checks for exceeding the unlocking amount, but there's no minimum amount validation. Consider adding a check for amounts greater than zero or minimum stakeable amount.

 useEffect(() => {
+  if (amountAsBN?.isZero()) {
+    return setAlert(t('Amount must be greater than zero.'));
+  }
   if (amountAsBN?.gt(unlockingAmount || BN_ZERO)) {
     return setAlert(t('It is more than total unlocking amount.'));
   }
   setAlert(undefined);
 }, [unlockingAmount, t, amountAsBN]);
packages/extension-polkagate/src/popup/staking/solo/redeem/index.tsx (2)

12-15: Rename the Proxy type import to avoid shadowing.

The Proxy type import shadows the global Proxy property, which could lead to confusion. Consider renaming it to something more specific like StakingProxy.

-import type { Proxy, ProxyItem, TxInfo } from '../../../../util/types';
+import type { Proxy as StakingProxy, ProxyItem, TxInfo } from '../../../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 81-116: Improve error handling and fee management in submit function.

The submit function has several areas that could be improved:

  1. The error handling is too broad and only logs to console
  2. The fee fallback logic could result in '0' being saved to history

Consider these improvements:

 const submit = useCallback(async () => {
   try {
     if (!formatted) {
+      throw new Error('No formatted address provided');
       return;
     }
+    if (!estimatedFee) {
+      throw new Error('Fee estimation failed');
+    }

     // ... existing code ...

     const info = {
       // ... existing code ...
-      fee: fee || String(estimatedFee || 0),
+      fee: fee ?? String(estimatedFee),
       // ... existing code ...
     };

     // ... existing code ...
   } catch (e) {
     console.log('error:', e);
+    if (e instanceof Error) {
+      // Handle specific error types appropriately
+      if (e.message.includes('password')) {
         setIsPasswordError(true);
+      } else {
+        // Handle other errors (e.g., show error message to user)
+      }
+    }
   }
 }, [/* ... */]);
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/redeem/index.tsx (2)

12-12: Rename the Proxy type to avoid shadowing

The Proxy type shadows the global Proxy property, which could lead to confusion. Consider renaming it to something more specific like ProxyConfig or StakingProxy.

-import type { Proxy, ProxyItem, TxInfo } from '../../../../util/types';
+import type { StakingProxy as Proxy, ProxyItem, TxInfo } from '../../../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 12-12: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 95-95: Improve fee handling in transaction info

The current implementation has two potential issues:

  1. Using String(estimatedFee || 0) could lead to unexpected behavior if estimatedFee is undefined
  2. Direct string conversion of BN might lose precision
-fee: fee || String(estimatedFee || 0),
+fee: fee?.toString() || estimatedFee?.toString() || '0',
🧰 Tools
🪛 Biome (1.9.4)

[error] 12-12: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/accountDetails/unlock/Review.tsx (1)

Line range hint 70-76: Improve fee handling in extraInfo

The current implementation of fee conversion could be more robust:

Consider these improvements:

  const extraInfo = useMemo(() => ({
    action: 'Unlock Referenda',
    amount,
-   fee: String(estimatedFee || 0),
+   fee: estimatedFee?.toString() ?? '0',
    subAction: 'Unlock'
-  }), [amount, estimatedFee]);
+  }), [amount, estimatedFee?.toString()]);

This change:

  1. Uses optional chaining and nullish coalescing for safer undefined handling
  2. Memoizes based on the string value to prevent unnecessary rerenders
packages/extension-polkagate/src/popup/staking/solo/fastUnstake/index.tsx (2)

56-59: Consider adding error handling for the fee estimation

The migration to useEstimatedFee hook is a good improvement that simplifies the code. However, consider handling potential estimation failures.

-  const estimatedFee = useEstimatedFee(address, tx?.());
+  const { estimatedFee, error: feeEstimationError } = useEstimatedFee(address, tx?.());

Line range hint 60-69: Consider extracting eligibility checks into helper functions

While the logic is correct, the eligibility checks could be more readable if extracted into named helper functions.

+  const checkDepositAvailability = useCallback((
+    deposit: BN | undefined,
+    consts: StakingConsts | undefined,
+    balances: DeriveBalancesAll | undefined,
+    fee: BN | undefined
+  ): boolean | undefined => {
+    if (!deposit || !consts || !balances || !fee) {
+      return undefined;
+    }
+    return deposit.add(fee).lt(getValue('available', balances) || BN_ZERO);
+  }, []);
+
+  const checkUnlockingStatus = useCallback((
+    redeemable: BN | undefined,
+    account: AccountStakingInfo | undefined
+  ): boolean | undefined => {
+    if (redeemable === undefined || !account) {
+      return undefined;
+    }
+    return !(!redeemable.isZero() || account.unlocking?.length);
+  }, []);

-  const hasEnoughDeposit = fastUnstakeDeposit && stakingConsts && myBalances && estimatedFee && getValue('available', myBalances)
-    ? new BN(fastUnstakeDeposit).add(estimatedFee).lt(getValue('available', myBalances) || BN_ZERO)
-    : undefined;
+  const hasEnoughDeposit = checkDepositAvailability(
+    fastUnstakeDeposit,
+    stakingConsts,
+    myBalances,
+    estimatedFee
+  );

-  const hasUnlockingAndRedeemable = redeemable && stakingAccount
-    ? !!(!redeemable.isZero() || stakingAccount.unlocking?.length)
-    : undefined;
+  const hasUnlockingAndRedeemable = checkUnlockingStatus(redeemable, stakingAccount);
packages/extension-polkagate/src/popup/staking/pool/stake/bondExtra/Review.tsx (2)

11-16: Consider renaming the imported Proxy type to avoid shadowing.

The imported Proxy type shadows the global Proxy object, which could lead to confusion. Consider renaming it to something more specific like ProxyConfig or ProxyDetails.

-import type { MyPoolInfo, Proxy, ProxyItem, TxInfo } from '../../../../../util/types';
+import type { MyPoolInfo, ProxyConfig, ProxyItem, TxInfo } from '../../../../../util/types';

Note: This would require updating the type definition in the util/types file and all its usages.

🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 89-106: Enhance error handling in the BondExtra function.

While the function catches errors, it only logs them to console and sets a generic password error flag. Consider:

  1. Adding specific error messages for different failure scenarios
  2. Distinguishing between password errors and other transaction failures
 } catch (e) {
-  console.log('error:', e);
+  console.error('BondExtra failed:', e);
+  if (e.message.includes('password')) {
     setIsPasswordError(true);
+  } else {
+    // Handle other transaction errors
+    onAction('error', e.message);
+  }
 }
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/partial/SelectValidatorsReview.tsx (1)

Line range hint 91-122: Consider enhancing error handling in the nominate function

While the error handling is generally good, consider providing more specific error feedback to users instead of just logging to console.

    } catch (e) {
      console.log('error:', e);
+     const errorMessage = e instanceof Error ? e.message : 'An unknown error occurred';
+     // Consider adding a user-friendly error notification here
      setIsPasswordError(true);
    }
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/rewards/Review.tsx (3)

11-15: Consider renaming the Proxy type to avoid shadowing.

While the imports enhance type safety, the Proxy type shadows the global Proxy object. Consider renaming it to something more specific like BlockchainProxy or ChainProxy to avoid potential confusion.

🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


60-62: Add error handling for API unavailability.

While the optional chaining improves robustness, consider adding explicit error handling when the API or its methods are unavailable. This would provide better feedback to users.

  useEffect((): void => {
    if (!payoutStakers || !batch || !selectedToPayout) {
+     console.warn('Required API methods are not available');
      return;
    }

    const _tx = selectedToPayout.length === 1
      ? payoutStakers(selectedToPayout[0][1], Number(selectedToPayout[0][0]), selectedToPayout[0][2])
      : batch(selectedToPayout.map((p) => payoutStakers(p[1], Number(p[0]), p[2])));

    setTx(_tx);
+   
+   if (!_tx) {
+     console.error('Failed to create transaction');
+   }
  }, [batch, payoutStakers, selectedToPayout]);

Also applies to: 76-89


Line range hint 95-134: Enhance error handling in transaction execution.

While the basic error handling is present, consider adding more specific error handling for different failure scenarios:

  • Network errors
  • Insufficient balance
  • Transaction rejected
  const unstake = useCallback(async () => {
    try {
      if (!api || !tx || !formatted || !chain) {
+       console.error('Required dependencies not available');
        return;
      }

      const from = selectedProxyAddress ?? formatted;
      const signer = keyring.getPair(from);

      signer.unlock(password);
      setShowWaitScreen(true);

      const ptx = selectedProxy ? api.tx['proxy']['proxy'](formatted, selectedProxy.proxyType, tx) : tx;
      const { block, failureText, fee, success, txHash } = await signAndSend(api, ptx, signer, formatted);

      // ... rest of the code ...

    } catch (e) {
-     console.log('error:', e);
+     console.error('Transaction failed:', e);
+     
+     if (e.message?.includes('balance')) {
+       // Handle insufficient balance
+     } else if (e.message?.includes('rejected')) {
+       // Handle rejection
+     }
      setIsPasswordError(true);
    }
  }, [/* ... */]);
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/account/unlock/Review.tsx (1)

Line range hint 144-144: Consider handling undefined estimatedFee more explicitly

The current fallback to '0' when estimatedFee is undefined might hide fee calculation issues. Consider adding a validation check or logging when this occurs.

-fee: fee || String(estimatedFee || 0),
+fee: fee || (estimatedFee ? String(estimatedFee) : (() => {
+  console.warn('Estimated fee was undefined');
+  return '0';
+})()),
packages/extension-polkagate/src/fullscreen/governance/delegate/remove/RemoveDelegate.tsx (3)

103-104: Approve fee calculation changes with suggestions for error handling.

The changes correctly wrap the partialFee with api?.createType('Balance', ...) to ensure proper type safety. However, the error handling could be improved.

Consider enhancing the error handling:

-      ? undelegate(BN_ZERO).paymentInfo(formatted).then((i) => setEstimatedFee(api?.createType('Balance', i?.partialFee) as Balance)).catch(console.error)
-      : batch(params.map((param) => undelegate(param))).paymentInfo(formatted).then((i) => setEstimatedFee(api?.createType('Balance', i?.partialFee) as Balance)).catch(console.error);
+      ? undelegate(BN_ZERO).paymentInfo(formatted)
+          .then((i) => {
+            if (!i?.partialFee) {
+              throw new Error('Failed to get partial fee');
+            }
+            setEstimatedFee(api?.createType('Balance', i.partialFee));
+          })
+          .catch((error) => {
+            console.error('Fee estimation failed:', error);
+            // Consider setting an error state here to show in UI
+          })
+      : batch(params.map((param) => undelegate(param))).paymentInfo(formatted)
+          .then((i) => {
+            if (!i?.partialFee) {
+              throw new Error('Failed to get partial fee');
+            }
+            setEstimatedFee(api?.createType('Balance', i.partialFee));
+          })
+          .catch((error) => {
+            console.error('Batch fee estimation failed:', error);
+            // Consider setting an error state here to show in UI
+          });

Line range hint 96-105: Consider extracting fee estimation logic into a custom hook.

The current fee estimation logic could be simplified and made more reusable by extracting it into a custom hook, following the PR's objective of utilizing fee hooks.

Create a custom hook like this:

function useUndelegateFee(
  api: ApiPromise | undefined,
  formatted: string | undefined,
  params: number[] | undefined
): Balance | undefined {
  const [estimatedFee, setEstimatedFee] = useState<Balance>();
  
  useEffect(() => {
    if (!formatted || !api?.tx?.convictionVoting?.undelegate || !params?.length) {
      return;
    }

    if (!api?.call?.['transactionPaymentApi']) {
      return setEstimatedFee(api?.createType('Balance', BN_ONE));
    }

    const estimateFee = async () => {
      try {
        const info = params.length === 1
          ? await api.tx.convictionVoting.undelegate(BN_ZERO).paymentInfo(formatted)
          : await api.tx.utility.batchAll(
              params.map((param) => api.tx.convictionVoting.undelegate(param))
            ).paymentInfo(formatted);

        if (!info?.partialFee) {
          throw new Error('Failed to get partial fee');
        }

        setEstimatedFee(api.createType('Balance', info.partialFee));
      } catch (error) {
        console.error('Fee estimation failed:', error);
        // Handle error appropriately
      }
    };

    void estimateFee();
  }, [api, formatted, params]);

  return estimatedFee;
}

This would simplify the component code to:

const estimatedFee = useUndelegateFee(api, formatted, params);

The fallback fee mechanism needs a comprehensive review across the codebase

The search results confirm that this minimal fee fallback pattern (BN_ONE) is used extensively throughout the codebase in various transaction-related components:

  • Multiple staking operations (solo and pool)
  • Governance operations
  • Crowdloan contributions
  • Fund transfers
  • Common hooks (useEstimatedFee.ts)

This widespread usage of a minimal fallback fee is concerning because:

  1. It's used in production code across critical transaction types
  2. The pattern is implemented in a shared hook, suggesting it affects all fee estimations
  3. The minimal fee (BN_ONE) is likely insufficient for any real transaction
  4. There's no user warning when falling back to this minimal fee
🔗 Analysis chain

Line range hint 98-102: Review the fallback fee mechanism.

The current fallback mechanism sets a minimal fee (BN_ONE) when transactionPaymentApi is unavailable. This might be insufficient for actual transactions.

Let's verify if this fallback is used in production:

Consider implementing a more robust fallback mechanism:

  1. Use a configurable minimum fee based on the network
  2. Show a warning to users when falling back to default fee
  3. Consider preventing transaction submission when accurate fee estimation isn't available
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other instances of this fallback pattern
rg -A 2 "if.*!api\?\.call\?\.\['transactionPaymentApi'\]"

Length of output: 9293

packages/extension-polkagate/src/fullscreen/governance/post/decisionDeposit/index.tsx (1)

6-6: Rename the Proxy type to avoid shadowing global Proxy

Consider renaming the imported Proxy type to something more specific like ProxyConfig or ProxyDefinition to avoid shadowing the global Proxy object.

-import type { Proxy, ProxyItem, TxInfo } from '../../../../util/types';
+import type { Proxy as ProxyConfig, ProxyItem, TxInfo } from '../../../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 6-6: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (1)

Line range hint 115-120: Consider improving effect hook cleanup and error handling.

The effect hook that calls getFee could benefit from:

  1. Proper cleanup to prevent state updates on unmounted component
  2. Better error handling beyond just logging to console

Consider this improvement:

  useEffect(() => {
+   let isSubscribed = true;
    if (amountAsBN && redeem && chilled && maxUnlockingChunks && unlockingLen !== undefined && unbonded && formatted && staked) {
-     getFee().catch(console.error);
+     getFee()
+       .then(() => {
+         if (!isSubscribed) return;
+         // fee is set in getFee function
+       })
+       .catch((error) => {
+         if (!isSubscribed) return;
+         console.error('Fee estimation failed:', error);
+         setAlert(t('Failed to estimate transaction fee'));
+       });
    }
+   return () => {
+     isSubscribed = false;
+   };
  }, [amountAsBN, api, chilled, formatted, getFee, maxUnlockingChunks, redeem, staked, unbonded, unlockingLen]);
packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (3)

14-14: Rename shadowed Proxy type to avoid confusion

The Proxy type is shadowing the global Proxy property. Consider renaming it to something more specific like StakingProxy or ChainProxy to avoid potential confusion and maintain clarity.

-import type { Proxy, ProxyItem, TxInfo } from '../../../../util/types';
+import type { Proxy as StakingProxy, ProxyItem, TxInfo } from '../../../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 28-45: Consider standardizing amount types

The Props interface uses mixed types for amounts:

  • amount: string
  • staked: BN
  • total: BN | undefined

Consider standardizing all amount-related props to use BN for consistency and type safety. This would help prevent potential conversion errors and make the code more maintainable.

interface Props {
  address: string;
-  amount: string;
+  amount: BN;
  // ... other props
}

If string representation is needed for display purposes, consider handling the conversion at the display level rather than in the props.

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 89-143: Enhance error handling and function organization

The unstake function handles multiple complex operations but could benefit from several improvements:

  1. Error handling is too generic. Consider catching specific errors and providing appropriate user feedback.
  2. The function is handling multiple responsibilities (transaction building, signing, history saving). Consider breaking it down into smaller, focused functions.
  3. Critical operations lack explanatory comments.
  4. Success/failure handling could be more robust.

Consider refactoring like this:

const buildUnstakingTransactions = async () => {
  const txs = [];
  
  // Handle redeeming if max unlocking chunks reached
  if (unlockingLen >= maxUnlockingChunks) {
    const optSpans = await api.query['staking']['slashingSpans'](formatted);
    const spanCount = optSpans.isNone ? 0 : optSpans.unwrap().prior.length + 1;
    txs.push(redeem(spanCount));
  }
  
  // Handle chilling if unstaking all
  if ((isUnstakeAll || amount === amountToHuman(staked, decimal)) && hasNominator) {
    txs.push(chilled());
  }
  
  // Add main unbond transaction
  const amountAsBN = amountToMachine(amount, decimal);
  txs.push(unbonded(amountAsBN));
  
  return txs;
};

const unstake = useCallback(async () => {
  try {
    if (!api || !chain || !formatted || !unbonded || !redeem || !chilled || hasNominator === undefined) {
      throw new Error('Required dependencies not initialized');
    }

    const from = selectedProxyAddress ?? formatted;
    const signer = keyring.getPair(from);

    try {
      signer.unlock(password);
    } catch (e) {
      setIsPasswordError(true);
      throw new Error('Invalid password');
    }

    setShowWaitScreen(true);
    
    const txs = await buildUnstakingTransactions();
    const transaction = txs.length > 1 
      ? api.tx['utility']['batchAll'](txs)
      : txs[0];
    
    const finalTx = selectedProxy 
      ? api.tx['proxy']['proxy'](formatted, selectedProxy.proxyType, transaction)
      : transaction;

    const result = await signAndSend(api, finalTx, signer, formatted);
    
    const info = {
      action: 'Solo Staking',
      amount,
      block: result.block,
      date: Date.now(),
      failureText: result.failureText,
      fee: result.fee || String(estimatedFee || 0),
      from: { address: String(formatted), name },
      subAction: 'Unstake',
      success: result.success,
      throughProxy: selectedProxyAddress 
        ? { address: selectedProxyAddress, name: selectedProxyName }
        : undefined,
      txHash: result.txHash
    };

    setTxInfo({ ...info, api, chain });
    saveAsHistory(from, info);
    
    setShowWaitScreen(false);
    setShowConfirmation(true);
  } catch (e) {
    console.error('Unstaking error:', e);
    setIsPasswordError(true);
    setShowWaitScreen(false);
  }
}, [/* ... dependencies ... */]);
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/myPool/SetState.tsx (1)

Line range hint 108-108: Consider handling zero fee edge case

The fallback to String(estimatedFee || 0) when fee is undefined could mask potential issues. Consider adding a warning or preventing the transaction if estimatedFee is unexpectedly zero or undefined.

-fee: fee || String(estimatedFee || 0),
+fee: fee || (estimatedFee ? String(estimatedFee) : (() => { 
+  console.warn('Estimated fee is unexpectedly zero or undefined');
+  return '0';
+})()),
🧰 Tools
🪛 Biome (1.9.4)

[error] 7-7: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/stake/solo/fastUnstake/index.tsx (3)

48-49: Consider improving the optional chaining pattern

The current pattern inputs?.call?.() could be more concisely written and potentially avoid unnecessary function creation.

-const estimatedFee = useEstimatedFee(address, inputs?.call?.());
+const estimatedFee = useEstimatedFee(address, inputs?.call && inputs.call());

Line range hint 52-56: Consider adding explicit zero checks for better safety

The deposit check could be made more robust by adding explicit zero checks for the involved values.

 const hasEnoughDeposit = fastUnstakeDeposit && stakingConsts && myBalances && estimatedFee && availableBalance
-    ? new BN(fastUnstakeDeposit).add(estimatedFee).lt(availableBalance || BN_MAX_INTEGER)
+    ? !fastUnstakeDeposit.isZero() && !estimatedFee.isZero() 
+      ? new BN(fastUnstakeDeposit).add(estimatedFee).lt(availableBalance || BN_MAX_INTEGER)
+      : false
     : undefined;

Line range hint 119-122: Consider improving accessibility

The NumberPassFail component could benefit from additional accessibility attributes.

 const NumberPassFail = ({ condition, no }: { condition: boolean | undefined, no: number }) => (
     condition
-      ? <CheckCircleOutlineSharpIcon sx={{ bgcolor: 'success.main', borderRadius: '50%', color: '#fff', fontSize: '20px', ml: '-1px' }} />
+      ? <CheckCircleOutlineSharpIcon 
+          aria-label={t('Requirement passed')}
+          role="img"
+          sx={{ bgcolor: 'success.main', borderRadius: '50%', color: '#fff', fontSize: '20px', ml: '-1px' }} 
+        />
       : <Typography fontSize='13px' sx={{ bgcolor: condition === undefined ? 'action.disabledBackground' : 'warning.main', border: '1px solid', borderColor: '#fff', borderRadius: '50%', color: 'white', height: '18px', lineHeight: 1.4, textAlign: 'center', width: '18px' }}>
         {no}
       </Typography>
 );
packages/extension-polkagate/src/fullscreen/governance/post/castVote/index.tsx (1)

Line range hint 1-248: Consider architectural improvements for better maintainability

The component has grown complex with multiple responsibilities and state variables. Consider:

  1. Breaking down the step-based rendering into separate components
  2. Using useReducer for managing related state (step, alterType, voteInformation)
  3. Extracting vote-related logic into custom hooks

Example structure:

// hooks/useVoteState.ts
function useVoteState() {
  return useReducer(voteReducer, initialState);
}

// components/VoteSteps/
- About.tsx
- Cast.tsx
- Preview.tsx
- Review.tsx
- Confirmation.tsx

// components/VoteModal.tsx (main component)
function VoteModal() {
  const [state, dispatch] = useVoteState();
  // Simplified logic focusing on orchestration
}
packages/extension-polkagate/src/popup/manageProxies/Review.tsx (3)

Line range hint 82-108: Enhance error handling in transaction execution.

While the transaction execution logic is solid, the error handling could be more informative for users. Consider capturing and displaying specific error messages instead of just logging to console.

    } catch (e) {
-     console.log('error:', e);
+     console.error('Transaction failed:', e);
+     setHelperText(e?.message || t<string>('An error occurred while processing the transaction'));
      setIsPasswordError(true);
    }

Line range hint 166-166: Remove commented out estimatedFee prop.

The commented out estimatedFee prop appears to be leftover from the migration to hooks. Clean up by removing this line if it's no longer needed.

-       // estimatedFee={estimatedFee}
🧰 Tools
🪛 Biome (1.9.4)

[error] 10-10: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


36-36: Consider splitting the Review component for better maintainability.

The Review component handles multiple responsibilities including proxy management, fee estimation, transaction execution, and state management. Consider breaking it down into smaller, more focused components:

  • ProxyManagementForm
  • TransactionExecutor
  • FeeCalculator

This would improve maintainability and make the code easier to test.

packages/extension-polkagate/src/fullscreen/manageProxies/Review.tsx (4)

10-10: Rename the Proxy type to avoid shadowing global object

The Proxy type shadows the global Proxy object, which could lead to confusion. Consider renaming it to something more specific like ProxyConfig or ProxyDefinition.

-import type { Proxy, ProxyItem, TxInfo } from '../../util/types';
+import type { ProxyConfig as Proxy, ProxyItem, TxInfo } from '../../util/types';
🧰 Tools
🪛 Biome (1.9.4)

[error] 10-10: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


95-97: Simplify the review text construction

The current string concatenation logic is complex and hard to read. Consider using an array-based approach for better readability.

-      reviewText: `You are ${toAdds && toAdds > 0 ? `adding ${toAdds} ${toRemoves && toRemoves > 0 ? ' and' : ''}` : ''} ${toRemoves && toRemoves > 0 ? `removing ${toRemoves}` : ''} ${(toAdds ?? 0) + (toRemoves ?? 0) > 1 ? 'proxies' : 'proxy'}`
+      reviewText: (() => {
+        const actions = [];
+        if (toAdds && toAdds > 0) actions.push(`adding ${toAdds}`);
+        if (toRemoves && toRemoves > 0) actions.push(`removing ${toRemoves}`);
+        const proxyText = (toAdds ?? 0) + (toRemoves ?? 0) > 1 ? 'proxies' : 'proxy';
+        return `You are ${actions.join(' and ')} ${proxyText}`;
+      })()

Line range hint 101-117: Add error handling for empty transaction batch

The transaction building logic should handle the case where no valid transactions are generated.

   const call = useMemo(() => {
     if (!removeProxy || !addProxy || !batchAll) {
       return undefined;
     }
 
     const temp: SubmittableExtrinsic<'promise'>[] = [];
 
     proxyItems?.forEach(({ proxy, status }) => {
       const { delay, delegate, proxyType } = proxy;
 
       status === 'remove' && temp.push(removeProxy(delegate, proxyType, delay));
       status === 'new' && temp.push(addProxy(delegate, proxyType, delay));
     });
 
+    if (temp.length === 0) {
+      return undefined;
+    }
+
     return temp.length > 1
       ? batchAll(temp)
       : temp[0];
   }, [addProxy, batchAll, proxyItems, removeProxy]);

Line range hint 192-192: Extract complex disabled state logic

The disabled state calculation combines multiple conditions. Consider extracting it to a separate function or hook for better readability and testability.

+  const isSigningDisabled = useMemo(() => {
+    return !depositToPay || 
+           feeAndDeposit.isAbleToPay !== true || 
+           !changedItems || 
+           changedItems.length === 0;
+  }, [depositToPay, feeAndDeposit.isAbleToPay, changedItems]);
+
   <SignArea2
     address={address}
     call={call}
-    disabled={!depositToPay || feeAndDeposit.isAbleToPay !== true || !changedItems || changedItems.length === 0}
+    disabled={isSigningDisabled}
     // ... other props
   />
🧰 Tools
🪛 Biome (1.9.4)

[error] 10-10: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/unstake/Review.tsx (5)

Line range hint 34-49: Add JSDoc documentation for Props interface.

Consider adding JSDoc comments to document the purpose and requirements of each prop, especially for critical parameters like estimatedFee, maxUnlockingChunks, and unstakeAllAmount. This would improve maintainability and help future developers understand the component's requirements.

🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 58-71: Consider using useReducer for complex state management.

The component manages multiple related state variables (password, isPasswordError, selectedProxy, etc.). Consider using useReducer to centralize state management and make state transitions more predictable.

🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 93-157: Refactor unstake function for better maintainability.

The unstake function is complex and handles multiple responsibilities. Consider:

  1. Extracting the batch transaction logic into a separate function
  2. Creating a dedicated function for saving transaction history
  3. Improving error handling to provide more specific error messages to users

Example refactor:

const saveTxHistory = (params: { from: string, txHash: string, /* ... */ }) => {
  // ... history saving logic
};

const createBatchTx = async (spanCount: number) => {
  return api.tx.utility.batchAll([
    poolWithdrawUnbonded(poolId, spanCount),
    unbonded(...params)
  ]);
};

const unstake = async () => {
  try {
    validateInputs();
    const tx = await prepareTx();
    const result = await executeTx(tx);
    await saveTxHistory(result);
    handleSuccess();
  } catch (e) {
    handleError(e);
  }
};
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 159-241: Enhance accessibility for better user experience.

Consider adding:

  1. ARIA labels for interactive elements
  2. Keyboard navigation support
  3. Error announcements for screen readers
  4. Loading state indicators for async operations
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Password state should be handled more securely

Based on the codebase analysis, there's a consistent pattern across multiple components where password state is managed without proper cleanup mechanisms. Specific recommendations:

  1. Add a cleanup effect to clear the password state:
useEffect(() => {
  return () => setPassword(undefined);
}, []);
  1. Add a timeout to automatically clear the password after use:
useEffect(() => {
  if (password) {
    const timer = setTimeout(() => setPassword(undefined), 60000); // 1 minute
    return () => clearTimeout(timer);
  }
}, [password]);
  1. Use a secure password input component that prevents password manager auto-fill by setting autocomplete="new-password".
🔗 Analysis chain

Line range hint 58-71: Review password handling security.

The password is stored in component state. Consider:

  1. Clearing the password state after use
  2. Using a secure input field that prevents password manager auto-fill
  3. Adding a timeout to automatically clear sensitive state
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for other password handling implementations in the codebase
rg -A 3 'password.*useState.*' --type typescript

Length of output: 85


Script:

#!/bin/bash
# Check for password handling patterns in the codebase
rg -A 3 'password.*useState'

# Check for password clearing implementations
rg -A 5 'setPassword\(.*undefined'

# Look for secure password input components
ast-grep --pattern 'type Password = { $$$'
ast-grep --pattern 'interface Password { $$$'

Length of output: 21594


Script:

#!/bin/bash
# Check for password clearing patterns in useEffect or cleanup functions
rg -A 5 'useEffect.*setPassword.*undefined'

# Look for password input components
ast-grep --pattern 'function PasswordInput'
ast-grep --pattern 'const PasswordInput'

# Check for password-related utility functions
rg -A 3 'clearPassword|resetPassword'

Length of output: 174

🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/stake/createPool/index.tsx (2)

Line range hint 1-240: Consider architectural improvements for better maintainability.

The component could benefit from several architectural improvements:

  1. Split the component into smaller, more focused components
  2. Add error boundaries
  3. Add loading states
  4. Consolidate state management

Consider these improvements:

  1. Extract the roles management into a separate component
  2. Use a reducer for state management
  3. Add loading indicators for async operations
  4. Add error boundaries for better error handling

Would you like me to provide a detailed implementation for any of these improvements?

🧰 Tools
🪛 Biome (1.9.4)

[error] 160-162: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


Remove @ts-nocheck and fix type issues properly across the codebase.

The search results show widespread usage of @ts-nocheck across the codebase, particularly in the extension-polkagate package. This indicates a systemic issue where TypeScript type checking is being bypassed instead of properly addressing type safety. This practice:

  • Increases the risk of runtime errors
  • Makes the codebase harder to maintain
  • Reduces code quality and reliability
  • Makes refactoring more difficult
  • Defeats the purpose of using TypeScript

Key areas needing attention:

  • Most components in packages/extension-polkagate/src/popup/
  • Utility functions in packages/extension-polkagate/src/util/
  • Hooks in packages/extension-polkagate/src/hooks/
  • Worker files in packages/extension-polkagate/src/util/workers/
🔗 Analysis chain

Line range hint 3-3: Remove @ts-nocheck and fix type issues properly.

Using @ts-nocheck suppresses all TypeScript checks, which could hide potential runtime errors. Instead, fix the type issues properly to maintain type safety.

Let's check for other instances of @ts-nocheck in the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find other files with @ts-nocheck
rg "@ts-nocheck" --type typescript

Length of output: 69


Script:

#!/bin/bash
# Find other files with @ts-nocheck using ripgrep without file type filter
rg "@ts-nocheck"

Length of output: 23466

🧰 Tools
🪛 Biome (1.9.4)

[error] 160-162: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/extension-polkagate/src/popup/staking/solo/settings/Review.tsx (2)

130-132: Consider extracting fee estimation to a custom hook.

The current implementation mixes transaction preparation with fee estimation logic. Consider creating a custom useTransactionFee hook to:

  • Separate concerns
  • Improve reusability
  • Handle errors more gracefully with user feedback

Example implementation:

const useTransactionFee = (tx: SubmittableExtrinsic<'promise'> | undefined, formatted: string) => {
  const [estimatedFee, setEstimatedFee] = useState<Balance>();
  const [error, setError] = useState<Error>();

  useEffect(() => {
    if (!tx) return;

    tx.paymentInfo(formatted)
      .then((i) => setEstimatedFee(api.createType('Balance', i?.partialFee ?? BN_ZERO) as Balance))
      .catch((e) => {
        console.error(e);
        setError(e);
      });
  }, [tx, formatted]);

  return { estimatedFee, error };
};
🧰 Tools
🪛 Biome (1.9.4)

[error] 131-131: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


Line range hint 1-324: Consider architectural improvements for better maintainability.

The component has grown complex with multiple responsibilities. Consider these improvements:

  1. Split into smaller components (e.g., separate components for transaction preparation, proxy selection)
  2. Use a reducer to manage related state (password, transaction, confirmation states)
  3. Implement comprehensive error handling beyond password errors

Example state management with reducer:

type State = {
  password: string | undefined;
  isPasswordError: boolean;
  showWaitScreen: boolean;
  showConfirmation: boolean;
  txInfo: TxInfo | undefined;
};

type Action =
  | { type: 'SET_PASSWORD'; payload: string }
  | { type: 'SET_PASSWORD_ERROR'; payload: boolean }
  | { type: 'SET_WAIT_SCREEN'; payload: boolean }
  | { type: 'SET_CONFIRMATION'; payload: boolean }
  | { type: 'SET_TX_INFO'; payload: TxInfo };

const reducer = (state: State, action: Action): State => {
  switch (action.type) {
    case 'SET_PASSWORD':
      return { ...state, password: action.payload };
    // ... other cases
  }
};
🧰 Tools
🪛 Biome (1.9.4)

[error] 131-131: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/extension-polkagate/src/fullscreen/stake/solo/stake/index.tsx (1)

Line range hint 122-141: Consider improving fee fallback handling

In the extraInfo object, the fee is set with String(estimatedFee || 0). This could potentially mislead users if the fee estimation is still loading, as it would show 0 instead of indicating that the fee is being calculated.

Consider this improvement:

       const extraInfo = {
         action: 'Solo Staking',
         amount,
-        fee: String(estimatedFee || 0),
+        fee: estimatedFee === undefined ? 'Calculating...' : String(estimatedFee),
         subAction: 'Stake'
       };

Also, consider adding a loading state to prevent users from proceeding until the fee is properly calculated:

   const buttonDisable = useMemo(() => 
-    !!alert || !amount
-    [alert, amount]
+    !!alert || !amount || estimatedFee === undefined,
+    [alert, amount, estimatedFee]
   );
packages/extension-polkagate/src/popup/staking/solo/rewards/PendingRewards.tsx (3)

Line range hint 65-71: Consider improving type safety for state variables

While the state management is correct, consider using a more specific type for selectedToPayout instead of ExpandedRewards[].

-const [selectedToPayout, setSelectedToPayout] = useState<ExpandedRewards[]>([]);
+const [selectedToPayout, setSelectedToPayout] = useState<readonly [string, string, number, BN][]>([]);

Line range hint 73-106: Simplify rewards transformation and improve type safety

The rewards transformation logic could be simplified and made more type-safe using array methods.

-    const rewardsArray: [string, string, number, BN][] = Object.entries(rewards || {}).reduce<[string, string, number, BN][]>(
-      (acc, [era, eraRewards]) => {
-        const eraRewardsArray = Object.entries(eraRewards || {}).reduce<[string, string, number, BN][]>(
-          (eraAcc, [validator, [page, amount]]) => {
-            eraAcc.push([era, validator, page, amount] as [string, string, number, BN]);
-
-            return eraAcc;
-          },
-          []
-        );
-
-        return acc.concat(eraRewardsArray);
-      },
-      []
-    );
+    const rewardsArray = Object.entries(rewards ?? {}).flatMap(([era, eraRewards]) =>
+      Object.entries(eraRewards ?? {}).map(([validator, [page, amount]]) =>
+        [era, validator, page, amount] as const
+      )
+    );

Line range hint 127-147: Improve readability of era date calculation

The eraToDate function's logic is correct but could benefit from clearer variable names and documentation.

 const eraToDate = useCallback((era: number): string | undefined => {
+  // Return early if required dependencies are not available
   if (!(currentBlock && historyDepth && era && forcing && progress && progress.sessionLength.gt(BN_ONE))) {
     return undefined;
   }

-  const EndEraInBlock =
+  const blocksUntilExpiry =
     (forcing.isForceAlways
       ? progress.sessionLength
       : progress.eraLength
     ).mul(
       historyDepth
         .sub(progress.activeEra)
         .addn(era)
         .add(BN_ONE)
     ).sub(
       forcing.isForceAlways
         ? progress.sessionProgress
         : progress.eraProgress);

-  return EndEraInBlock ? blockToDate(EndEraInBlock.addn(currentBlock).toNumber(), currentBlock, { day: 'numeric', month: 'short' }) : undefined;
+  return blocksUntilExpiry
+    ? blockToDate(
+        blocksUntilExpiry.addn(currentBlock).toNumber(),
+        currentBlock,
+        { day: 'numeric', month: 'short' }
+      )
+    : undefined;
 }, [currentBlock, forcing, historyDepth, progress]);
packages/extension-polkagate/src/fullscreen/governance/delegate/index.tsx (2)

Line range hint 1-400: Consider architectural improvements for better maintainability.

While the current implementation is functional, consider these improvements:

  1. Extract fee estimation logic into a custom hook (e.g., useEstimatedFee) to improve reusability and reduce component complexity.
  2. Consider using a reducer or context for state management instead of multiple useState hooks.
  3. Split the component into smaller, more focused components based on delegation status.

Example structure for the fee estimation hook:

function useEstimatedFee(
  api: ApiPromise,
  delegate: any,
  batch: any,
  delegateInfo?: DelegateInformation
): Balance | undefined {
  const [estimatedFee, setEstimatedFee] = useState<Balance>();

  useEffect(() => {
    // ... existing fee estimation logic
  }, [api, batch, delegate, delegateInfo]);

  return estimatedFee;
}

219-223: Improve error handling in fee estimation.

The current error handling only logs to console, which might leave users unaware of fee estimation failures. Consider:

  1. Surfacing errors to the user through a notification system
  2. Adding retry logic for transient failures
  3. Implementing proper error boundaries

Example implementation:

.then((i) => setEstimatedFee(api?.createType('Balance', i?.partialFee) as Balance))
.catch((error) => {
  console.error('Fee estimation failed:', error);
  notifyUser('Failed to estimate transaction fee. Please try again.');
  setEstimatedFee(undefined);
});

Also applies to: 231-234

packages/extension-polkagate/src/popup/staking/solo/stake/index.tsx (1)

124-131: Consider improving fee estimation robustness.

The current fee estimation implementation could benefit from several improvements:

  1. Race conditions could occur if dependencies change rapidly
  2. Pending promises aren't cleaned up on unmount
  3. No loading state during fee estimation

Consider these improvements:

  useEffect(() => {
+   let isSubscribed = true;
+   setEstimatedFee(undefined);
+
    if (!tx || !api || !formatted || !nominated || !batchAll) {
      return;
    }

    if (!api?.call?.['transactionPaymentApi']) {
      setEstimatedFee(api.createType('Balance', BN_ONE) as Balance);
      return;
    }

    if (isFirstTimeStaking && autoSelectedValidators?.length) {
      const ids = autoSelectedValidators.map((v) => v.accountId);

      batchAll([tx(...params), nominated(ids)])
        .paymentInfo(formatted)
        .then((i) => {
+         if (isSubscribed) {
            setEstimatedFee(api.createType('Balance', i?.partialFee) as Balance);
+         }
        })
        .catch((error) => {
+         if (isSubscribed) {
            console.error('Fee estimation failed:', error);
            setAlert(t('Failed to estimate transaction fee. Please try again.'));
+         }
        });

      return;
    }

    tx(...params)
      .paymentInfo(formatted)
      .then((i) => {
+       if (isSubscribed) {
          setEstimatedFee(api.createType('Balance', i?.partialFee) as Balance);
+       }
      })
      .catch((error) => {
+       if (isSubscribed) {
          console.error('Fee estimation failed:', error);
          setAlert(t('Failed to estimate transaction fee. Please try again.'));
+       }
      });
+
+   return () => {
+     isSubscribed = false;
+   };
  }, [amountAsBN, api, autoSelectedValidators, batchAll, bond, formatted, isFirstTimeStaking, nominated, params, tx]);
packages/extension-polkagate/src/popup/staking/pool/unstake/index.tsx (2)

149-161: Enhance error handling in fee estimation logic

While the type assertions for Balance are correct, the error handling in the fee estimation chain could be improved to provide better feedback to users when fee estimation fails.

Consider adding error state handling:

 if (!api?.call?.['transactionPaymentApi']) {
-  return setEstimatedFee(api?.createType('Balance', BN_ONE) as Balance);
+  console.warn('Transaction payment API not available, using default fee');
+  return setEstimatedFee(api?.createType('Balance', BN_ONE) as Balance);
 }

 poolWithdrawUnbonded && maxUnlockingChunks && unlockingLen !== undefined && unbonded && formatted && void unbonded(...params).paymentInfo(formatted).then((i) => {
   const fee = i?.partialFee;

   if (unlockingLen < maxUnlockingChunks) {
     setEstimatedFee(api.createType('Balance', fee) as Balance);
   } else {
     const dummyParams = [1, 1];

-    poolWithdrawUnbonded(...dummyParams).paymentInfo(formatted).then((j) => setEstimatedFee(api.createType('Balance', fee.add(j?.partialFee)) as Balance)).catch(console.error);
+    poolWithdrawUnbonded(...dummyParams).paymentInfo(formatted)
+      .then((j) => setEstimatedFee(api.createType('Balance', fee.add(j?.partialFee)) as Balance))
+      .catch((error) => {
+        console.error('Failed to estimate withdrawal fee:', error);
+        setEstimatedFee(undefined); // Reset fee to trigger disabled state
+      });
   }
-}).catch(console.error);
+}).catch((error) => {
+  console.error('Failed to estimate unbond fee:', error);
+  setEstimatedFee(undefined); // Reset fee to trigger disabled state
+});

45-45: Consider architectural improvements for better maintainability

The component handles multiple complex responsibilities and could benefit from being broken down into smaller, more focused components and custom hooks.

Suggestions for improvement:

  1. Extract fee estimation logic into a custom hook:
function useUnstakeFeeEstimation(
  api: ApiPromise,
  formatted: string,
  amount: string,
  decimal: number,
  maxUnlockingChunks: number,
  unlockingLen: number
): Balance | undefined {
  // Move fee estimation logic here
}
  1. Extract pool state logic into a custom hook:
function usePoolStateValidation(
  isPoolDepositor: boolean,
  isPoolRoot: boolean,
  poolState: string,
  poolMemberCounter: number
): {
  helperText: string | undefined,
  helperButton: number | undefined
} {
  // Move pool state validation logic here
}
  1. Consider breaking down the UI into smaller components:
const UnstakeForm = ({ /* props */ }) => {
  // Extract form-related JSX
}

const PoolInfo = ({ /* props */ }) => {
  // Extract pool information display
}

This will improve code maintainability, testability, and reusability.

packages/extension-polkagate/src/fullscreen/stake/solo/pending/index.tsx (1)

282-282: Destructuring pattern reinforces maintainability concerns

The tuple destructuring with skipped elements ([eraIndex, validator,, value]) makes the code harder to maintain. Using an interface would allow for more explicit and maintainable destructuring:

If keeping the interface suggestion from above:

-const [eraIndex, validator,, value] = info;
+const { eraIndex, validator, value } = info;
packages/extension-polkagate/src/fullscreen/governance/delegate/modify/ModifyDelegate.tsx (3)

85-93: Simplify conviction calculation logic

The nested ternary operations make the code difficult to read and maintain.

Consider this more readable approach:

-  const acceptableConviction = useMemo(() =>
-    newConviction !== undefined
-      ? newConviction === 0.1
-        ? 0
-        : newConviction
-      : conviction === 0.1
-        ? 0
-        : conviction
-  , [conviction, newConviction]);
+  const acceptableConviction = useMemo(() => {
+    const activeConviction = newConviction ?? conviction;
+    
+    if (activeConviction === undefined) {
+      return undefined;
+    }
+    
+    return activeConviction === 0.1 ? 0 : activeConviction;
+  }, [conviction, newConviction]);

Line range hint 214-227: Remove debug console.log statement

Debug logging statements should not be present in production code.

  useEffect(() => {
-   console.log('resets');
    setDelegateInformation({
      delegateAmount: newDelegateAmount ?? delegateAmount,
      delegateAmountBN: newDelegateAmountBN.isZero() ? delegateAmountBN : newDelegateAmountBN,
      delegatePower,
      delegateConviction: acceptableConviction,
      delegatedTracks: newSelectedTracks ?? selectedTracks,
      delegateeAddress
    });
  }, [acceptableConviction, delegateAmount, delegateAmountBN, delegatePower, 
      delegateeAddress, newDelegateAmount, newDelegateAmountBN, 
      newSelectedTracks, selectedTracks, setDelegateInformation]);
🧰 Tools
🪛 Biome (1.9.4)

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


17-17: Rename shadowed Proxy type

The current type name shadows the global Proxy type, which could lead to confusion.

- import type { BalancesInfo, Proxy, TxInfo } from '../../../../util/types';
+ import type { BalancesInfo, ProxyConfig, TxInfo } from '../../../../util/types';

// Update all references to Proxy type to use ProxyConfig instead
🧰 Tools
🪛 Biome (1.9.4)

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/governance/post/castVote/Cast.tsx (1)

Consider using the existing useEstimatedFee hook for fee calculation

The codebase has a dedicated useEstimatedFee hook in packages/extension-polkagate/src/hooks/useEstimatedFee.ts that's widely used across the application. In fact, the same hook is already being used in the governance module at packages/extension-polkagate/src/fullscreen/governance/post/castVote/index.tsx with similar parameters.

  • Replace the manual fee calculation in Cast.tsx with the useEstimatedFee hook to maintain consistency with the rest of the codebase
  • Update the imports to include the hook from ../../../hooks/useEstimatedFee
🔗 Analysis chain

Line range hint 169-174: Verify alignment with PR objectives regarding fee hook usage

The PR title mentions "fix: fee issue utilizing fee hook" but the current implementation continues to use direct fee calculation instead of the mentioned hook.

Let's verify if the fee hook is implemented elsewhere and should be used here:

Consider:

  1. Using the mentioned fee hook if it exists
  2. Creating a shared fee hook if it doesn't exist yet
  3. Updating the PR title if the approach has changed
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the fee hook implementation
rg -g '!*.{json,md}' "export.*useEstimatedFee.*=" .

# Search for examples of the fee hook usage
rg -g '!*.{json,md}' "const.*=.*useEstimatedFee\(" .

Length of output: 5221

packages/extension-polkagate/src/fullscreen/socialRecovery/Review.tsx (4)

169-170: Consider memoizing fee-related calculations.

The estimatedFee and feeAndDeposit calculations could trigger unnecessary re-renders. Consider wrapping them in useMemo to optimize performance.

-  const estimatedFee = useEstimatedFee(address, call);
-  const feeAndDeposit = useCanPayFeeAndDeposit(formatted?.toString(), selectedProxy?.delegate, estimatedFee, depositToPay);
+  const estimatedFee = useMemo(() => useEstimatedFee(address, call), [address, call]);
+  const feeAndDeposit = useMemo(
+    () => useCanPayFeeAndDeposit(formatted?.toString(), selectedProxy?.delegate, estimatedFee, depositToPay),
+    [formatted, selectedProxy?.delegate, estimatedFee, depositToPay]
+  );

Line range hint 132-168: Consider breaking down the complex withdrawTXs function.

The function handles multiple transaction types and has complex conditional logic. Consider splitting it into smaller, more focused functions for better maintainability and testability.

+ const buildWithdrawCalls = (withdrawInfo: WithdrawInfo): SubmittableExtrinsic<'promise', ISubmittableResult>[] => {
+   const calls: SubmittableExtrinsic<'promise', ISubmittableResult>[] = [];
+   
+   withdrawInfo.isRecoverable && calls.push(removeRecovery());
+   !(withdrawInfo.soloStaked.isZero()) && calls.push(chill(), unbonded(withdrawInfo.soloStaked));
+   !(withdrawInfo.redeemable.amount.isZero()) && calls.push(redeem(withdrawInfo.redeemable.count));
+   // ... other conditions
+   
+   return calls;
+ };

const withdrawTXs = useCallback((): SubmittableExtrinsic<'promise', ISubmittableResult> | undefined => {
  if (!api || !batchAll || /* ... other conditions */) {
    return;
  }

  const tx: SubmittableExtrinsic<'promise', ISubmittableResult>[] = [];
-  const withdrawCalls: SubmittableExtrinsic<'promise', ISubmittableResult>[] = [];
+  const withdrawCalls = buildWithdrawCalls(withdrawInfo);
  
  !withdrawInfo.claimed && tx.push(claimRecovery(withdrawInfo.lost));
  // ... rest of the function
}, [/* dependencies */]);

Line range hint 199-297: Extract WithdrawDetails into a separate component.

The WithdrawDetails component is defined inside the main component, which makes it harder to test and maintain. Consider moving it to a separate file and importing it.

Create a new file WithdrawDetails.tsx:

interface WithdrawDetailsProps {
  step: number;
  withdrawInfo: WithdrawInfo;
  api: ApiPromise;
  chainName: string;
  setNothingToWithdrawNow: (value: boolean) => void;
}

export const WithdrawDetails: React.FC<WithdrawDetailsProps> = ({ 
  step,
  withdrawInfo,
  api,
  chainName,
  setNothingToWithdrawNow
}) => {
  // ... existing implementation
};

Line range hint 132-168: Enhance error handling in withdrawTXs function.

The function should handle API errors and provide meaningful error messages to users. Consider adding try-catch blocks and error boundaries.

const withdrawTXs = useCallback((): SubmittableExtrinsic<'promise', ISubmittableResult> | undefined => {
+ try {
    if (!api || !batchAll || /* ... other conditions */) {
+     throw new Error('Required dependencies are not available');
      return;
    }

    // ... existing implementation
+  } catch (error) {
+    console.error('Error in withdrawTXs:', error);
+    throw new Error(`Failed to prepare withdrawal transactions: ${error.message}`);
+  }
}, [/* dependencies */]);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a7df2d8 and cae5351.

📒 Files selected for processing (43)
  • packages/extension-polkagate/src/fullscreen/accountDetails/unlock/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/delegate/index.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/delegate/modify/ModifyDelegate.tsx (5 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/delegate/remove/RemoveDelegate.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/post/castVote/Cast.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/post/castVote/index.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/post/decisionDeposit/index.tsx (4 hunks)
  • packages/extension-polkagate/src/fullscreen/manageProxies/Review.tsx (5 hunks)
  • packages/extension-polkagate/src/fullscreen/socialRecovery/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/SetState.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/manageValidators/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/solo/commonTasks/manageValidators/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/solo/fastUnstake/index.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/solo/pending/index.tsx (3 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/solo/restake/index.tsx (3 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/solo/stake/index.tsx (2 hunks)
  • packages/extension-polkagate/src/hooks/useIdentity.ts (1 hunks)
  • packages/extension-polkagate/src/popup/account/unlock/Review.tsx (4 hunks)
  • packages/extension-polkagate/src/popup/manageProxies/Review.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/send/Review.tsx (1 hunks)
  • packages/extension-polkagate/src/popup/send/partial/SendTxDetail.tsx (0 hunks)
  • packages/extension-polkagate/src/popup/staking/partial/SelectValidatorsReview.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/myPool/SetState.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/nominations/remove/index.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/redeem/index.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/rewards/Stake.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/rewards/Withdraw.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/stake/bondExtra/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/stake/createPool/index.tsx (1 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/unstake/Review.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/pool/unstake/index.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/fastUnstake/Review.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/fastUnstake/index.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/nominations/remove/index.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/redeem/index.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/restake/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/restake/index.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/rewards/PendingRewards.tsx (1 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/rewards/Review.tsx (3 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/settings/Review.tsx (1 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/stake/index.tsx (1 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (4 hunks)
💤 Files with no reviewable changes (1)
  • packages/extension-polkagate/src/popup/send/partial/SendTxDetail.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension-polkagate/src/popup/staking/solo/restake/Review.tsx

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/manageProxies/Review.tsx

[error] 10-10: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/rewards/Stake.tsx

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/governance/post/decisionDeposit/index.tsx

[error] 6-6: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/rewards/Withdraw.tsx

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/redeem/index.tsx

[error] 12-12: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/manageProxies/Review.tsx

[error] 10-10: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/fastUnstake/Review.tsx

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/unstake/Review.tsx

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/settings/Review.tsx

[error] 131-131: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/extension-polkagate/src/popup/staking/solo/redeem/index.tsx

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/fullscreen/governance/delegate/modify/ModifyDelegate.tsx

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/rewards/Review.tsx

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/stake/bondExtra/Review.tsx

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

🔇 Additional comments (63)
packages/extension-polkagate/src/hooks/useIdentity.ts (2)

19-19: Verify PR scope and purpose

This change to the useIdentity hook, particularly allowing null values for formatted, seems unrelated to the PR's stated purpose of "fix: fee issue utilizing fee hook". Please clarify:

  1. How this change relates to fee handling
  2. Whether this should be in a separate PR
  3. The motivation behind allowing null values for formatted

19-19: Verify the impact of allowing null formatted parameter

The signature change to accept null for the formatted parameter is a breaking change that could affect existing callers. While the implementation handles the null case, we should verify that all callers are updated accordingly.

✅ Verification successful

The null type addition for formatted parameter is safe

All current usages of useIdentity hook are already handling the null/undefined case safely through:

  • Optional chaining when accessing identity properties (e.g., ?.identity?.display)
  • Passing undefined explicitly (e.g., proxyAddress ?? undefined)
  • Type-safe string coercion (e.g., String(formatted))
  • Conditional access patterns
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all usages of useIdentity hook to verify they handle null case
rg -A 2 "useIdentity\(" --type ts

Length of output: 4183

packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/manageValidators/Review.tsx (2)

12-12: LGTM! Import changes align with the new fee estimation approach.

The imports have been properly updated to support the transition to using the useEstimatedFee hook.

Also applies to: 20-20


Line range hint 44-51: Verify that this fix resolves the original fee issue.

While the implementation of useEstimatedFee hook looks good, please verify that this change resolves the specific fee issue mentioned in the PR title.

✅ Verification successful

Let me gather more information about the fee hook implementation to verify if it properly addresses the issue.


The fee hook implementation is properly handling fee estimation

The implementation of useEstimatedFee hook shows robust fee handling with proper error cases:

  • Handles undefined address or call
  • Provides fallback for missing transaction payment API
  • Properly manages async fee estimation with error handling
  • Correctly updates fee state based on dependencies

The hook is consistently used across 30+ locations in the codebase, showing a standardized approach to fee estimation. The implementation aligns with the PR's objective to fix fee-related issues.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related fee issues or TODOs in the codebase
rg -i "fee.*issue|fee.*todo" --type ts --type tsx

# Check for other instances of fee estimation that might need similar updates
ast-grep --pattern 'useEstimatedFee($$$)'

Length of output: 4967


Script:

#!/bin/bash
# Check the implementation of useEstimatedFee hook
fd useEstimatedFee.ts --exec cat {}

# Look for recent changes or fixes related to fee estimation
rg -i "fix.*fee|fee.*fix" --type ts

Length of output: 1617

packages/extension-polkagate/src/fullscreen/stake/solo/commonTasks/manageValidators/Review.tsx (2)

19-19: LGTM! Clean import organization.

The addition of useEstimatedFee to the existing hook imports maintains good code organization by grouping related hooks together.


46-46: Verify the hook implementation and usage.

Let's ensure the useEstimatedFee hook is properly implemented and consistently used across the codebase.

✅ Verification successful

Let me gather more information about the hook implementation since the previous search didn't show its definition.


Let me check the implementation details of the hook.


Hook implementation and usage are correct and consistent

The useEstimatedFee hook is properly implemented with:

  • Correct type definitions and dependencies
  • Proper error handling and fallback mechanisms
  • Consistent usage across 30+ components in the codebase
  • Appropriate handling of different parameter types (function calls and direct extrinsics)

The usage in Review.tsx follows the same pattern as other components, correctly passing the required parameters (address, call, and params).

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the implementation and usage of useEstimatedFee hook

# Check the hook implementation
echo "Checking hook implementation:"
ast-grep --pattern 'export function useEstimatedFee($_) {
  $$$
}'

# Check all usages of the hook
echo -e "\nChecking hook usage patterns:"
rg 'useEstimatedFee\(' -A 2

Length of output: 12233


Script:

#!/bin/bash
# Search for the hook definition in hooks directory and any utility files
echo "Searching for useEstimatedFee definition:"
fd -e ts -e tsx | xargs rg -l "useEstimatedFee" | xargs rg "export.*useEstimatedFee" -A 5

# Check imports to locate the hook
echo -e "\nChecking imports of useEstimatedFee:"
rg "import.*useEstimatedFee"

Length of output: 7713


Script:

#!/bin/bash
# Get the full implementation of useEstimatedFee
echo "Getting useEstimatedFee implementation:"
cat packages/extension-polkagate/src/hooks/useEstimatedFee.ts

# Check the types and dependencies
echo -e "\nChecking imports and types used by the hook:"
rg "import.*Balance|import.*SubmittableExtrinsic|import.*AnyTuple" packages/extension-polkagate/src/hooks/useEstimatedFee.ts

Length of output: 2030

packages/extension-polkagate/src/popup/staking/solo/restake/index.tsx (2)

17-17: LGTM: Import changes align with the new fee estimation approach

The addition of useEstimatedFee hook and removal of Balance import reflect the improved approach to fee estimation.


55-55: 🛠️ Refactor suggestion

Consider optimizing fee estimation and adding error handling

While using useEstimatedFee hook simplifies the code, consider these improvements:

  1. Handle potential fee estimation errors
  2. Optimize hook dependencies to prevent unnecessary recalculations
-  const estimatedFee = useEstimatedFee(address, rebonded, [amountAsBN]);
+  const estimatedFee = useEstimatedFee(
+    address,
+    rebonded,
+    [amountAsBN],
+    {
+      onError: (error) => {
+        console.error('Fee estimation failed:', error);
+        setAlert(t('Failed to estimate transaction fee. Please try again.'));
+      }
+    }
+  );

Let's verify the hook implementation and its error handling:

packages/extension-polkagate/src/fullscreen/stake/pool/commonTasks/SetState.tsx (2)

19-19: LGTM: Import changes align with the hook migration

The addition of useEstimatedFee and removal of unused imports improve code cleanliness.


41-42: LGTM: Clean implementation of the fee estimation hook

The useEstimatedFee hook provides a more maintainable approach to fee estimation by:

  • Eliminating manual state management
  • Automatically handling fee updates when dependencies change
  • Improving type safety
packages/extension-polkagate/src/popup/staking/solo/nominations/remove/index.tsx (2)

Line range hint 157-177: LGTM: Proper fee handling in password confirmation

The integration with PasswordUseProxyConfirm is well-implemented, correctly passing the estimated fee for validation during the confirmation step.

🧰 Tools
🪛 Biome (1.9.4)

[error] 13-13: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 81-116: Verify fee estimation with proxy transactions

The fee handling looks correct, but we should verify that the useEstimatedFee hook properly accounts for proxy transactions when selected.

✅ Verification successful

Fee estimation correctly handles proxy transactions

The implementation is verified to work correctly with proxy transactions because:

  1. The useEstimatedFee hook receives the final transaction that includes the proxy wrapper when needed, as the broadcast utility wraps the original extrinsic with api.tx['proxy']['proxy'] before passing it for fee estimation
  2. The fee estimation is performed using paymentInfo() which calculates the complete transaction fee including any proxy overhead
  3. The proxy transaction construction in broadcast utility properly maintains the fee estimation context by constructing the complete transaction before fee calculation
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for proxy-related fee calculations in the useEstimatedFee hook implementation
ast-grep --pattern 'export function useEstimatedFee($$$)' 

# Search for proxy transaction handling in the hook
rg -A 10 "useEstimatedFee.*proxy" 

Length of output: 95


Script:

#!/bin/bash
# Let's first locate the useEstimatedFee hook implementation
fd useEstimatedFee

# Then search for any fee estimation logic with proxy support
rg -l "useEstimatedFee|estimatedFee"

# Look for proxy-related fee calculations in general
rg -A 5 "calculateFee.*proxy|estimateFee.*proxy"

Length of output: 5575


Script:

#!/bin/bash
# Let's examine the useEstimatedFee hook implementation
cat packages/extension-polkagate/src/hooks/useEstimatedFee.ts

# Also check the broadcast utility that's used in the code
rg -A 10 "export.*broadcast.*=" 

Length of output: 1514


Script:

#!/bin/bash
# Let's find where the proxy transaction is prepared
rg -A 5 "batch.*proxy|proxy.*batch"

# Check how proxy transactions are handled in the broadcast utility
rg -A 10 "const broadcast = |function broadcast"

Length of output: 2428


Script:

#!/bin/bash
# Let's examine the broadcast utility implementation to see how it handles proxy transactions
cat packages/extension-polkagate/src/util/api/broadcast.ts

# Also check if there are any proxy-specific fee calculations in the broadcast utility
rg -A 10 "signAndSend.*proxy|proxy.*signAndSend"

Length of output: 1835

🧰 Tools
🪛 Biome (1.9.4)

[error] 13-13: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/rewards/Stake.tsx (3)

62-63: LGTM! Clean implementation of the fee estimation hook

The implementation properly utilizes the useEstimatedFee hook with the required parameters, and the params array is correctly memoized. This change aligns well with the PR objective of fixing fee-related issues.


Line range hint 143-152: LGTM! Consistent fee display implementation

The fee display is properly implemented across both the AmountFee and PasswordUseProxyConfirm components, providing a consistent user experience for fee visualization.

Also applies to: 168-178

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 99-99: Add fee validation before transaction submission

The current implementation falls back to '0' if estimatedFee is undefined. This could lead to transaction failures if the fee estimation fails. Consider adding validation:

-        fee: fee || String(estimatedFee || 0),
+        fee: fee || (estimatedFee 
+          ? String(estimatedFee)
+          : (() => { throw new Error('Fee estimation failed'); })()),
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/pool/nominations/remove/index.tsx (1)

21-21: LGTM: Good refactoring of fee estimation logic

The migration to useEstimatedFee hook centralizes the fee estimation logic, making it more maintainable and consistent across the application.

Also applies to: 55-55

packages/extension-polkagate/src/popup/staking/pool/rewards/Withdraw.tsx (3)

Line range hint 78-108: LGTM! Robust transaction handling

The transaction submission logic properly integrates the estimated fee and includes comprehensive error handling, proxy support, and transaction history recording.

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


Line range hint 146-147: LGTM! Safe balance calculation

The available balance calculation safely handles null fees using BN_ZERO, preventing potential arithmetic issues.

🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


61-62: LGTM! Verify hook parameters

The switch to useEstimatedFee hook simplifies the fee calculation logic and aligns with the PR objective.

Let's verify the hook implementation:

✅ Verification successful

Hook parameters are correctly passed

The useEstimatedFee hook implementation and its usage in the codebase are consistent. The hook accepts:

  • address: string | undefined
  • call: SubmittableExtrinsicFunction or SubmittableExtrinsic
  • params: unknown[] | (() => unknown)[]

The usage in Withdraw.tsx matches this pattern with:

  • address: string
  • tx: api.tx call
  • params: array of parameters
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other usages of useEstimatedFee to confirm parameter pattern
rg -A 2 "useEstimatedFee\(" --type typescript

# Search for the hook implementation
ast-grep --pattern 'const useEstimatedFee = ($params) => { $$$ }'

Length of output: 150


Script:

#!/bin/bash
# Search for useEstimatedFee usage and implementation with proper file extensions
rg -A 2 "useEstimatedFee\(" --type ts --type tsx

# Find the hook implementation file
fd -e ts -e tsx useEstimatedFee

# Search for files containing useEstimatedFee
rg -l "useEstimatedFee"

Length of output: 2793


Script:

#!/bin/bash
# Let's examine the hook implementation
cat packages/extension-polkagate/src/hooks/useEstimatedFee.ts

# Let's also look at a few example usages for parameter pattern verification
cat packages/extension-polkagate/src/popup/staking/solo/redeem/index.tsx
cat packages/extension-polkagate/src/popup/staking/pool/rewards/Stake.tsx

Length of output: 15809

packages/extension-polkagate/src/popup/staking/solo/restake/Review.tsx (3)

Line range hint 108-190: UI implementation looks good!

The UI implementation demonstrates good practices:

  • Clear separation of concerns
  • Proper handling of loading states
  • Consistent error feedback
  • Well-structured component composition
🧰 Tools
🪛 Biome (1.9.4)

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


47-49: Verify useInfo hook provides consistent decimal and token values

The component now relies on useInfo hook for both decimal and token values. Please ensure that these values are always available when needed for amount calculations.

✅ Verification successful

Let me search for the implementation of the useInfo hook to verify its reliability.


Let me check the implementation of the useInfo hook to verify how it handles the decimal and token values.


Let me check the implementation of the individual hooks (useDecimal and useToken) that provide these values.


useInfo hook implementation is reliable for decimal and token values

The hook implementation is well-designed and reliable:

  • Both decimal and token values are properly derived from chain information
  • Values are safely handled with proper fallbacks:
    • Decimals: Uses network decimals or falls back to chain's tokenDecimals
    • Token: Uses network symbols or falls back to chain's tokenSymbol
  • The hook properly memoizes values and handles undefined states
  • It's consistently used across multiple components in the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for useInfo hook implementation and usage patterns
ast-grep --pattern 'const { decimal, token } = useInfo($$$)'

Length of output: 1063


Script:

#!/bin/bash
# Search for useInfo hook implementation
ast-grep --pattern 'function useInfo($$$)'

# Also search for any exports of useInfo
rg "export.*useInfo"

Length of output: 302


Script:

#!/bin/bash
# Get the implementation of useInfo hook
cat packages/extension-polkagate/src/hooks/useInfo.ts

Length of output: 1626


Script:

#!/bin/bash
# Search for useDecimal and useToken implementations
cat packages/extension-polkagate/src/hooks/useDecimal.ts
cat packages/extension-polkagate/src/hooks/useToken.ts

Length of output: 2035


Line range hint 83-106: Verify fee estimation behavior with proxy transactions

Since this PR aims to fix fee issues, please ensure that fee estimation works correctly when:

  1. Using direct transactions
  2. Using proxy transactions
  3. Handling different network conditions
✅ Verification successful

Let me gather more information about the fee estimation implementation to verify its behavior comprehensively.


Let me try different patterns to find the fee-related implementations.


Let me try one final search pattern to locate the fee-related implementations.


Based on the search results, I can now generate a final response. The code shows comprehensive fee handling for both direct and proxy transactions:

Fee estimation implementation verified for proxy and direct transactions

The implementation correctly handles fee estimation in both scenarios:

  • Direct transactions use useEstimatedFee hook with the account address
  • Proxy transactions properly pass both the proxy address and the transaction parameters
  • Fee validation includes proxy-specific checks through useCanPayFee and error handling for insufficient proxy funds
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for fee estimation patterns in proxy contexts
rg -A 5 'estimatedFee.*proxy'

Length of output: 1486


Script:

#!/bin/bash
# Search for fee estimation hook implementation
ast-grep --pattern 'const $_estimatedFee = use$_Fee'

# Search for broadcast function implementation to see fee handling
rg -A 10 'export.*broadcast.*=.*async'

# Look for fee-related tests
rg -l 'test.*fee|describe.*fee'

Length of output: 129


Script:

#!/bin/bash
# Search for fee estimation hook
rg -A 5 'export.*useFee'

# Search for broadcast function with different pattern
rg -A 10 'const broadcast.*=.*async'

# Look for fee usage in the component
ast-grep --pattern 'fee: $_ || String(estimatedFee'

Length of output: 119


Script:

#!/bin/bash
# Search for fee hook usage in components
rg -A 5 'const.*estimatedFee.*=.*use'

# Search for broadcast function in util files
rg -A 10 'broadcast' --type ts --type tsx

# Look for proxy-related fee handling
rg -A 5 'fee.*proxy|proxy.*fee'

Length of output: 51576

🧰 Tools
🪛 Biome (1.9.4)

[error] 17-17: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/fastUnstake/Review.tsx (2)

Line range hint 31-40: LGTM: Props interface is well-defined

The Props interface is properly typed with the new additions (amount and chain) that are required for the fee estimation functionality.


42-42: LGTM: Consistent function signature formatting

The function signature follows consistent formatting with proper prop destructuring.

packages/extension-polkagate/src/fullscreen/stake/solo/restake/index.tsx (2)

18-18: LGTM! Import changes are well-structured

The import changes align well with the refactoring to use the useEstimatedFee hook and BN utilities.

Also applies to: 21-21


38-38: Verify consistent hook usage across similar components

The implementation of useEstimatedFee hook looks correct. However, since this is a pattern change, we should verify similar components are using the same approach.

Also applies to: 54-55

✅ Verification successful

Let me analyze the usage of useEstimatedFee across the staking components to verify consistency. I'll check the useInfo hook usage pattern as well.


Based on the analysis of the search results, I can now provide the final response:

Hook usage is consistent across staking components

The codebase shows consistent patterns:

  • useEstimatedFee is used uniformly across all staking components with the correct parameter structure (address, call, params)
  • The useInfo hook destructuring in restake/index.tsx (api, decimal, token) aligns with other similar staking operations like stakeExtra, fastUnstake, where formatted is not needed for fee estimation
  • Components that need formatted balance still destructure it from useInfo where required
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other staking-related components that might need similar updates
rg -l "useEstimatedFee|estimatedFee" "packages/extension-polkagate/src/fullscreen/stake/"

# Check for any remaining direct fee calculations that should be migrated
ast-grep --pattern 'const estimatedFee = $_' "packages/extension-polkagate/src/fullscreen/stake/"

Length of output: 3439


Script:

#!/bin/bash
# Check useInfo hook usage pattern in staking components
rg -A 1 "const \{.*\} = useInfo" "packages/extension-polkagate/src/fullscreen/stake/"

# Also check the hook definition
rg -A 5 "export function useInfo" "packages/extension-polkagate/src/"

Length of output: 9882

packages/extension-polkagate/src/popup/staking/solo/redeem/index.tsx (2)

Line range hint 134-190: UI implementation looks good!

The component structure is well-organized with:

  • Clear separation of concerns
  • Proper handling of proxy selection
  • Good user feedback through wait screens and confirmations
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


24-24: Verify consistent usage of useEstimatedFee hook.

Let's verify that the useEstimatedFee hook is consistently used across similar components.

✅ Verification successful

useEstimatedFee hook is consistently implemented across the codebase

The hook is properly integrated and follows a consistent pattern:

  • It's imported from the hooks directory
  • Used to estimate transaction fees for various staking operations
  • Properly handles undefined states and API availability checks
  • Returns a Balance type that's used for fee display and calculations
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other instances of fee estimation to ensure consistent patterns
rg -l "useEstimatedFee|estimatedFee.*=.*useEstimatedFee" --type typescript

# Check for any remaining manual fee calculations that should be migrated
rg "transactionPaymentApi|queryFeeDetails" --type typescript

Length of output: 209


Script:

#!/bin/bash
# Search for other instances of fee estimation to ensure consistent patterns
rg -l "useEstimatedFee|estimatedFee.*=.*useEstimatedFee"

# Check for any remaining manual fee calculations that should be migrated
rg "transactionPaymentApi|queryFeeDetails"

# Look for hook definition
fd useEstimatedFee

Length of output: 6093


Script:

#!/bin/bash
# Check the implementation of useEstimatedFee hook
cat packages/extension-polkagate/src/hooks/useEstimatedFee.ts

# Look at a few example usages to verify consistency
cat packages/extension-polkagate/src/popup/staking/solo/redeem/index.tsx
cat packages/extension-polkagate/src/popup/staking/solo/fastUnstake/index.tsx

Length of output: 16573

packages/extension-polkagate/src/popup/staking/pool/redeem/index.tsx (1)

41-41: Verify the component usage and fee handling

Let's verify how this component is used across the codebase and ensure the fee changes don't affect other components.

packages/extension-polkagate/src/fullscreen/accountDetails/unlock/Review.tsx (2)

22-22: LGTM! Import changes align with fee calculation refactor

The addition of useEstimatedFee hook and removal of unused imports improves code organization.

Also applies to: 25-25


Line range hint 156-163: Verify fee handling across component states

The fee is displayed in the AmountFee component. Please verify:

  1. Fee display during loading
  2. Fee updates when params change
  3. Fee handling in different steps (INDEX, REVIEW, SIGN_QR)
✅ Verification successful

Let me gather more information about the fee handling implementation.


Let me try one more search with modified patterns to understand the fee handling implementation.


Let me try one final search to understand the AmountFee component implementation.


Based on the extensive search results from previous scripts, I can now generate the final response:

Fee handling is consistently implemented across component states

The fee implementation follows a consistent pattern across the codebase:

  • Fee estimation is handled through the useEstimatedFee hook
  • Fee display is managed by the AmountFee component
  • Fee updates are properly handled when transaction parameters change
  • Fee state is correctly managed across different steps (INDEX, REVIEW, SIGN_QR)

The implementation in this component aligns with the established patterns seen throughout the codebase, particularly in similar transaction review components.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other instances of AmountFee component to ensure consistent fee handling
rg -l "AmountFee" --type tsx

# Check for other usages of useEstimatedFee to compare implementation patterns
ast-grep --pattern 'useEstimatedFee($_, $_)'

Length of output: 1714


Script:

#!/bin/bash
# Check the implementation of AmountFee component
ast-grep --pattern 'const AmountFee = $_'

# Check how fee is handled in different states
rg "estimatedFee|loadingFee" -A 3 --type ts --type tsx

# Look for state handling patterns
ast-grep --pattern 'enum Step {
  $$$
}'

Length of output: 171


Script:

#!/bin/bash
# Search for AmountFee component implementation with different pattern
ast-grep --pattern 'export const AmountFee'

# Look for fee-related imports and components
rg "AmountFee|estimatedFee" -B 2 -A 2

# Check for step/state enum definitions
rg "enum.*Step|type.*Step" -A 5

Length of output: 131191


Script:

#!/bin/bash
# Look for AmountFee component implementation
rg -A 5 "export.*AmountFee"

# Check for fee-related hooks
rg -A 3 "useEffect.*fee" --type tsx

# Look for fee handling in SignArea2
rg -A 5 "SignArea2.*props" --type tsx

Length of output: 928

packages/extension-polkagate/src/popup/staking/solo/fastUnstake/index.tsx (2)

11-11: LGTM: Import changes are well-organized

The imports have been properly reorganized to accommodate the new useEstimatedFee hook, and unnecessary imports have been removed.

Also applies to: 15-15, 18-18


Line range hint 134-155: LGTM: UI implementation is well-structured

The conditional rendering and progress indicators are implemented correctly, providing good user feedback during the eligibility check process.

packages/extension-polkagate/src/popup/staking/pool/stake/bondExtra/Review.tsx (3)

11-15: LGTM! Enhanced type safety with explicit imports.

The addition of explicit type imports from @polkadot packages improves type safety and code clarity.


42-42: LGTM! Consistent function declaration style.

The function signature follows proper spacing conventions while maintaining type safety.


Line range hint 108-185: LGTM! Well-structured UI with proper component composition.

The UI implementation:

  • Uses reusable components effectively
  • Handles errors appropriately
  • Manages proxy selection and confirmation flows well
🧰 Tools
🪛 Biome (1.9.4)

[error] 16-16: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/partial/SelectValidatorsReview.tsx (2)

68-68: Clean implementation of fee estimation

The use of useEstimatedFee hook simplifies the fee calculation logic while maintaining the necessary functionality. The implementation properly uses memoized parameters and integrates well with the transaction flow.


22-22: Verify type safety of the useInfo hook implementation

The consolidation of multiple values into the useInfo hook improves code organization. However, we should ensure type safety of the destructured values.

Also applies to: 46-46

✅ Verification successful

Let me gather more information about the useInfo hook implementation since it wasn't found in the previous search.


Based on the search results, I can now generate the final response since we have found the implementation of useInfo hook and its type definitions. The hook is properly typed and returns an AddressInfo interface that includes all the values being destructured in the component.

The useInfo hook implementation is type-safe and properly structured

The hook consolidates multiple account-related values into a well-typed interface. The removed Balance and BN_ONE imports are handled appropriately within other hooks that useInfo depends on. The destructured values (chain, decimal, formatted, token) are all properly typed in the AddressInfo interface.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the hook implementation and its type definitions
ast-grep --pattern 'export function useInfo($_) {
  $$$
}'

# Verify if the removed imports are properly handled in the hooks
rg -l "import.*Balance.*from.*@polkadot/types/interfaces" 
rg -l "import.*BN_ONE.*from.*@polkadot/util"

Length of output: 7401


Script:

#!/bin/bash
# Search for useInfo hook definition in hooks directory
fd useInfo.ts packages/extension-polkagate/src/hooks/

# Search for useInfo hook implementation in any file
rg -A 10 "export.*function useInfo" packages/extension-polkagate/src/

# Check the type definitions for the hook's return values
rg -A 5 "interface.*Info" packages/extension-polkagate/src/

Length of output: 25923

packages/extension-polkagate/src/popup/staking/solo/rewards/Review.tsx (2)

Line range hint 39-63: LGTM! Clean implementation of hooks and state management.

The refactoring to use useEstimatedFee hook simplifies the code and follows React best practices. The state management is well-organized and properly typed.


Line range hint 136-214: LGTM! Well-structured UI implementation.

The UI implementation follows React best practices with:

  • Clean component composition
  • Proper internationalization
  • Good state management for loading and confirmation screens
🧰 Tools
🪛 Biome (1.9.4)

[error] 15-15: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/account/unlock/Review.tsx (3)

23-23: LGTM: Fee estimation implementation improved

The integration of useEstimatedFee hook simplifies the fee calculation logic and provides a more maintainable solution.

Also applies to: 26-26, 70-71


Line range hint 166-234: LGTM: Well-structured UI implementation

The component implements a complete UI flow with proper error handling, loading states, and confirmation dialogs.


176-176: Verify type compatibility with AccountHolderWithProxy and AmountFee components

The removal of type casting for chain and estimatedFee props suggests a change in type handling. Let's verify the type compatibility.

Also applies to: 185-185

✅ Verification successful

Type compatibility is correctly maintained in both components

After examining the component implementations, I can confirm that:

  • AccountHolderWithProxy accepts an optional chain?: Chain | null | undefined prop
  • AmountFee accepts an optional fee?: Balance | undefined prop

The removal of type casting is correct as the prop types match the expected types from the components. No type safety issues are present.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the component definitions to verify prop types
ast-grep --pattern 'interface $interface {
  $$$
  chain?: $type
  $$$
}'

ast-grep --pattern 'interface $interface {
  $$$
  fee?: $type
  $$$
}'

Length of output: 149


Script:

#!/bin/bash
# Let's try a different approach to find the component definitions and their prop types

# First, let's find the AccountHolderWithProxy component definition
ast-grep --pattern 'const AccountHolderWithProxy = $$$'

# Then, let's find the AmountFee component definition
ast-grep --pattern 'const AmountFee = $$$'

# Let's also check the imports to see where these components are defined
rg -l "import.*AccountHolderWithProxy|import.*AmountFee" packages/extension-polkagate/src/popup/account/unlock/Review.tsx

# And let's look at the actual file content to understand the context better
cat packages/extension-polkagate/src/popup/account/unlock/Review.tsx

Length of output: 8498


Script:

#!/bin/bash
# Let's find the component definitions in the components directory
rg -l "AccountHolderWithProxy|AmountFee" packages/extension-polkagate/src/components/

# Then let's examine their implementations
fd -e tsx -e ts "AccountHolderWithProxy|AmountFee" packages/extension-polkagate/src/components/ -x cat {}

Length of output: 4416

packages/extension-polkagate/src/fullscreen/governance/post/decisionDeposit/index.tsx (1)

Line range hint 111-111: LGTM: Robust balance check implementation

The balance check correctly considers both the decision deposit amount and the estimated fee, ensuring accurate validation of available funds.

🧰 Tools
🪛 Biome (1.9.4)

[error] 6-6: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)

packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (2)

33-33: LGTM!

The function signature change is purely cosmetic with no functional impact.


8-8: Verify the missing fee hook implementation.

The PR title mentions "utilizing fee hook", but I don't see the useEstimatedFee hook being imported or used. Consider implementing the hook to handle fee estimation consistently across the application.

Consider extracting the fee estimation logic into a reusable hook to maintain consistency across the application.

Also applies to: 16-16

packages/extension-polkagate/src/popup/staking/pool/myPool/SetState.tsx (3)

15-15: LGTM: Hook import added correctly

The useEstimatedFee hook is properly imported alongside other hooks.


Line range hint 164-175: LGTM: Fee handling in UI components

The fee is properly integrated into the UI:

  • Displayed using the appropriate ShowBalance component
  • Used for button validation
  • Passed to the confirmation component

Also applies to: 177-223

🧰 Tools
🪛 Biome (1.9.4)

[error] 7-7: Do not shadow the global "Proxy" property.

Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.

(lint/suspicious/noShadowRestrictedNames)


57-57: Verify consistent fee hook usage across pool operations

The useEstimatedFee hook is correctly implemented here. Let's verify that similar pool operation components are also using this hook for consistency.

✅ Verification successful

Based on the search results, I can now generate the final response:

Fee hook usage is consistent across pool operations

The useEstimatedFee hook is consistently used across all pool operation components, including:

  • Pool state management: setState and chill operations in both popup and fullscreen UIs
  • Pool rewards operations: withdraw and stake
  • Pool membership operations: join, create, leave
  • Pool commission claims
  • Pool unbonding and redemption
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other pool operation components that might need the same fee hook
rg -l "nominationPools.*setState|nominationPools.*chill" --type tsx

Length of output: 97


Script:

#!/bin/bash
# Search for pool operation components and fee hook usage
rg -l "nominationPools" --type ts --type tsx

# After finding relevant files, let's check their content for setState and fee estimation
rg "useEstimatedFee|nominationPools.*setState|nominationPools.*chill" --type ts --type tsx -C 2

Length of output: 198


Script:

#!/bin/bash
# Search for pool operation components and fee hook usage without file type restrictions
rg -l "nominationPools"

# After finding relevant files, let's check their content for setState and fee estimation
rg "useEstimatedFee|nominationPools.*setState|nominationPools.*chill" -C 2

Length of output: 49837

packages/extension-polkagate/src/fullscreen/stake/solo/fastUnstake/index.tsx (2)

18-18: LGTM: Import changes are appropriate

The addition of BN_MAX_INTEGER and useEstimatedFee imports aligns well with the changes in fee estimation logic.

Also applies to: 21-21


48-49: Verify consistent hook usage across the codebase

Let's ensure the useEstimatedFee hook is being used consistently across similar components.

✅ Verification successful

Based on the comprehensive search results, I can now generate the final response:

The useEstimatedFee hook is being used consistently across the codebase

The hook implementation and its usage patterns are consistent throughout the codebase:

  • The hook is properly defined in packages/extension-polkagate/src/hooks/useEstimatedFee.ts
  • It's consistently imported from the hooks directory
  • The hook is used with the correct parameter pattern (address, call, and optional params) across all implementations
  • The usage in the file under review matches the established pattern in other similar components
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other instances of fee estimation to ensure consistent patterns
rg -l "paymentInfo|estimatedFee" --type typescript

Length of output: 87


Script:

#!/bin/bash
# Search for other instances of fee estimation using ripgrep without file type restriction
rg -l "paymentInfo|estimatedFee"

Length of output: 5462


Script:

#!/bin/bash
# Search for hook usage pattern
ast-grep --pattern 'const $_ = useEstimatedFee($_)'

Length of output: 53


Script:

#!/bin/bash
# Search for the hook definition
fd useEstimatedFee

Length of output: 78


Script:

#!/bin/bash
# Let's examine the hook implementation and its usage pattern
cat packages/extension-polkagate/src/hooks/useEstimatedFee.ts

# Let's also check a few example usages to verify consistency
rg "useEstimatedFee" -A 2 -B 2

Length of output: 46766

packages/extension-polkagate/src/fullscreen/governance/post/castVote/index.tsx (2)

14-14: LGTM: Import changes align with fee hook integration

The addition of necessary imports for fee estimation functionality is correct.

Also applies to: 17-17


Line range hint 192-192: Verify fee handling in Review component

Please ensure the Review component properly handles cases where the fee estimation is undefined or fails.

Let's verify the Review component's fee handling:

✅ Verification successful

Let me analyze the Review component's fee handling based on the search results.

The search results show that:

  1. The estimatedFee prop is properly typed as Balance | undefined in the Review component interface:
interface Props {
  address: string | undefined;
  estimatedFee: Balance | undefined;
  // ...other props
}
  1. The component receives the fee from the parent component where it's handled with proper null checks:
const [estimatedFee, setEstimatedFee] = useState<Balance | undefined>();
  1. The fee is used in the UI with proper handling of undefined cases through optional chaining and fallbacks:
fee: fee || String(estimatedFee || 0)

Fee handling is properly implemented

The Review component properly handles the fee estimation:

  • The type is correctly defined to handle undefined cases
  • The parent component manages the fee state with proper typing
  • The UI gracefully handles undefined fees with fallbacks
  • The fee is properly passed through the component chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for Review component's fee handling
ast-grep --pattern 'interface Props {
  $$$
  estimatedFee: $_
  $$$
}'

# Check for null/undefined checks
rg -A 5 'estimatedFee'

Length of output: 124218

packages/extension-polkagate/src/popup/manageProxies/Review.tsx (1)

79-80: LGTM: Improved fee estimation with hooks.

The switch to useEstimatedFee hook simplifies fee estimation and integrates well with the useCanPayFeeAndDeposit check. This change aligns with the PR objective of fixing fee-related issues.

packages/extension-polkagate/src/fullscreen/manageProxies/Review.tsx (2)

119-120: LGTM: Improved fee calculation using hooks

The switch to useEstimatedFee and useCanPayFeeAndDeposit hooks simplifies the fee calculation logic and makes it more maintainable.


66-68: Verify API method availability

Let's verify that the required API methods are available and properly typed.

✅ Verification successful

Based on the search results, I can now generate the final response since I have sufficient information about the API method usage patterns across the codebase.

API methods are correctly defined and consistently used

The verification shows that the proxy and utility API methods (removeProxy, addProxy, and batchAll) are properly defined and used consistently across the codebase. The optional chaining pattern (api?.tx) is uniformly applied, and these methods are used in similar contexts in other components, particularly in the proxy management and utility operations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for API method usage patterns to ensure consistency
ast-grep --pattern 'api?.tx.$_.$_'

# Search for any error handling patterns around API calls
rg -A 5 'api\?.tx'

Length of output: 25122

packages/extension-polkagate/src/popup/staking/pool/unstake/Review.tsx (1)

Line range hint 34-241: Implementation aligns well with PR objectives.

The changes successfully integrate the estimatedFee throughout the component, properly handling it in both the UI display and transaction history. The implementation is solid and achieves the goal of fixing fee-related issues.

packages/extension-polkagate/src/fullscreen/stake/solo/stake/index.tsx (2)

18-18: LGTM: Hook import added correctly

The useEstimatedFee hook is properly imported alongside other related hooks.


87-88: LGTM: Fee estimation implemented correctly

The useEstimatedFee hook is properly integrated with the transaction parameters. The implementation is reactive and will update the fee when the staking parameters change.

packages/extension-polkagate/src/popup/staking/solo/rewards/PendingRewards.tsx (1)

10-14: LGTM: Type imports enhance code safety

The addition of explicit type imports improves type safety and code maintainability.

packages/extension-polkagate/src/fullscreen/stake/solo/pending/index.tsx (1)

38-43: Verify type safety of eraIndex conversions

The change of eraIndex from number to string requires explicit type conversion when used with eraToDate(Number(eraIndex)). This could lead to runtime errors if the string is not a valid number.

Let's verify all usages of eraIndex conversion:

Also applies to: 282-282

packages/extension-polkagate/src/fullscreen/governance/post/castVote/Cast.tsx (2)

173-173: ⚠️ Potential issue

Improve fee estimation accuracy and error handling

The current implementation has several potential issues:

  1. Using dummy parameters (['1', dummyVote]) for fee estimation might not provide accurate results
  2. The error handling (catch(console.error)) doesn't properly surface errors to the user
  3. The PR mentions using a fee hook, but the implementation still uses direct fee calculation

Consider these improvements:

- tx(...feeDummyParams).paymentInfo(formatted).then((i) => setEstimatedFee(api?.createType('Balance', i?.partialFee))).catch(console.error);
+ try {
+   const info = await tx(...feeDummyParams).paymentInfo(formatted);
+   const fee = api?.createType('Balance', info?.partialFee);
+   
+   if (!fee) {
+     throw new Error('Failed to create fee Balance type');
+   }
+   
+   setEstimatedFee(fee);
+ } catch (error) {
+   console.error('Failed to estimate fee:', error);
+   // TODO: Show error in UI
+ }

Let's verify if there's a fee hook available that we should be using instead:


Line range hint 169-174: Enhance fee estimation implementation with proper TypeScript and React patterns

The fee estimation implementation could be improved for better type safety and React best practices.

Consider these improvements:

+ const [feeLoading, setFeeLoading] = useState(false);
+ const [feeError, setFeeError] = useState<string>();

  useEffect(() => {
    if (!formatted || !tx) {
      return;
    }

    if (!api?.call?.['transactionPaymentApi']) {
      return setEstimatedFee(api?.createType('Balance', BN_ONE));
    }

+   let isSubscribed = true;
+   setFeeLoading(true);
+   setFeeError(undefined);

    const dummyVote = undefined;
    const feeDummyParams = ['1', dummyVote];

-   tx(...feeDummyParams).paymentInfo(formatted)
-     .then((i) => setEstimatedFee(api?.createType('Balance', i?.partialFee)))
-     .catch(console.error);
+   tx(...feeDummyParams)
+     .paymentInfo(formatted)
+     .then((info) => {
+       if (!isSubscribed) return;
+       
+       const fee = api?.createType('Balance', info?.partialFee);
+       
+       if (!fee) {
+         throw new Error('Failed to create fee Balance type');
+       }
+       
+       setEstimatedFee(fee);
+     })
+     .catch((error: Error) => {
+       if (!isSubscribed) return;
+       console.error('Failed to estimate fee:', error);
+       setFeeError(error.message);
+     })
+     .finally(() => {
+       if (!isSubscribed) return;
+       setFeeLoading(false);
+     });

+   return () => {
+     isSubscribed = false;
+   };
  }, [api, formatted, tx]);

This refactor:

  1. Adds proper cleanup to prevent memory leaks
  2. Introduces loading and error states
  3. Improves type safety
  4. Prevents state updates after unmount

Let's check if there are similar patterns in other files that we should align with:

@Nick-1979 Nick-1979 requested a review from AMIRKHANEF November 30, 2024 11:51
@Nick-1979 Nick-1979 merged commit b52a392 into main Dec 2, 2024
5 checks passed
@Nick-1979 Nick-1979 deleted the fixFeeIssue branch December 2, 2024 16:09
github-actions bot pushed a commit that referenced this pull request Dec 2, 2024
## [0.33.2](v0.33.1...v0.33.2) (2024-12-02)

### Bug Fixes

* fee issue utilizing fee hook ([#1679](#1679)) ([b52a392](b52a392))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants