Skip to content

Comments

fix/PRO-3156/exchange-gnosis-issues#269

Merged
RanaBug merged 1 commit intostagingfrom
fix/PRO-3156/exchange-gnosis-issues
Mar 25, 2025
Merged

fix/PRO-3156/exchange-gnosis-issues#269
RanaBug merged 1 commit intostagingfrom
fix/PRO-3156/exchange-gnosis-issues

Conversation

@RanaBug
Copy link
Collaborator

@RanaBug RanaBug commented Mar 25, 2025

Description

  • Fix XDAI and ETH balance in the Exchange App
  • Fix swaps transactions in batch transactions

How Has This Been Tested?

  • Manual testing

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

  • New Features
    • Enhanced exchange processing with improved formatting of transaction values for clearer display during exchanges.
    • Refined token presentation with updated naming and filtering, ensuring users see more accurate token information.

@RanaBug RanaBug requested a review from IAmKio March 25, 2025 16:17
@RanaBug RanaBug self-assigned this Mar 25, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 25, 2025

Walkthrough

This pull request updates the processing of transaction values in the ExchangeAction component and enhances the token mapping in the tokensData service. In the ExchangeAction component, transaction values are now converted using BigNumber from ethers and formatted using formatEther from viem before being batched. In the tokensData service, conditional logic adjusts token names and symbols based on specific criteria and filters out certain tokens based on contract addresses. No changes were made to the exported entities in either module.

Changes

File(s) Change Summary
src/apps/.../ExchangeAction.tsx Modified transaction processing: replaced ethers import with BigNumber, added formatEther from viem, converted transaction values to BigInt, formatted them to string representation, and applied these changes in both bestOffer.offer and stepTransactions before batching.
src/services/tokensData.ts Enhanced token mapping: added conditional logic to update token name and symbol (e.g., converting 'XDAI' to 'Wrapped XDAI' and 'Ethereum' to 'Wrapped Ether' under certain conditions) and filtered tokens with a specific contract to improve type safety and accuracy in token data returns.

Sequence Diagram(s)

sequenceDiagram
    participant UI as User Interface
    participant EA as ExchangeAction Component
    participant BN as BigNumber Conversion
    participant FE as FormatEther Function
    participant BT as Batch Processor

    UI->>EA: Click Exchange Button
    EA->>BN: Convert transaction value
    BN-->>EA: Return BigInt value
    EA->>FE: Format BigInt to Ether string
    FE-->>EA: Return formatted value
    EA->>BT: addToBatch(formatted value)
Loading
sequenceDiagram
    participant Data as loadTokensData Function
    participant Mapper as Token Mapper
    participant Filter as Token Filter

    Data->>Mapper: Map tokens data for each blockchain
    Mapper-->>Data: Apply conditional name & symbol changes
    Data->>Filter: Exclude tokens by contract check and filter nulls
    Filter-->>Data: Return cleaned, updated token list
Loading

Suggested reviewers

  • IAmKio

Poem

I'm a rabbit hopping with delight,
In lines of code both crisp and bright.
Transactions now glow with newfound grace,
While tokens transform in their rightful place.
My little paws applaud each change,
Celebrating progress in this coding range!

✨ 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
🪧 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 plan to trigger planning for file edits and PR creation.
  • @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.

@cloudflare-workers-and-pages
Copy link

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: a3fceb7
Status: ✅  Deploy successful!
Preview URL: https://c9a2e8ae.x-e62.pages.dev
Branch Preview URL: https://fix-pro-3156-exchange-gnosis.x-e62.pages.dev

View logs

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: 2

🧹 Nitpick comments (2)
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (1)

3-5: Consider avoiding mixing libraries for big number manipulation.
You’re importing BigNumber from ethers and formatEther from viem. While this can work, relying on multiple libraries to handle big-number logic can introduce confusion and subtle inconsistencies. If possible, consolidate the handling of big-number arithmetic and Ether formatting under a single library for more predictable behavior.

