Skip to content

Conversation

@vignesha22
Copy link
Contributor

@vignesha22 vignesha22 commented Apr 29, 2025

Description

  • Fixed import folder

Types of changes

What types of changes does your code introduce?

  • Bugfix (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 not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Further comments (optional)

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling and address normalization when fetching sponsor balances in metadata endpoints, preventing failures if the Paymaster address is missing or invalid.
    • Adjusted whitelist checks in sponsor mode to account for different entry point versions, ensuring correct behavior for EPV_06.
  • New Features

    • Added a comprehensive mapping of oracle decimal values for multiple blockchains, enhancing accuracy when retrieving price data.
  • Chores

    • Updated documentation with a new changelog entry for version 4.0.2.
    • Bumped backend version to 4.0.2.

@coderabbitai
Copy link

coderabbitai bot commented Apr 29, 2025

Walkthrough

This update introduces a new version (4.0.2) with changes focused on improving paymaster and metadata handling. A new OracleDecimals constant is added to provide a mapping of oracle addresses to their decimal precision across multiple chains. The Paymaster class now uses this mapping to retrieve oracle decimals, falling back to on-chain queries when necessary. Metadata routes are updated to normalize addresses, add error handling for contract calls, and refine sponsor balance selection logic. Additionally, the paymaster route's whitelist check is adjusted to account for different entry point versions, specifically handling EPV_06 differently.

Changes

File(s) Change Summary
backend/CHANGELOG.md, backend/package.json Added a changelog entry for version 4.0.2 (2025-04-29) documenting fixes; incremented package.json version from 4.0.1 to 4.0.2.
backend/src/constants/MultitokenPaymaster.ts Introduced the OracleDecimals constant, a mapping of chain IDs and oracle addresses to their decimal precision. No changes to existing logic.
backend/src/paymaster/index.ts Imported OracleDecimals and added a new private method getChainlinkOracleDecimals to the Paymaster class for retrieving oracle decimals from the cache or on-chain. Updated getPriceFromChainlink to use this method.
backend/src/routes/metadata-routes.ts Added address normalization and error handling for sponsor balance and paymaster deposit retrieval in metadata endpoints. Adjusted logic to select the appropriate balance value for responses.
backend/src/routes/paymaster-routes.ts Modified whitelist check logic to exempt EPV_06 entry point version from unconditional errors when the sender is not whitelisted.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant MetadataRoute
    participant PaymasterContract
    participant Logger

    Client->>MetadataRoute: Request /metadata endpoint
    MetadataRoute->>MetadataRoute: Normalize paymaster address
    alt Paymaster address exists
        MetadataRoute->>PaymasterContract: getDeposit()
        alt Success
            PaymasterContract-->>MetadataRoute: Return deposit
        else Failure
            MetadataRoute->>Logger: Log error
            MetadataRoute->>MetadataRoute: Set sponsorBalance = 0
        end
    else
        MetadataRoute->>MetadataRoute: Set sponsorBalance = 0
    end
    MetadataRoute-->>Client: Respond with sponsorBalance
Loading
sequenceDiagram
    participant Paymaster
    participant OracleDecimals
    participant OracleContract

    Paymaster->>OracleDecimals: Check for decimals (chainId, oracleAddress)
    alt Found in OracleDecimals
        OracleDecimals-->>Paymaster: Return decimals
    else Not found
        Paymaster->>OracleContract: Call decimals()
        OracleContract-->>Paymaster: Return decimals
    end
    Paymaster->>Paymaster: Use decimals in price calculation
Loading

Poem

🐇✨
A hop, a skip, a version bump anew,
Decimals mapped for oracles—now clear and true!
Metadata checks with care and grace,
No more errors from a missing paymaster’s place.
Whitelists flex for EPV_06’s dance,
Rabbits rejoice—code gets a chance!
Onward we leap, with bugs left behind,
In this patch, more harmony you’ll find!


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 334de83 and e56532b.

📒 Files selected for processing (6)
  • backend/CHANGELOG.md (1 hunks)
  • backend/package.json (1 hunks)
  • backend/src/constants/MultitokenPaymaster.ts (1 hunks)
  • backend/src/paymaster/index.ts (3 hunks)
  • backend/src/routes/metadata-routes.ts (5 hunks)
  • backend/src/routes/paymaster-routes.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/paymaster/index.ts (1)
backend/src/constants/MultitokenPaymaster.ts (1)
  • OracleDecimals (206-360)
🪛 LanguageTool
backend/CHANGELOG.md

[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...data fetching if the config doesnt have Paymaster address ## [4.0.1] - 2025-04-22 ### Fi...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🪛 markdownlint-cli2 (0.17.2)
backend/CHANGELOG.md

2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


4-4: Lists should be surrounded by blank lines
null

(MD032, blanks-around-lists)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
backend/src/constants/MultitokenPaymaster.ts (1)

204-360: Excellent addition of oracle decimals as constants.

This addition of the OracleDecimals constant provides a comprehensive lookup table of oracle contract addresses and their decimal precision across multiple chains. This will help prevent errors from RPC endpoints that fail when querying decimals directly from the contract.

backend/src/paymaster/index.ts (3)

17-17: Import updated to include OracleDecimals constant.

The import statement has been correctly updated to include the newly added OracleDecimals constant from the MultitokenPaymaster.js file.


436-441: Good implementation of oracle decimals caching mechanism.

This new helper method getChainlinkOracleDecimals efficiently retrieves oracle decimals by first checking the constant mapping and only falling back to on-chain calls when necessary. This will improve reliability when some RPC endpoints fail to return decimals.


585-585: Method call updated to use the new caching mechanism.

The direct call to chainlinkContract.decimals() has been replaced with the new getChainlinkOracleDecimals method, which properly leverages the constants-based caching system.

backend/package.json (1)

3-3: Version bump to reflect the bugfix changes.

The version has been properly incremented to 4.0.2, which aligns with the changelog entry and follows semantic versioning principles for a bugfix release.

backend/src/routes/paymaster-routes.ts (1)

259-262: Whitelist bypass for EPV-06 can open an unintended sponsorship path

The new condition skips the DB-level whitelist for EPV_06 completely (!isWhitelisted && epVersion !== EPV_06).
If a user’s address is not whitelisted in the database and the network operator has not deployed a verifying paymaster, signV06 will still sign the UserOp with the global paymaster, effectively sponsoring any address on-chain.

Please double-check that:

  1. signV06 / the on-chain paymaster still performs an equivalent check, and
  2. agnostic clients cannot force EPV_06 to get free sponsorship by simply pointing to the old entry-point.

If the on-chain guard is weaker (or was purposely removed), consider one of:

- if (!isWhitelisted && epVersion !== EPVersions.EPV_06) {
+ if (!isWhitelisted && (epVersion !== EPVersions.EPV_06 || !networkConfig.contracts.etherspotPaymasterAddress)) {
     throw new Error('This sender address has not been whitelisted yet');
   }

or keep the current behaviour but add a comment + doc clarification explaining why EPV_06 is safe without the DB whitelist.

backend/src/routes/metadata-routes.ts (1)

239-240: Nullish-coalescing picks VP deposit even if it is zero

verifyingPaymasterDeposit ?? sponsorBalance will always favour verifyingPaymasterDeposit, even when it equals 0 (a valid BigNumber).
This shadows a non-zero sponsorBalance and can under-report available funds.

Consider an explicit “greater-than-zero” preference:

-  sponsorBalance: verifyingPaymasterDeposit ?? sponsorBalance,
+  sponsorBalance: (verifyingPaymasterDeposit.gt(0) ? verifyingPaymasterDeposit : sponsorBalance),

(Replace .gt(0) with an equivalent check depending on the final numeric format.)

Also applies to: 339-340

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

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • 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 generate sequence diagram to generate a sequence diagram of the changes in 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.

@vignesha22 vignesha22 changed the base branch from master to develop April 29, 2025 18:33
@vignesha22 vignesha22 merged commit 6512a57 into develop Apr 29, 2025
2 of 4 checks passed
@vignesha22 vignesha22 deleted the Import_Bug_Fix branch April 29, 2025 18:36
Copy link

@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.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
backend/src/routes/metadata-routes.ts (1)

35-144: 🧹 Nitpick (assertive)

Heavy code duplication across /metadata, /metadata/v2 and /metadata/v3

The three handlers share ~80 % identical logic (secret retrieval, network config, sponsor paymaster look-ups, etc.).
Maintaining fixes (like the BigNumber bug) in three places invites divergence.

Nice-to-have refactor idea:

  1. Extract a helper getMetadata({ apiKey, chainId, epVersion }) that returns the prepared payload.
  2. Each route becomes a thin wrapper deciding the EP version and delegating to the helper.

This reduces maintenance cost and the chance of future inconsistencies.

Also applies to: 147-255, 257-355

🧰 Tools
🪛 Biome (1.9.4)

[error] 48-48: Declare variables separately

Unsafe fix: Break out into multiple declarations

(lint/style/useSingleVarDeclarator)

🛑 Comments failed to post (2)
backend/CHANGELOG.md (1)

2-6: 🧹 Nitpick (assertive)

Comprehensive changelog entry for the new version.

The changelog entry clearly documents the two fixes included in this version:

  1. Addition of oracle decimals as constants for the multiTokenPaymaster
  2. Fix for metadata fetching when configuration lacks a Paymaster address

However, there's a minor grammar issue in the second point.

- Fixed bug in metadata fetching if the config doesnt have Paymaster address
+ Fixed bug in metadata fetching if the config doesn't have a Paymaster address
📝 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.

## [4.0.2] - 2025-04-29
### Fixes
- Added oracle decimals as constants for multiTokenPaymaster as some rpc endpoints return error
- Fixed bug in metadata fetching if the config doesn't have a Paymaster address
🧰 Tools
🪛 LanguageTool

[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...data fetching if the config doesnt have Paymaster address ## [4.0.1] - 2025-04-22 ### Fi...

(AI_EN_LECTOR_MISSING_DETERMINER_A)

🪛 markdownlint-cli2 (0.17.2)

2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above

(MD022, blanks-around-headings)


3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


4-4: Lists should be surrounded by blank lines
null

(MD032, blanks-around-lists)

backend/src/routes/metadata-routes.ts (1)

95-117: ⚠️ Potential issue

Use BigNumber.from(0) instead of numeric 0 to avoid type drift & runtime surprises

sponsorBalance / verifyingPaymasterDeposit start as the literal number 0 and are later overwritten with an ethers.BigNumber instance.
Mixing number and BigNumber leads to a union type (number | BigNumber), which:

  • defeats static-type checks,
  • can break runtime JSON serialisation (native number vs BigNumber’s toJSON()),
  • causes subtle bugs when callers expect only one type.

Recommended quick fix (repeat for every affected declaration):

-import { Contract, Wallet, providers, utils } from "ethers";
+import { Contract, Wallet, providers, utils, BigNumber } from "ethers";
@@
-      let sponsorBalance = 0;
+      let sponsorBalance = BigNumber.from(0);
@@
-      let verifyingPaymasterDeposit = 0;
+      let verifyingPaymasterDeposit = BigNumber.from(0);

…and at response time either toString() or formatUnits() the value so the API consumer receives a predictable JSON type.

Also applies to: 207-216, 218-227, 319-327

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants