Release 24.04.2025 #204
Release 24.04.2025 #204
Conversation
* added changes to support MTP for EPV07
* Changes on epv08 * cron job fix * added default config and deposit api * added check whitelist fn for sponsor and useVp * added checks for policyId to be number
* added seed data * removed entryPoint params * updated version
WalkthroughThis update introduces comprehensive support for a new entry point version, EPV_08, across the backend service. The changes span configuration, environment variables, ABIs, paymaster logic, models, migrations, and all major API routes. New ABIs and bytecode for VerifyingPaymaster V3 and MultiTokenPaymaster V2 are added. Database schema migrations introduce new columns for EP versioning and paymaster addresses. The codebase is refactored to generalize paymaster logic, add new signing and deployment methods for EPV_08, and extend whitelist, deposit, metadata, and paymaster routes to handle the new version. Configuration handling and error messages are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
participant Paymaster
participant Blockchain
Client->>API: Request (e.g., /paymaster, /whitelist/v3, /deposit/v3) with epVersion=EPV_08
API->>DB: Lookup API key, whitelist, paymaster addresses for EPV_08
DB-->>API: Return config, whitelist, paymaster info
API->>Paymaster: Instantiate with EPV_08 config & ABI
API->>Blockchain: Interact with EPV_08 paymaster contract (sign, deploy, deposit, etc.)
Blockchain-->>API: Return contract results (signatures, receipts, balances)
API-->>Client: Respond with result (paymasterAndData, status, etc.)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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: 30
🔭 Outside diff range comments (7)
backend/src/paymaster/token.test.ts (1)
44-61: 💡 Verification agent🧩 Analysis chain
Tests should be enabled to verify functionality
All tests are currently skipped with
test.skip. Given the significant refactoring from PimlicoPaymaster to TokenPaymaster, these tests should be enabled to verify the functionality works correctly with the new implementation.Run this script to see if there are any active TokenPaymaster tests elsewhere in the codebase:
Also applies to: 63-81, 83-102, 104-124, 126-159, 161-194, 196-230, 232-266, 268-303, 305-339, 341-376, 378-413, 415-450, 452-487
🏁 Script executed:
#!/bin/bash # Check if there are any non-skipped TokenPaymaster tests rg "describe\(\"TokenPaymaster" --type js --type ts | grep -v "skip" # Check for other related test files that might cover this functionality fd "token.test" --type file --exec grep -l "TokenPaymaster" {} \;Length of output: 253
Enable the SMOKE calculateTokenAmount test
The file
backend/src/paymaster/token.test.tsalready contains an active suite under
describe("TokenPaymaster on Mumbai", …). Only the SMOKE test forcalculateTokenAmountis currently skipped.• File:
backend/src/paymaster/token.test.ts
Lines: 44–61- test.skip("SMOKE: validate the calculateTokenAmount function with valid details", async () => { + test("SMOKE: validate the calculateTokenAmount function with valid details", async () => {Please remove the
.skiponce the new TokenPaymaster implementation is ready, so that we verifycalculateTokenAmountworks as expected.backend/src/repository/whitelist-repository.ts (1)
68-84:⚠️ Potential issueMissing epVersion field in updateOneById method
The
updateOneByIdmethod doesn't include theepVersionfield in the list of fields to update. This could lead to the field being reset to null when updating a record.Apply this fix to include the epVersion field in updates:
async updateOneById(record: ArkaWhitelist): Promise<ArkaWhitelist | null> { const result = await this.sequelize.models.ArkaWhitelist.update({ apiKey: record.apiKey, addresses: record.addresses, - policyId: record.policyId + policyId: record.policyId, + epVersion: record.epVersion }, { where: { id: record.id } })backend/src/plugins/config.ts (2)
18-45:⚠️ Potential issueOptional properties are marked as required – validation will now fail
Using the pattern
Type.String() || undefinedalways returns the left operand (a truthy object), so the field stays required.
Example:EP7_TOKEN_VGL: Type.String() || undefined // still “required string”With the new
EPV_08additions this will reject any.envthat omits those keys, contradicting the fallback defaults you add later.Suggested fix for every optional value:
-EP7_TOKEN_VGL: Type.String() || undefined, +EP7_TOKEN_VGL: Type.Optional(Type.String()),Do this for
EP7_TOKEN_PGL,EPV_06,EPV_07,EPV_08,MTP_VGL_MARKUP,USE_SKANDHA_FOR_GAS_DATA,MTP_PVGL,MTP_PPGL, etc.Without the change
validate(envVar)will throw on fresh deployments.
74-91: 🛠️ Refactor suggestionUndefined →
undefinedarrays break schema check
process.env.EPV_08?.split(',')yieldsundefinedwhen the env var is absent, but (after the previous issue is fixed) the schema still expects an array (even when optional).
A simple fallback prevents runtimeTypeError: Cannot read properties of undefined (reading 'split'):EPV_06: (process.env.EPV_06 ?? '').split(',').filter(Boolean), EPV_07: (process.env.EPV_07 ?? '').split(',').filter(Boolean), -EPV_08: process.env.EPV_08?.split(','), +EPV_08: (process.env.EPV_08 ?? '').split(',').filter(Boolean),Or rely on defaults directly when empty.
backend/src/paymaster/index.ts (3)
69-82: 🧹 Nitpick (assertive)Constructor signature is getting unwieldy – switch to a config object
Nine positional parameters (and growing) are hard to reason about and easy to mis-order.
Prefer passing a singleoptionsobject (or loading from an injected config service) so that:
- Call-sites become self-documenting (
new Paymaster({ feeMarkUp, multiTokenMarkUp, … }))- Future parameters are additive and backwards-compatible.
978-979: 🛠️ Refactor suggestionPermit exact-balance payments
gteblocks the case where the sender holds exactly the required amount.
Use>(orgt) so equality is allowed.- if (tokenAmountRequired.gte(tokenBalance)) + if (tokenAmountRequired.gt(tokenBalance))
1404-1410:⚠️ Potential issueMissing
returncauses silentundefinedInside the
catchblock ofgetPriceFromCoingecko, whentokenDatadoes exist you silently fall through without returning, so the caller receivesundefinedand will likely crash later.- if (!tokenData) { - log?.error('Price fetch error on tokenAddress: ' + tokenAddress, 'CoingeckoError') - throw new Error(`${tokenAddress} ${ErrorMessage.COINGECKO_PRICE_NOT_FETCHED}`); - } + if (!tokenData) { + log?.error(`Price fetch error on tokenAddress: ${tokenAddress}`, 'CoingeckoError') + throw new Error(`${tokenAddress} ${ErrorMessage.COINGECKO_PRICE_NOT_FETCHED}`); + } + return { + ethPrice: ethers.utils.parseUnits( + (Number(nativePrice) / tokenData.price).toFixed(tokenData.decimals), + tokenData.decimals + ), + ...tokenData + };Without this, every happy-path through the
catchresults inundefined.🧰 Tools
🪛 Biome (1.9.4)
[error] 1405-1405: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 1408-1408: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (33)
backend/.env.example(1 hunks)backend/CHANGELOG.md(1 hunks)backend/config.json.default(1 hunks)backend/migrations/2025032800001-update-apiKey-table.cjs(1 hunks)backend/migrations/2025032800002-update-arka-whitelist-table.cjs(1 hunks)backend/migrations/20250416000001-update-apiKey-table.cjs(1 hunks)backend/migrations/20250416000002-update-MTP-table.cjs(1 hunks)backend/migrations/2025042200001-seed-data.cjs(1 hunks)backend/package.json(1 hunks)backend/src/abi/MultiTokenPaymasterAbiV2.ts(1 hunks)backend/src/abi/VerifyingPaymasterAbiV3.ts(1 hunks)backend/src/constants/ErrorMessage.ts(1 hunks)backend/src/constants/MultitokenPaymaster.ts(1 hunks)backend/src/models/api-key.ts(3 hunks)backend/src/models/multiTokenPaymaster.ts(2 hunks)backend/src/models/whitelist.ts(2 hunks)backend/src/paymaster/index.test.ts(1 hunks)backend/src/paymaster/index.ts(13 hunks)backend/src/paymaster/token.test.ts(3 hunks)backend/src/paymaster/token.ts(5 hunks)backend/src/plugins/config.ts(5 hunks)backend/src/repository/api-key-repository.ts(2 hunks)backend/src/repository/multi-token-paymaster.ts(2 hunks)backend/src/repository/whitelist-repository.ts(3 hunks)backend/src/routes/admin-routes.ts(7 hunks)backend/src/routes/deposit-route.ts(3 hunks)backend/src/routes/metadata-routes.ts(4 hunks)backend/src/routes/paymaster-routes.ts(15 hunks)backend/src/routes/token-routes.ts(4 hunks)backend/src/routes/whitelist-routes.ts(13 hunks)backend/src/server.ts(7 hunks)backend/src/types/sponsorship-policy-dto.ts(3 hunks)backend/src/types/whitelist-dto.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/src/paymaster/token.test.ts (1)
backend/src/paymaster/token.ts (1)
TokenPaymaster(17-90)
backend/src/server.ts (2)
backend/src/paymaster/index.ts (1)
Paymaster(53-1421)backend/src/utils/monitorTokenPaymaster.ts (1)
checkDeposit(6-22)
backend/src/repository/whitelist-repository.ts (1)
backend/src/models/whitelist.ts (1)
ArkaWhitelist(3-11)
🪛 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)
6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
8-8: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
12-12: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
🪛 Biome (1.9.4)
backend/src/routes/admin-routes.ts
[error] 504-504: Declare variables separately
Unsafe fix: Break out into multiple declarations
(lint/style/useSingleVarDeclarator)
backend/src/routes/metadata-routes.ts
[error] 247-247: Declare variables separately
Unsafe fix: Break out into multiple declarations
(lint/style/useSingleVarDeclarator)
backend/src/routes/paymaster-routes.ts
[error] 305-305: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
backend/src/paymaster/index.ts
[error] 352-352: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 949-949: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (41)
backend/src/paymaster/index.test.ts (1)
4-4: Update import path forPAYMASTER_ADDRESS
The test now importsPAYMASTER_ADDRESSfrom../constants/Token.js, matching the updated constants module.backend/src/constants/ErrorMessage.ts (1)
59-60: Add missing comma and new error message
A comma was correctly added afterCOINGECKO_PRICE_NOT_FETCHED, and the newMTP_EP_SUPPORTmessage clearly indicates unsupported entry point versions for multi-token paymaster. Ensure this constant is used in the relevant routes so that users receive this error when hitting an unsupported EP version.backend/src/models/whitelist.ts (2)
8-8: Well-structured model property additionAdding the
epVersionproperty with appropriate typing and default value correctly extends the model to support multiple entry point versions.
38-42: Properly defined database column for EP versioningThe database column definition for
epVersionfollows good practices with proper typing, nullability, and field naming conventions that match the rest of the model.backend/src/constants/MultitokenPaymaster.ts (1)
192-193: Good addition of ETH support for ArbitrumAdding native ETH support with the standard placeholder address (
0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE) is appropriate and follows conventions for representing native tokens in token lists.backend/config.json.default (1)
472-481: Proper configuration for EPV_08 on SepoliaThe addition of a new configuration entry for Sepolia (chainId 11155111) with a v3 bundler and the EPV_08 entryPoint address is correctly structured and follows the pattern of other configurations in the file.
backend/migrations/2025032800001-update-apiKey-table.cjs (1)
1-13: LGTM! Migration properly adds the VERIFYING_PAYMASTERS_V3 column.The migration correctly adds the new column to store verifying paymaster addresses for EPV_08 in the api_keys table.
backend/migrations/2025032800002-update-arka-whitelist-table.cjs (1)
1-13: LGTM! Migration properly adds the EP_VERSION column.The migration correctly adds the new column to track entry point versions for whitelist entries, supporting the new EPV_08 functionality.
backend/src/repository/multi-token-paymaster.ts (1)
3-3: LGTM! Properly imported EPVersions enum.The import is correctly added to support strongly-typed version parameters.
backend/src/models/multiTokenPaymaster.ts (1)
10-10: LGTM! Added epVersion property to model class.Properly added the new property to track entry point versions.
backend/migrations/20250416000002-update-MTP-table.cjs (1)
4-4: Good addition of EntryPoint versioning!Adding the EP_VERSION column with a default of 'EPV_06' is a good approach for supporting multiple EntryPoint versions. This ensures backward compatibility with existing data.
backend/migrations/20250416000001-update-apiKey-table.cjs (1)
1-11: Good migration implementation with proper safety checksThe migration correctly adds a nullable column for multi-token paymasters V2 support with appropriate IF EXISTS checks in both up and down migrations.
backend/src/paymaster/token.test.ts (1)
3-3: Consistent replacement of PimlicoPaymaster with TokenPaymasterThe imports, class instantiation, and test description are properly updated to reflect the refactoring from Pimlico to Token paymaster.
Also applies to: 8-8, 10-10, 42-42
backend/src/server.ts (6)
12-12: Good replacement of Pimlico with Token-based componentsThe import changes correctly reflect the architectural shift from Pimlico-specific to more generic Token-based paymaster handling.
Also applies to: 22-22
64-65: Constructor updated with additional EntryPoint version parametersThe Paymaster constructor now receives additional parameters for multi-token paymaster gas limits and EntryPoint v8 verification gas limits, which aligns with the PR objective of adding support for EntryPoint version 8.
266-268: Improved null checking for etherspotPaymasterAddressThis change adds a safety check to verify etherspotPaymasterAddress exists before calling checkDeposit, preventing potential null reference errors.
311-313: Consistent null checking for etherspotPaymasterAddressSimilar to the earlier check, this ensures the etherspotPaymasterAddress exists before calling checkDeposit when processing networks from the default config.
165-165: Updated contract ABI referenceThe contract instantiation now correctly uses ERC20PaymasterAbi instead of PimlicoAbi, which aligns with the architectural changes.
105-105: Updated route registrationThe server now registers tokenRoutes instead of pimlicoRoutes, completing the refactoring of the Pimlico-specific components.
backend/src/types/sponsorship-policy-dto.ts (3)
38-38: Support for EPV_08 version added to the enumThe enum is properly extended with the new entry point version
EPV_08, maintaining consistency with the existing pattern.
51-52: Handler for EPV_08 string conversion addedThe
getEPVersionfunction is correctly updated to handle the new EPV_08 version string, maintaining the pattern established for previous versions.
65-66: Handler for EPV_08 enum conversion addedThe
getEPVersionStringfunction is properly updated to convert the EPV_08 enum value to its string representation, following the established pattern.backend/src/routes/token-routes.ts (5)
6-6: Import path updated from Pimlico to TokenThe import path for
PAYMASTER_ADDRESShas been correctly updated to use the Token constants module instead of Pimlico, aligning with the broader refactoring to standardize on Token paymaster terminology.
14-14: Route handler renamed to reflect token-based implementationThe Fastify plugin has been renamed from
pimlicoRoutestotokenRoutes, making the naming more consistent with the actual implementation and other changes in the codebase.
127-128: Added epVersion field to response objectThe response now includes the EP version for each multi-token paymaster record, which is essential for supporting multiple entry point versions. This is a good enhancement that aligns with the database schema changes.
146-146: Updated default export nameThe default export has been correctly updated to match the renamed
tokenRoutesfunction.
116-116:Details
❓ Verification inconclusive
API parameter index changed - potential breaking change
The extraction of
api_keynow usesbody.params[0]instead of the previousbody.params[1]. This change could break existing client code if clients were relying on the previous parameter order.
🏁 Script executed:
#!/bin/bash # Verify if there are any client implementations that might be affected by the parameter order change # Look for code that calls this endpoint and expects parameters in a specific order # First check if there are any direct calls to this endpoint in the codebase rg -A 3 -B 3 "getAllCommonERC20PaymasterAddress" --type tsLength of output: 1567
Confirm backward‐compatibility of
apiKeyextractionI didn’t find any internal client calls to
/getAllCommonERC20PaymasterAddressin the repo, but external consumers relying onbody.params[1]for theapiKeywill break now that it’s read frombody.params[0]. To safeguard against regressions:
- Support both parameter orders (e.g. fall back to
body.params[1]ifbody.params[0]is missing) or switch to named JSON properties.- Update API documentation and example payloads to reflect the new position.
- Bump the API (major) version and notify clients of this breaking change.
backend/src/repository/whitelist-repository.ts (3)
4-4: Added import for EPVersions enumThe EPVersions enum is properly imported to support type checking for the new methods that use EP version.
18-19: Added epVersion to repository create methodThe create method now correctly stores the epVersion from the input DTO, defaulting to null if not provided, aligning with the model changes.
55-66: Added method to query whitelist by API key, EP version, and policy IDThe new method follows the existing pattern for the similar API key and policy ID method, but adds EP version support. The early null return when epVersion is missing prevents unnecessary database queries.
backend/src/paymaster/token.ts (5)
4-5: Updated imports from Pimlico to Token-related modulesThe imports have been correctly updated to use the generic token-related modules instead of Pimlico-specific ones, consistent with the class renaming below.
17-17: Renamed class from PimlicoPaymaster to TokenPaymasterThe class has been renamed to better reflect its generic token handling capabilities rather than being tied to a specific provider.
192-192: Updated return type to TokenPaymasterThe return type of
getERC20Paymasterfunction has been correctly updated to match the renamed class.
201-201: Removed trailing whitespace in owner address stringMinor cleanup of the default owner address string by removing trailing whitespace.
210-210: Updated instantiation to use TokenPaymasterThe function now correctly returns an instance of TokenPaymaster instead of PimlicoPaymaster.
backend/src/repository/api-key-repository.ts (1)
64-71: Surface silent failures by checking the update count
sequelize.update()resolves to[affectedCount]and silently returns[0]when no rows match.
Consider mirroring the behaviour ofdelete()and throw whenaffectedCount === 0so callers don’t mistake “nothing updated” for success.const [affected] = await this.sequelize.models.APIKey.update( { [column]: vpAddresses }, { where: { apiKey } }, ); if (affected === 0) { throw new Error(`APIKey ${apiKey} not found – no rows updated`); } return affected;backend/src/models/api-key.ts (1)
71-75: Verify migrations & indices for the two new columns
VERIFYING_PAYMASTERS_V3andMULTI_TOKEN_PAYMASTERS_V2are now part of the model.
Please ensure that:
- The matching migrations have been applied in every environment before the new code is deployed.
- Any queries / indices that previously referenced only the V and V2 columns are extended accordingly.
This avoids
Unknown columnruntime errors and keeps look-ups performant.Also applies to: 87-91
backend/src/routes/admin-routes.ts (1)
20-23: ConfirmPaymasterconstructor signatureA tenth argument
server.config.EP8_PVGLis now passed.
Double-check that thePaymasterclass has been updated to accept (and use) this parameter; otherwise the service will crash on startup.backend/migrations/2025042200001-seed-data.cjs (1)
4-195: Potential duplicate primary-key rows
TOKEN_ADDRESS '0x50c5725949A6F0c72E6C4a641F24049A917DB0Cb'appears twice (lines ~19 & ~101).
If a uniqueness constraint exists onTOKEN_ADDRESS+CHAIN_ID, the migration will fail on the second insert.Please de-duplicate before shipping.
backend/src/routes/deposit-route.ts (1)
47-66: Boolean query params arrive as strings
const useVp = query['useVp'] ?? false;
When the caller sends/deposit?useVp=true,useVpis the string"true", which is truthy, so the logic works.
However!useVplater becomesfalseas intended only if the param is present and non-empty –"false"will still be truthy.Safer coercion:
-const useVp = query['useVp'] ?? false; +const useVp = String(query['useVp']).toLowerCase() === 'true';backend/src/abi/MultiTokenPaymasterAbiV2.ts (1)
1-878: ABI JSON looks good – just be mindful of bundle sizeNothing jumps out as incorrect: the array is exported as
const, is type-safe (as const), and contains a complete ABI.
A minor point to watch is import cost – this file is ~900 lines and will be loaded into memory any time it is imported. If you only need small fragments at run-time, consider code-splitting or lazy-loading to avoid inflating cold-start time.
* changed Bignumber to hex string as per current change in skandha
Deploying arka with
|
| Latest commit: |
fb1d25e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://13104659.arka-3qg.pages.dev |
| Branch Preview URL: | https://develop.arka-3qg.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
backend/src/types/whitelist-dto.ts (1)
1-7: Properly implemented type-safe EPVersion support!The changes appropriately implement the optional
epVersionfield with theEPVersionsenum type, providing strong type safety as recommended in the previous review. This enhancement aligns well with the PR objective of adding support for entryPoint version 8.backend/src/routes/paymaster-routes.ts (1)
206-208:⚠️ Potential issue
useVpis still parsed as a raw query-string – logic silently bypassed
useVparrives as"true" | "false"(string). The conditionalif (useVp && …)always evaluates totrue(non-empty string) causing unintended mode switches.
The exact problem was raised in an earlier review and is still unresolved.-const useVp = query['useVp'] ?? false; +const useVp = + String(query['useVp']).toLowerCase() === 'true';backend/src/paymaster/index.ts (1)
191-263: 🛠️ Refactor suggestion
signV08duplicates 90 % ofsignV07– extract shared helper
The two methods differ only in ABI reference and theEP*_PVGLconstant. Duplication invites divergence and bugs.Suggested approach (sketch):
-async signV07(...) { return this.signGeneric('v07', ...); } -async signV08(...) { return this.signGeneric('v08', ...); } +private async signGeneric( + version: 'v07' | 'v08', + userOp, validUntil, validAfter, + entryPoint, paymasterAddress, + bundlerRpc, signer, estimate, log +) { + const cfg = version === 'v07' + ? { abi: EtherspotAbiV07, pvgl: this.EP7_PVGL } + : { abi: verifyingPaymasterV3Abi, pvgl: this.EP8_PVGL }; + ... +}Refactor once now; you will thank yourself when EPV-09 arrives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
backend/src/paymaster/index.ts(14 hunks)backend/src/routes/paymaster-routes.ts(15 hunks)backend/src/types/whitelist-dto.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
backend/src/routes/paymaster-routes.ts
[error] 308-308: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
backend/src/paymaster/index.ts
[error] 352-352: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 949-949: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
backend/src/paymaster/index.ts (1)
836-838:this.MTP_VGL_MARKUPis a string – implicit coercion hides unit errors
Adding a plain string to aBigNumberworks only when the string is a base-10 integer, but future edits may prepend0xor leave it empty leading to runtime failures.-userOp.verificationGasLimit = BigNumber.from(response.verificationGasLimit).add(this.MTP_VGL_MARKUP).toHexString(); +userOp.verificationGasLimit = BigNumber + .from(response.verificationGasLimit) + .add(BigNumber.from(this.MTP_VGL_MARKUP)) + .toHexString();Consider normalising constructor arguments to
BigNumberonce and store them as such.
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit
New Features
/deposit/v3,/metadata/v3, and/whitelist/v3.Improvements
Bug Fixes
Documentation
Refactor