src/services/tokensData.ts (1)

50-84: Consolidate special token handling and confirm downstream usage.
You’re conditionally renaming XDAI to WXDAI and Ethereum to WETH when the contract address differs from the native placeholder (0xeeee...). While this approach is valid, consider encapsulating these transformations in a helper function or data structure to reduce repetition and facilitate maintenance if new cases emerge. Also verify that no consumer of this data still relies on the original token names/symbols.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4d0e822 and a3fceb7.

📒 Files selected for processing (2)
  • src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (5 hunks)
  • src/services/tokensData.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: unit-tests
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
src/apps/the-exchange/components/ExchangeAction/ExchangeAction.tsx (2)

125-127: Ensure transaction value is correctly formatted for cross-chain offers.
Passing the output of formatEther(bigIntValue) might lead to unexpected results if the underlying cross-chain transactions require a WEI integer value. Double-check the entire flow to confirm that the contract calls and signing methods align with this decimal representation.


109-109:

Details

❓ Verification inconclusive

Verify transaction value usage in addToBatch.
Here, value: integerValue is a string from formatEther. Confirm whether addToBatch is designed to handle decimal ETH strings or if it requires a WEI-based format.


🏁 Script executed:

#!/bin/bash
# Search for the definition of addToBatch to see its parameter types or usage.
rg -A 15 "function addToBatch" 

Length of output: 32


I'm not seeing any reference to a definition or usage of addToBatch with the initial search. To improve our verification, let's run an updated search for any occurrence of addToBatch in the codebase—which may reveal its declaration or usage pattern (e.g. via arrow functions, class methods, or assignments). This should help clarify if it expects a WEI value or a decimal string from formatEther.

Below is a script to recursively search for any instance of addToBatch with some context:

Once we see the output from this search, we can determine whether a manual verification is needed to check if addToBatch expects a WEI-based format.


🏁 Script executed:

#!/bin/bash
rg -n -A 10 "addToBatch" .

Length of output: 15680


Action Required: Verify Transaction Value Format

Upon inspection, addToBatch (defined in src/providers/GlobalTransactionsBatchProvider.tsx) simply accepts and stores the transaction object without performing any conversion on its fields. The value property is being passed as integerValue—a string output from formatEther (a decimal representation of ETH). There’s no internal logic converting this value to a WEI-based format.

  • Please verify that downstream processes consuming this transaction are designed to work with a decimal ETH string.
  • If a WEI-based value is required, convert integerValue using the appropriate method (for example, utilizing parseEther or another conversion utility) before passing it to addToBatch.

Comment on lines +94 to +96
const { value } = bestOffer.offer.transactions[i];
const bigIntValue = BigNumber.from(value).toBigInt();
const integerValue = formatEther(bigIntValue);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Check for mismatch in transaction value representation.
Using formatEther(bigIntValue) produces a decimal string (e.g. "0.5" for half an ETH), whereas most lower-level transaction methods expect a value designated in WEI. Ensure addToBatch and subsequent transaction logic will properly interpret the returned string. If it expects a WEI amount, consider passing the raw wei value instead (e.g., a BigInt or a hex string).

chainId: chainNameToChainIdTokensData(swapToken?.blockchain) || 0,
to: stepTransactions[i].to || '',
value: ethers.BigNumber.from(stepTransactions[i].value),
value: integerValue,
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconfirm the final transaction value format.
As with previous segments, value: integerValue is a decimal string from formatEther, which could introduce confusion or errors if the receiving function or contract requires a raw WEI value.

Copy link
Collaborator

@IAmKio IAmKio left a comment

Choose a reason for hiding this comment

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

LGTM

@RanaBug RanaBug merged commit 6a1a4e2 into staging Mar 25, 2025
7 checks passed
@RanaBug RanaBug deleted the fix/PRO-3156/exchange-gnosis-issues branch April 3, 2025 09:08
@coderabbitai coderabbitai bot mentioned this pull request Aug 8, 2025
3 tasks
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