fix(trading): unserialized store data

[adderpositive]

Description

Delete unserialized data from the store and created special selectors for this purpose.

Related Issue

Resolve #17620

Screenshots

Before

After

[coderabbitai]

Walkthrough

The changes standardize the retrieval and structure of trading-related state across multiple modules. Several files now use memoized selectors—such as selectTradingSellInfo, selectTradingExchangeInfo, and selectTradingBuyInfo—instead of directly accessing Redux state. The data structures in trading modules have been revised: properties such as buySymbols and sellSymbols have moved from sets to arrays in certain contexts, while selectors still return unique values. Adjustments were made to type definitions, with new types like TradingExchangeInfoSelector and TradingSellInfoSelector replacing previous interfaces. Additionally, tests and fixtures have been updated to reflect these structural and selector-based changes, and remnants of older helper functions were removed, aligning the logic for loading and processing trading details with the new patterns.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ddf478 and 8a2403f.

📒 Files selected for processing (13)

• packages/suite/src/actions/wallet/__tests__/tradingExchangeActions.test.ts (2 hunks)
• packages/suite/src/actions/wallet/tradingExchangeActions.ts (2 hunks)
• packages/suite/src/actions/wallet/tradingSellActions.ts (2 hunks)
• packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts (2 hunks)
• packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts (2 hunks)
• packages/suite/src/hooks/wallet/trading/form/useTradingSellFormDefaultValues.ts (2 hunks)
• packages/suite/src/hooks/wallet/trading/useTradingDetail.ts (4 hunks)
• packages/suite/src/reducers/wallet/__tests__/tradingReducer.test.ts (1 hunks)
• packages/suite/src/reducers/wallet/tradingReducer.ts (3 hunks)
• packages/suite/src/types/trading/trading.ts (3 hunks)
• packages/suite/src/types/trading/tradingForm.ts (3 hunks)
• packages/suite/src/utils/wallet/trading/__fixtures__/exchangeUtils.ts (1 hunks)
• packages/suite/src/views/wallet/trading/common/TradingTransactions/TradingTransactionsList.tsx (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (9)

• GitHub Check: run-e2e-suite-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
• GitHub Check: run-e2e-suite-desktop-tests (@group=other, trezor-user-env-unix)
• GitHub Check: run-e2e-suite-desktop-tests (@group=passphrase, trezor-user-env-unix)
• GitHub Check: run-e2e-suite-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
• GitHub Check: run-e2e-suite-desktop-tests (@group=device-management, trezor-user-env-unix)
• GitHub Check: run-e2e-suite-desktop-tests (@group=suite, trezor-user-env-unix)
• GitHub Check: Setup and Cache Dependencies
• GitHub Check: build-web
• GitHub Check: Analyze with CodeQL (javascript)

🔇 Additional comments (38)

packages/suite/src/actions/wallet/tradingSellActions.ts (4)

21-22: LGTM - Data structure change from Set to array

The change from Set<string> to string[] for currencies improves serialization capabilities of the store data, which aligns well with the PR objective.

---

25-31: Good selector pattern implementation

Creating a specialized selector type that converts arrays back to Sets provides a clean interface for consumers while keeping the store serializable.

---

69-78: Array-based implementation looks good

The implementation correctly uses array methods (push) instead of Set methods (add) to populate the collections, maintaining consistency with the new data structure approach.

---

83-84: Effective deduplication strategy

Using [...new Set(array)] is an elegant way to deduplicate values while maintaining the array structure in the store.

packages/suite/src/reducers/wallet/__tests__/tradingReducer.test.ts (1)

159-161: Test fixture updated correctly for new data structure

The test fixtures have been properly updated to use arrays instead of Sets, ensuring the tests align with the implementation changes.

packages/suite/src/utils/wallet/trading/__fixtures__/exchangeUtils.ts (1)

453-454: Fixture updated for array-based storage

The exchange info fixture now correctly uses empty arrays instead of empty Sets, aligning with the serializable store approach.

packages/suite/src/actions/wallet/__tests__/tradingExchangeActions.test.ts (2)

5-6: Good imports for selector pattern

Adding imports for AppState and the new selector supports the transition to a selector-based state access pattern.

---

112-117: Test updated to use selector pattern

The test now correctly uses the selector to access state rather than directly accessing it, which is a better practice that mirrors how components will access the data.

Note that the test is comparing against new Set<string> even though the store now contains arrays, which is correct since the selector is expected to transform the arrays into Sets.

packages/suite/src/hooks/wallet/trading/form/useTradingSellForm.ts (2)

50-50: Good addition of the selector import.

Adding a dedicated selector import is a positive change that follows best practices for Redux state management.

---

81-81: Improved state access with dedicated selector.

Replacing direct state access with a memoized selector provides better performance and maintainability. This change follows Redux best practices by encapsulating state selection logic.

packages/suite/src/views/wallet/trading/common/TradingTransactions/TradingTransactionsList.tsx (2)

12-15: Good addition of selector imports.

Adding imports for dedicated selectors follows Redux best practices and creates a more maintainable codebase.

---

44-45: Improved state access using selectors.

Replacing direct state access with dedicated selectors enhances code maintainability and follows best practices. The selector pattern centralizes state access logic, making it easier to refactor and test.

packages/suite/src/hooks/wallet/trading/form/useTradingExchangeForm.ts (2)

53-53: Good addition of the selector import.

Adding a dedicated selector import follows best practices for Redux state management.

---

86-86: Improved state access with dedicated selector.

Converting from direct state access to using a memoized selector improves code maintainability and follows Redux best practices. This change encapsulates the state selection logic in a reusable function.

packages/suite/src/hooks/wallet/trading/form/useTradingSellFormDefaultValues.ts (2)

11-11: Good type import update.

Importing the more specific TradingSellInfoSelector type instead of the general SellInfo improves type safety.

---

27-27: Improved parameter typing.

Updating the parameter type to use the more specific TradingSellInfoSelector enhances type safety and aligns with the changes in data structure from sets to arrays. This change ensures better consistency across the codebase.

packages/suite/src/actions/wallet/tradingExchangeActions.ts (5)

22-24: Good approach to storing serializable data structures in Redux store

Switching from using Set<CryptoId> to arrays for buySymbols and sellSymbols is good practice because arrays are serializable while Sets are not. This helps prevent issues with Redux store serialization.

---

26-29: Well-designed selector type pattern

Creating a selector type that converts arrays to Sets is a good pattern. This allows you to store serializable data in Redux while providing the benefits of Set (uniqueness, efficient lookups) to the consuming components.

---

67-67: Consistent with array storage pattern

Correctly updated to return empty arrays instead of empty sets, maintaining consistency with the serializable data pattern.

---

74-83: Improved collection logic using arrays

Good refactoring of the symbol collection logic to use arrays with push operations instead of Sets, which aligns with the new serializable data approach.

---

89-90: Effective deduplication while maintaining serializable format

Using [...new Set(array)] is a clean way to deduplicate values while preserving the array format. This ensures the data remains serializable while still providing unique values.

packages/suite/src/types/trading/tradingForm.ts (3)

43-44: Good approach using selector types for form components

Importing the selector types rather than direct data structure types is the right approach for form components that need to work with typed data from selectors.

---

200-200: Updated to use the serialization-friendly pattern

Correctly updated to use TradingSellInfoSelector instead of direct SellInfo type, ensuring form components receive properly typed data from selectors.

---

244-244: Consistent selector typing across form interfaces

Good consistency in using TradingExchangeInfoSelector here, matching the pattern used for sell information.

packages/suite/src/hooks/wallet/trading/useTradingDetail.ts (4)

8-8: Good approach importing specialized selectors

Adding imports for the specific selectors improves code organization and readability by making explicit which selectors are being used.

Also applies to: 17-19

---

62-62: Simplified logic for retrieving trading information

Direct access to the infos parameter makes the code more straightforward and easier to understand compared to the previous implementation that likely used a separate function.

---

103-105: Clean implementation using selectors

Using selectors directly in the hook provides type safety and encapsulates the state structure, making the code more maintainable.

---

110-114: Structured object passing improves code clarity

Passing the selectors as a structured object with named properties makes the code more readable and maintains the relationship between the data types.

packages/suite/src/types/trading/trading.ts (3)

35-36: Correct imports for selector types

Importing the specialized selector types improves type checking and makes the code more maintainable.

---

79-80: Updated trade info map to use selector types

This change ensures consistency across the app by using selector types in the trade info map, aligning with the serialization-friendly approach.

---

93-97: Well-structured infos object with proper typing

Creating a properly typed structure for the infos object improves type safety and makes the component interfaces clearer.

packages/suite/src/reducers/wallet/tradingReducer.ts (7)

12-12: Good addition of memoization utility

Adding the createWeakMapSelector import enables performance optimization for selectors and aligns with Redux best practices for derived data.

---

29-33: Type definitions properly updated

The addition of TradingExchangeInfoSelector and TradingSellInfoSelector types provides good type safety for the new selectors.

---

215-216: Well-typed memoization setup

Creating a typed version of the selector creator enhances type safety throughout the Redux selectors.

---

217-230: Good implementation of memoized sell info selector

The selectTradingSellInfo selector properly transforms the sell information into a standardized format with Sets for currency collections. The null check and early return pattern is appropriate for handling the undefined case.

---

232-245: Good implementation of memoized exchange info selector

The selectTradingExchangeInfo selector follows the same pattern as the sell info selector, ensuring consistency. Converting arrays to Sets for symbol collections standardizes the data format for consumers of this selector.

---

251-257: Good refactoring of the selector usage

Updating selectSupportedSymbols to use the new memoized selectors improves performance and maintainability. This change aligns with the PR objective of fixing unserialized store data by ensuring data is accessed through standardized selectors.

---

215-245:

✅ Verification successful

Verify serialization/deserialization across Redux lifecycle

These selectors convert array data to Sets, which aren't directly serializable in JSON. This aligns with the PR objective of fixing "unserialized store data," but ensure the Redux store itself contains serializable data while these selectors provide the runtime Sets.

---

🏁 Script executed:

#!/bin/bash
# Verify that the store is using serializable data structures for trading info

# Check if the trading reducer actions maintain serializable data
echo "Checking trading action creators for serializable data:"
rg -A 5 "TRADING_(EXCHANGE|SELL).SAVE_.*_INFO" --include="*.ts" | grep -v "Set("

# Check for serialization/deserialization in related files
echo "Looking for JSON serialization/deserialization usage:"
rg "(JSON\.parse|JSON\.stringify)" --include="*.ts" --include="*.tsx" -A 3 -B 3

Length of output: 1286

---

Below is a revised verification script that uses glob flags (‑g) instead of the unsupported “--include” flag. Please run this script manually to gather the output for verifying that your trading reducer actions maintain serializable data and that JSON serialization/deserialization is handled correctly. Once you’ve obtained the output, we can determine if any further changes are needed.

Please verify the results manually and confirm that:

• The actions (e.g., those matching TRADING_(EXCHANGE|SELL).SAVE_.*_INFO) are not inadvertently introducing non-serializable data (like a Set).
• There is no unexpected JSON serialization/deserialization logic that might indicate hidden issues in the Redux flow.

Once verified, we can be confident the runtime selectors converting arrays to Sets do not compromise the serializability of the Redux store.

---

🏁 Script executed:

#!/bin/bash
echo "Checking trading action creators for serializable data structures:"
rg -g '*.ts' 'TRADING_(EXCHANGE|SELL).SAVE_.*_INFO' -A 5 | grep -v "Set("

echo -e "\nChecking JSON serialization/deserialization usage in code files:"
rg -g '*.ts' -g '*.tsx' '(JSON\.parse|JSON\.stringify)' -A 3 -B 3

Length of output: 68215

---

Redux Store Serialization Verified
The verification confirms that the trading reducer and action creators are maintaining serializable data (using plain arrays/objects) while the selectors transform these into runtime Sets. This design meets the PR objective without compromising the Redux store’s serializability.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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 generate docstrings to generate docstrings for this 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.