feat(suite): display rent in fees summary #17244
Conversation
|
🚀 Expo preview is ready!
|
TomasBoda
changed the title
TomasBoda
requested review from
MiroslavProchazka,
mroz22,
szymonlesisz and
marekrjpolak
as code owners
WalkthroughThe changes remove the boolean flag indicating account creation and replace it with a new property, 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (11)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🔇 Additional comments (9)
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/blockchain-link-types/src/params.ts (1)
34-35: Update comment to match the new property nameThe comment on line 35 still references
isCreatingAccountwhich has been renamed toisStaking.- newTokenAccountProgramName?: 'spl-token' | 'spl-token-2022'; // program name of the Solana Token account that is being created, ignored if isCreatingAccount is false, default: 'spl-token' + newTokenAccountProgramName?: 'spl-token' | 'spl-token-2022'; // program name of the Solana Token account that is being created, ignored if isStaking is false, default: 'spl-token'packages/connect/src/api/solana/api/solanaComposeTransaction.ts (1)
99-103: Remove unusedisCreatingAccountvariable.The
isCreatingAccountvariable is defined but no longer used in the return value, creating dead code. Consider removing it if it's not needed for determiningnewTokenAccountProgramName.-const isCreatingAccount = - this.params.token && - recipientTokenAccounts === undefined && - // if the recipient account has no owner, it means it's a new account and needs the token account to be created - (recipientAccountOwner === SYSTEM_PROGRAM_PUBLIC_KEY || recipientAccountOwner == null); const newTokenAccountProgramName = isCreatingAccount ? this.params.token?.program : undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/blockchain-link-types/src/params.ts(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(3 hunks)packages/connect-explorer/src/pages/methods/solana/solanaComposeTransaction.mdx(1 hunks)packages/connect/e2e/__fixtures__/solanaComposeTransaction.ts(1 hunks)packages/connect/src/api/blockchainEstimateFee.ts(1 hunks)packages/connect/src/api/solana/api/solanaComposeTransaction.ts(1 hunks)packages/connect/src/types/api/solana/index.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(1 hunks)packages/suite/src/components/suite/modals/ReduxModal/TransactionReviewModal/TransactionReviewOutputList/TransactionReviewTotalOutput.tsx(1 hunks)packages/suite/src/hooks/wallet/__fixtures__/useSendForm.ts(1 hunks)packages/suite/src/support/messages.ts(1 hunks)suite-common/wallet-core/src/send/sendFormSolanaThunks.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: build-web
- GitHub Check: build-web
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: EAS Update
- GitHub Check: test
🔇 Additional comments (14)
packages/suite/src/components/suite/modals/ReduxModal/TransactionReviewModal/TransactionReviewOutputList/TransactionReviewTotalOutput.tsx (1)
38-38: Improved fee label clarity for Solana transactionsThe fee label has been updated to explicitly include rent in the description for Solana transactions, which aligns with the PR objective to display rent fees in the fees summary.
packages/connect/src/types/api/solana/index.ts (1)
76-77: Property renamed to reflect the staking contextThe property was renamed from
isCreatingAccounttoisStakingto better represent the context of the operation, which aligns with the PR objective to implement rent fee calculations for Solana staking transactions.packages/suite/src/support/messages.ts (1)
3243-3246: Added message entry for Solana fee with rent looks good.The addition of
TR_TX_FEE_INCLUDING_RENTwith the message "Fee (incl. rent)" effectively implements the requested feature to display rent in fees summary for Solana transactions. This provides users with clearer information about the fee structure.suite-common/wallet-core/src/send/sendFormSolanaThunks.ts (1)
186-186: Properly updated transaction context from account creation to staking.The change from
isCreatingAccounttoisStakingproperly aligns with the PR objective to improve rent fee calculation in Solana transactions.packages/suite/src/hooks/wallet/__fixtures__/useSendForm.ts (1)
1278-1278: Test fixture correctly updated to match implementation change.The
isStakingproperty replacesisCreatingAccountin the test fixture, maintaining consistency with the implementation changes.packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
140-140: Value changed from false to true, correctly identifying staking transactions.Changed property from
isCreatingAccount: falsetoisStaking: true, which correctly identifies these transactions as staking operations, aligning with the file's purpose of handling staking actions.packages/connect/e2e/__fixtures__/solanaComposeTransaction.ts (1)
85-85: Test fixture updated to use new staking property.The property
isCreatingAccounthas been replaced withisStakingin the test fixture, maintaining its value asfalsewhich is appropriate for this specific token transfer test case.packages/connect-explorer/src/pages/methods/solana/solanaComposeTransaction.mdx (1)
73-73: Parameter name updated to reflect staking operation.The change from
isCreatingAccounttoisStakingin the response payload structure aligns with the PR objective to display rent fees for Solana staking transactions.packages/connect/src/api/blockchainEstimateFee.ts (1)
48-48: Parameter validation updated for staking context.The validation schema has been updated to match the renamed parameter, ensuring consistent use of
isStakingthroughout the codebase.packages/connect/src/api/solana/api/solanaComposeTransaction.ts (1)
111-111:❓ Verification inconclusive
Value hardcoded instead of using conditional logic.
The property
isStakingis now hardcoded tofalseinstead of using conditional logic to determine its value. This seems to be returning a default value for a non-staking transaction context.
🏁 Script executed:
#!/bin/bash # Search for usages of this field to understand if hardcoding is appropriate rg -A 5 "isStaking.*additionalInfo" --type tsLength of output: 653
Consistent Dynamic Handling of
isStakingRequiredCurrently, in
packages/connect/src/api/solana/api/solanaComposeTransaction.tstheisStakingproperty is hardcoded tofalse, whereas insuite-common/wallet-core/src/send/sendFormSolanaThunks.tsit’s dynamically derived fromtransaction.payload.additionalInfo.isStaking. Please verify whether the conditional logic for determining a staking transaction should also apply here. If staking transactions are expected in this context, update the logic to pull the correct value; otherwise, add documentation explaining why a hardcodedfalseis appropriate.packages/blockchain-link/src/workers/solana/index.ts (4)
1-1: Added import for stake account size constant.The imported constant will be used for calculating the minimum balance required for rent exemption for staking accounts.
456-457: Parameter renamed for clarity.Renaming from
isCreatingAccounttoisStakingprovides better semantic clarity about the transaction's purpose.
469-486: Improved code structure with dedicated helper functions.The refactoring introduces clear separation between staking and token account fee calculations, improving code maintainability and making the functionality more explicit.
The implementation correctly uses
getMinimumBalanceForRentExemptionwith the appropriate account size for each type of account. For staking, it uses the importedSTAKE_ACCOUNT_V2_SIZEconstant.
483-485: Logic updated to distinguish between staking and token account creation.The code now uses
isStakingto determine which type of account creation fee to calculate, providing more accurate fee estimates based on the transaction type.
tomasklim
added
the
release
TomasBoda
force-pushed
the
feat/solana-rent-fee
branch
from
6020d7e to
3685deb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
packages/product-components/src/components/FeeRate/FeeRate.tsx (1)
14-31: Confirm rounding behavior for Ethereum fees.
UsingMath.ceilensures the fee is always rounded up, potentially leading to higher calculated fees than expected. If the intention is to avoid underestimating fees, this is fine, but confirm that this aligns with UX and product requirements. Otherwise, considerMath.roundorMath.floor, or incorporate user feedback to decide.packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeMisc.tsx (1)
26-28: Consider using proper typing instead of type assertionsThe component uses type assertions to work around typing limitations, but this approach can be error-prone. Consider refactoring to use generic types more effectively or restructuring the component to avoid the need for these assertions.
- // Type assertion allowing to make the component reusable, see https://stackoverflow.com/a/73624072. - const { getValues } = props as unknown as UseFormReturn<FormState>; - const errors = props.errors as unknown as FieldErrors<FormState>; + // Extract typed methods from props using generics + const { getValues } = props as UseFormReturn<TFieldValues & FormState>; + const errors = props.errors as FieldErrors<TFieldValues & FormState>;packages/suite/src/components/wallet/Fees/StandardFee/BitcoinFeeCards.tsx (1)
28-53: Consider using unique identifiers instead of array indices as React keys.Using array indices as keys (line 30) can lead to performance issues and unexpected behavior when the list items change order or are filtered. If each fee option has a unique identifier like
idorvalue, consider using that as the key instead.- <FeeCard - key={index} + <FeeCard + key={fee.value}packages/suite/src/components/wallet/Fees/StandardFee/EthereumFeeCards.tsx (1)
20-40: Consider using unique identifiers instead of array indices as React keys.Using array indices as keys (line 22) can lead to performance issues and unexpected behavior when the list items change order or are filtered. If each fee option has a unique identifier like
idorvalue, consider using that as the key instead.- <FeeCard - key={index} + <FeeCard + key={fee.value}packages/suite/src/components/wallet/Fees/StandardFee/FeeCard.tsx (1)
27-27: Consider memoizing the onClick handler.The current implementation creates a new function on each render. Consider using useCallback to memoize this function to optimize performance, especially if this component is rendered frequently.
<RadioCard - onClick={() => changeFeeLevel(value)} + onClick={React.useCallback(() => changeFeeLevel(value), [changeFeeLevel, value])} isActive={isSelected} >packages/suite/src/components/wallet/Fees/CustomFee/CustomFee.tsx (1)
77-83: Consider using median fee for getCurrentFee.The implementation selects the middle fee level as the current fee reference. While this works, consider whether using the median or a more statistically relevant measure might better represent the "typical" fee.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
packages/blockchain-link-types/src/params.ts(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(3 hunks)packages/connect-explorer/src/pages/methods/solana/solanaComposeTransaction.mdx(1 hunks)packages/connect/e2e/__fixtures__/solanaComposeTransaction.ts(1 hunks)packages/connect/src/api/blockchainEstimateFee.ts(1 hunks)packages/connect/src/api/solana/api/solanaComposeTransaction.ts(1 hunks)packages/connect/src/types/api/solana/index.ts(1 hunks)packages/product-components/src/components/FeeRate/FeeRate.tsx(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(1 hunks)packages/suite/src/components/suite/modals/ReduxModal/TransactionReviewModal/TransactionReviewOutputList/TransactionReviewTotalOutput.tsx(1 hunks)packages/suite/src/components/suite/modals/ReduxModal/TransactionReviewModal/TransactionReviewSummary.tsx(2 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/BasicTxDetails.tsx(2 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/CancelTransaction/CancelTransaction.tsx(1 hunks)packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ChangeFee.tsx(1 hunks)packages/suite/src/components/wallet/Fees/CustomFee.tsx(0 hunks)packages/suite/src/components/wallet/Fees/CustomFee/CurrentFee.tsx(1 hunks)packages/suite/src/components/wallet/Fees/CustomFee/CustomFee.tsx(1 hunks)packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeEthereum.tsx(1 hunks)packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeMisc.tsx(1 hunks)packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeWrapper.tsx(1 hunks)packages/suite/src/components/wallet/Fees/FeeDetails.tsx(0 hunks)packages/suite/src/components/wallet/Fees/Fees.tsx(7 hunks)packages/suite/src/components/wallet/Fees/StandardFee/BitcoinFeeCards.tsx(1 hunks)packages/suite/src/components/wallet/Fees/StandardFee/EthereumFeeCards.tsx(1 hunks)packages/suite/src/components/wallet/Fees/StandardFee/FeeCard.tsx(1 hunks)packages/suite/src/components/wallet/Fees/StandardFee/MiscFeeCards.tsx(1 hunks)packages/suite/src/components/wallet/Fees/StandardFee/StandardFee.tsx(1 hunks)packages/suite/src/components/wallet/Fees/index.tsx(1 hunks)packages/suite/src/hooks/wallet/__fixtures__/useSendForm.ts(1 hunks)packages/suite/src/support/messages.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/suite/src/components/wallet/Fees/CustomFee.tsx
- packages/suite/src/components/wallet/Fees/FeeDetails.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/suite/src/components/wallet/Fees/index.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/connect/src/types/api/solana/index.ts
- packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
- packages/suite/src/components/suite/modals/ReduxModal/TransactionReviewModal/TransactionReviewOutputList/TransactionReviewTotalOutput.tsx
- packages/blockchain-link-types/src/params.ts
- packages/suite/src/hooks/wallet/fixtures/useSendForm.ts
- packages/connect/src/api/blockchainEstimateFee.ts
- packages/connect/e2e/fixtures/solanaComposeTransaction.ts
- packages/connect-explorer/src/pages/methods/solana/solanaComposeTransaction.mdx
- packages/connect/src/api/solana/api/solanaComposeTransaction.ts
- packages/suite/src/support/messages.ts
🧰 Additional context used
🧠 Learnings (1)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/CancelTransaction/CancelTransaction.tsx (1)
Learnt from: Lemonexe
PR: trezor/trezor-suite#16525
File: packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/CancelTransaction/CancelTransactionModal.tsx:60-76
Timestamp: 2025-02-03T17:19:25.374Z
Learning: The CancelTransactionModal component should handle error states from composeCancelTransactionThunk by showing CancelTransactionFailed component and disable the cancel button during loading.
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-deploy
- GitHub Check: EAS Update
- GitHub Check: test
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: prepare_android_test_app
- GitHub Check: build-web
🔇 Additional comments (37)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ChangeFee.tsx (1)
33-33: Good addition of the symbol prop to FeeRate component.The addition of the
symbolprop to the FeeRate component aligns with the PR objective to improve fee display, particularly for supporting rent fees in Solana transactions. This is a necessary change to ensure the fee rate component has the correct context for rendering fees.packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/CancelTransaction/CancelTransaction.tsx (1)
71-75: Consistent update for the FeeRate component.The addition of the
symbolprop to the FeeRate component here maintains consistency with other instances in the codebase. This change is necessary to support the improved fee calculation and display, particularly for Solana transactions with rent fees.packages/suite/src/components/suite/modals/ReduxModal/TransactionReviewModal/TransactionReviewSummary.tsx (2)
50-50: Simplified variable access.Good refactoring to use the already destructured
symbolvariable directly instead of accessing it throughaccount.symbol. This improves code clarity and consistency.
89-89: Consistent implementation of the symbol prop.Both instances of FeeRate have been updated with the symbol prop, ensuring consistent behavior throughout the component. This maintains feature parity across different network types and is necessary for the proper display of Solana rent fees.
Also applies to: 93-93
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/BasicTxDetails.tsx (1)
154-154: Properly updated all FeeRate instances.Good job updating both Bitcoin and Ethereum FeeRate components with the symbol prop. This ensures consistent behavior across different network types and supports the PR objective to improve fee displays, particularly for Solana transactions.
Also applies to: 178-178
packages/product-components/src/components/FeeRate/FeeRate.tsx (4)
1-1: Well-handled import.
The explicit import ofhasNetworkSettlementLayerensures more robust fee logic by enabling network-specific conditional checks. No issues here.
8-8: Good addition of thesymbolprop.
Providing thesymbolprop allows for network-specific fee handling and improves clarity and maintainability by consolidating network metadata in one place.
11-13: Conditional render check is concise.
ReturningnullwhenfeeRateis not provided is a succinct approach. Make sure that upstream components handle thisnullstate properly.
35-35: Proper display of fee value and units.
AppendinggetFeeUnits(networkType)to the fee nicely differentiates networks. The approach is clear and consistent.packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeWrapper.tsx (1)
1-29: Well-structured component with clear responsibilityThis component creates a consistent warning UI wrapper for custom fee components. The implementation is clean, properly typed, and follows good React patterns.
packages/suite/src/components/wallet/Fees/CustomFee/CurrentFee.tsx (1)
1-29: Clean implementation of fee display componentThis component nicely handles displaying the current fee with appropriate formatting and an icon. The structure using Row layout creates good alignment between the label and value.
packages/blockchain-link/src/workers/solana/index.ts (4)
1-1: Good addition of required Solana constantThe import of
STAKE_ACCOUNT_V2_SIZEfrom the Solana wallet SDK is appropriate for calculating staking-related rent exemption.
470-471: Improved semantic naming from account creation to stakingChanging
isCreatingAccounttoisStakingbetter reflects the purpose of the flag in the context of the PR objective to implement rent fee calculation for Solana staking.
483-496: Well-structured helper functions for fee calculationThese helper functions cleanly separate the logic for calculating different types of rent exemption fees, making the code more maintainable and easier to understand.
497-499: Clear conditional logic for fee calculationThe code now properly determines which fee calculation to use based on whether the transaction is for staking or token account creation, aligning with the PR objective to display rent in fees summary.
packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeMisc.tsx (1)
14-96: Comprehensive custom fee component with proper validationThis component handles custom fee inputs for non-Ethereum cryptocurrencies with appropriate validation rules and user feedback. The implementation includes:
- Network-specific validation (Bitcoin decimal limits)
- Range validation against min/max fee values
- Fee difference warnings when composed fee rate differs from input
- Clean UI with proper error messaging
packages/suite/src/components/wallet/Fees/StandardFee/BitcoinFeeCards.tsx (2)
41-42: Consider using nullish coalescing operator.Using the nullish coalescing operator (
??) instead of logical OR (||) is more appropriate when dealing with possibly undefined values, as it only falls back to the right-hand side when the left-hand side isnullorundefined, not when it's any falsy value.- amount={fee?.networkAmount ?? ''} + amount={fee?.networkAmount ?? ''}The code is using the nullish coalescing operator correctly.
1-54: The component looks well-structured and properly implements fee cards for Bitcoin transactions.The component does a good job of displaying fee options with relevant information like confirmation time, fiat value, and fee rate. The conditional rendering based on available information is also well-handled.
packages/suite/src/components/wallet/Fees/StandardFee/EthereumFeeCards.tsx (1)
1-41: The component is well-structured and properly implements fee cards for Ethereum transactions.The component effectively displays fee options with relevant information like fiat value and fee rate. The conditional rendering based on available information is handled appropriately.
packages/suite/src/components/wallet/Fees/StandardFee/MiscFeeCards.tsx (2)
19-23: Clearly handling network-specific fee display logic.The component correctly identifies Solana network and adjusts the fee amount display accordingly. The implementation properly handles the specific requirements for displaying fees for networks like Solana, Ripple, and Cardano.
1-51: The component effectively handles fee display for miscellaneous network types.The component appropriately handles the display of fees for networks like Solana, Ripple, and Cardano. The conditional logic for different network types is well-implemented, and the component integrates well with the overall fee display system.
packages/suite/src/components/wallet/Fees/StandardFee/FeeCard.tsx (1)
1-45: Well-designed reusable component for fee display.The
FeeCardcomponent is well-structured and provides a consistent way to display fee options across different network types. The component:
- Has well-defined props with appropriate types
- Uses a consistent layout structure
- Handles user interactions for selecting fee levels
- Provides flexibility through customizable child components for different sections of the card
This implementation promotes code reuse and maintains a consistent UI across the application.
packages/suite/src/components/wallet/Fees/CustomFee/CustomFeeEthereum.tsx (5)
52-55: Validate ETH decimals limit with proper condition.The Ethereum gas price validation handles decimal validation correctly, limiting it to 9 decimal places (GWEI precision). The exception condition is properly implemented to bypass this validation for non-Ethereum networks.
59-65: Check for proper handling of min/max fee range.The validation correctly verifies if the custom fee falls within the acceptable range and provides a clear error message that includes both minimum and maximum values.
69-80: Good UX approach for fee limit validation.The implementation includes a user-friendly approach to handling incorrect fee limits by providing a button that automatically sets the recommended value when the input is below the recommended limit.
81-99: Well-structured gas limit input with proper error handling.The gas limit input is well-structured and includes appropriate validation and error handling. The use of conditional rendering for error messages and the validation button enhances the user experience.
101-117: Properly implemented gas price input for legacy EVM transactions.The implementation for gas price input includes appropriate validation, fee unit display, and error message handling, following the pattern established in the gas limit input.
packages/suite/src/components/wallet/Fees/StandardFee/StandardFee.tsx (3)
42-61: Responsive design strategy for fee cards is well implemented.The resizeObserver implementation calculates the optimal number of columns based on the container width and minimum card width. This ensures good responsive behavior across different screen sizes.
79-85: Clean network-specific component mapping.The implementation uses a clear mapping strategy to select the appropriate fee cards component based on the network type. The fallback to MiscFeeCards ensures all network types are handled gracefully.
86-92: Well-structured responsive layout.The component renders a responsive grid layout for fee cards that adjusts based on available space, providing a consistent user experience across different device sizes.
packages/suite/src/components/wallet/Fees/CustomFee/CustomFee.tsx (2)
69-74: Validate logic handles different network types appropriately.The shared validation rules correctly distinguish between Bitcoin/Ethereum (requiring integers) and other networks. This ensures appropriate validation based on the network characteristics.
96-123: Network-specific custom fee components are cleanly conditionally rendered.The component conditionally renders either CustomFeeEthereum or CustomFeeMisc based on the network type, providing specialized fee inputs for each network while maintaining a consistent interface.
packages/suite/src/components/wallet/Fees/Fees.tsx (5)
46-47: Clear documentation for Solana-specific property.The comment clarifies that
feePerTxis Solana-specific, which helps maintain code clarity as the PR implements rent fees for Solana transactions.
101-111: Solana fee options correctly handle fee structure.The implementation for Solana network type properly extracts and structures fee options, including the Solana-specific
feePerTxproperty which will be used to display rent fees as mentioned in the PR objectives.
200-203: Label update improves clarity.Changing the fee level label from "normal" to "standard" improves UI consistency and user understanding.
213-224: Clean integration of new StandardFee component.The StandardFee component is properly integrated with all necessary props, replacing the previous FeeDetails component. This change supports the display of rent fees for Solana as part of the PR objectives.
229-230: Symbol prop added for network context.Adding the symbol prop to the CustomFee component ensures it has the necessary network context to properly display fee information, including rent fees for Solana transactions.
packages/suite/src/components/wallet/Fees/StandardFee/BitcoinFeeCards.tsx
Outdated
Show resolved
Hide resolved
| bottomLeftChild={ | ||
| <FiatValue | ||
| disableHiddenPlaceholder | ||
| amount={fee.networkAmount || ''} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace logical OR with nullish coalescing operator.
Using the nullish coalescing operator (??) instead of logical OR (||) is more appropriate when dealing with possibly undefined values, as it only falls back to the right-hand side when the left-hand side is null or undefined, not when it's any falsy value (like 0, which could be a valid amount).
- amount={fee.networkAmount || ''}
+ amount={fee.networkAmount ?? ''}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| amount={fee.networkAmount || ''} | |
| amount={fee.networkAmount ?? ''} |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| bottomLeftChild={ | ||
| <FiatValue | ||
| disableHiddenPlaceholder | ||
| amount={feeOption.networkAmount || ''} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace logical OR with nullish coalescing operator.
Using the nullish coalescing operator (??) instead of logical OR (||) is more appropriate when dealing with possibly undefined values, as it only falls back to the right-hand side when the left-hand side is null or undefined, not when it's any falsy value (like 0, which could be a valid amount).
- amount={feeOption.networkAmount || ''}
+ amount={feeOption.networkAmount ?? ''}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| amount={feeOption.networkAmount || ''} | |
| amount={feeOption.networkAmount ?? ''} |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
TomasBoda
force-pushed
the
feat/solana-rent-fee
branch
from
539c44e to
952b62a
Compare
|
How about here? |
|
Should we introduce something special also here? |
TomasBoda
force-pushed
the
feat/solana-rent-fee
branch
from
8de064b to
a74bf8a
Compare
Description
Calculate rent fee for Solana in send and stake form.
Related Issue
Resolve #17211
Screenshots
This tooltip text needs to be changed, cc @jirih-stsh