feat: Replace ethers with viem #223
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughMigrates backend from ethers.js to viem: removes ethers dependency, replaces BigNumber with bigint, updates constants, paymaster and token modules to viem clients/types, refactors contract interactions, encoding, signing, gas estimation, and fee utilities. Interfaces updated to bigint return types. Package version bumped to 4.2.0. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Paymaster as Paymaster (viem)
participant Pub as PublicClient
participant Acc as PrivateKeyAccount
participant Token as ERC20Paymaster (Contract)
Caller->>Paymaster: getPaymasterAndData(userOp, ...)
Paymaster->>Pub: read gas/oracle/token data
Paymaster->>Token: read.priceMarkup()/previousPrice()/decimals
Paymaster->>Paymaster: encode params (toHex/concat)
Paymaster->>Acc: sign (hash -> sign)
Acc-->>Paymaster: signature (Hex)
Paymaster-->>Caller: paymasterAndData (Hex)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing touches🧪 Generate unit tests
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. Comment |
Deploying arka with
|
| Latest commit: |
ba2136d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://44a09b87.arka-3qg.pages.dev |
| Branch Preview URL: | https://viem-rewrite.arka-3qg.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
backend/src/utils/common.ts (1)
109-115: Minor consistency: setgasPriceequal to the received price.You already do this; ok. Consider also ensuring non-negative and non-zero here (same as above), or returning
nullto let the caller retry another source.backend/src/paymaster/token.ts (2)
108-123: Remove unused ‘deployer’ requirement in validation.
validatePaymasterOptionsrequiresdeployerbut never uses it. Worse,getERC20Paymaster’soptionstype omitsdeployer, so passing any options will always throw. This is a breaking change.Apply:
-async function validatePaymasterOptions( - publicClient: PublicClient, - erc20: string, - options?: ERC20PaymasterBuildOptions -): Promise<Required<Omit<ERC20PaymasterBuildOptions, "deployer">>> { +async function validatePaymasterOptions( + publicClient: PublicClient, + erc20: string, + options?: Omit<ERC20PaymasterBuildOptions, "deployer"> +): Promise<Required<Omit<ERC20PaymasterBuildOptions, "deployer">>> { @@ - if (parsedOptions.deployer === undefined) { - throw new Error("Deployer must be provided") - } + // no deployer needed for validationOptionally, remove
deployer?: PrivateKeyAccountfromERC20PaymasterBuildOptionsentirely if it’s unused.- owner?: string - deployer?: PrivateKeyAccount + owner?: string
204-229: FixvalidatePaymasterOptionsto drop the unnecessarydeployercheckThe current implementation of
validatePaymasterOptions(backend/src/paymaster/token.ts:112–118) always throwsif (parsedOptions.deployer === undefined) { throw new Error("Deployer must be provided") }but
getERC20Paymasternever supplies adeployer(itsoptionstype omits that field), causing every custom‐options invocation to fail.Please:
- Remove (or conditionally guard) the
parsedOptions.deployervalidation invalidatePaymasterOptions.- Verify that the returned object still satisfies the requirements of
calculateERC20PaymasterAddress(i.e., it providesentrypoint,nativeAssetOracle,tokenAddress,tokenOracle, andowner).- Once that’s updated, run a full type‐check in the backend folder to ensure no signature‐breakage:
cd backend && npm run check:types- Finally, test that calling
now returns a validawait getERC20Paymaster(publicClient, erc20, entryPoint, { owner, entrypoint?, nativeAssetOracle?, tokenAddress?, tokenOracle? })TokenPaymasterwithout throwing.backend/src/paymaster/index.ts (3)
540-587: Fix the error message inconsistency.Line 566 references
data[0].reasonbut should referencedata[1].reasonfor the symbol fetch error.- throw new Error('Failed to get symbol for token ' + data[0].reason); + throw new Error('Failed to get symbol for token ' + data[1].reason);
984-1034: Fix the token balance comparison logic.Line 1009 uses
>=but should use>for the token amount comparison, as having exactly the required amount should be sufficient.- if (tokenAmountRequired >= tokenBalance) + if (tokenAmountRequired > tokenBalance)Also, Line 1016 correctly adds a buffer to verification gas limit, but the conversion should be more explicit:
- userOp.verificationGasLimit = (BigInt(response.verificationGasLimit) + 100000n).toString(); + userOp.verificationGasLimit = toHex(BigInt(response.verificationGasLimit) + 100000n);
1036-1104: Fix the token balance comparison logic.Similar to the previous method, Line 1068 should use
>instead of>=for the token balance comparison.- if (tokenAmountRequired >= tokenBalance) + if (tokenAmountRequired > tokenBalance)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
backend/package.json(1 hunks)backend/src/constants/MultitokenPaymaster.ts(1 hunks)backend/src/paymaster/index.ts(52 hunks)backend/src/paymaster/token.ts(9 hunks)backend/src/utils/common.ts(3 hunks)backend/src/utils/interface.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/paymaster/token.ts (1)
backend/src/constants/Token.ts (3)
TOKEN_ADDRESS(1-39)ORACLE_ADDRESS(52-97)bytecode(215-215)
backend/src/paymaster/index.ts (3)
backend/src/constants/MultitokenPaymaster.ts (2)
TokenDecimalsAndSymbol(10-207)OracleDecimals(209-363)backend/src/paymaster/token.ts (1)
TokenPaymaster(31-106)backend/src/utils/common.ts (1)
getGasFee(57-62)
🔇 Additional comments (29)
backend/src/utils/interface.ts (1)
17-19: No direct JSON serialization of raw BigInt fee fields detected
- We searched all
JSON.stringifycalls andlogger.info/logger.error/logger.debuginvocations for references tomaxFeePerGas,maxPriorityFeePerGas, orgasPrice—none were found.- Fastify response paths (
.send() do not include these properties directly; fee estimates are sent as hex‐string values (seefeeEstimates.maxFeePerGasand.maxPriorityFeePerGasinpaymaster/index.ts).- Internally, calls to
walletClient.sendTransactionconsume BigInt values directly (not serialized to JSON), and all HTTP/RPC payloads convert BigInt fee fields to strings or hex beforehand.Current code does not attempt to
JSON.stringifyraw BigInt values for gas fees. You can safely resolve this concern, but please ensure any future additions continue converting BigInt fee fields to strings or hex before serialization or logging.backend/src/paymaster/token.ts (4)
83-90: LGTM: paymasterAndData concatenation uses correct 20B + 32B layout.
concat([address, toHex(tokenAmount, { size: 32 })])matches expected encoding.
99-105: LGTM: explicit path for externally computed tokenAmount.API aligns with the new bigint usage.
124-138: Runtime validations viagetBytecodeare good.Checks for oracle/token deployment existence are correct and Viem-idiomatic.
167-183: LGTM: constructor encoding viaencodeDeployData.Return type and args ordering look consistent with the ABI.
backend/src/paymaster/index.ts (24)
2-28: LGTM! Clean migration to Viem imports.The imports have been properly updated from ethers to viem, with all necessary types and utilities included. The structure is well-organized and comprehensive.
94-100: LGTM! Correct type migration from BigNumber to bigint.The property types have been correctly updated to use native
bigintinstead of ethers BigNumber, maintaining consistency with the viem migration.
110-123: LGTM! Proper initialization of bigint properties.The constructor correctly uses
parseUnitsfor the fee markup andBigInt()for the paymaster verification gas limits, properly migrating from ethers to viem patterns.
125-127: LGTM! Correct implementation of bit packing.The
packUintmethod properly uses viem'stoHexwith bit shifting operations using native bigint arithmetic, which is correct for packing two 128-bit values.
129-135: LGTM! Proper paymaster data packing.The method correctly uses viem's
concatand type casting toHexfor proper data concatenation and formatting.
137-156: LGTM! Correct migration of signing logic.The method properly:
- Uses viem's contract read interface
- Converts hash to bytes using
hexToBytes- Uses the new
PrivateKeyAccountsigning interface- Employs viem's ABI parameter encoding
158-230: LGTM! Comprehensive V07 signing implementation.The method correctly:
- Creates public client with viem
- Uses viem's contract interface with proper type casting
- Handles initCode concatenation properly
- Uses
toHexfor gas limit conversions- Makes RPC requests through the public client
- Returns properly formatted hex values
The error handling and gas estimation logic are also correctly implemented.
232-304: LGTM! Correct V08 signing implementation.Similar to V07, this method properly implements viem patterns with:
- Correct client initialization
- Proper contract interface usage
- Appropriate gas limit handling
- Consistent error handling
The implementation is consistent with the V07 version while using the appropriate V3 ABI.
306-363: LGTM! Proper V06 implementation.The V06 signing method correctly:
- Uses viem public client creation
- Implements proper contract interface
- Makes RPC requests through the client
- Maintains consistent error handling patterns
365-395: LGTM! Correct multi-token paymaster V07 implementation.The method properly:
- Uses viem's contract read interface
- Employs proper ABI parameter encoding with
parseAbiParameters- Handles signing with the new account interface
- Uses appropriate data concatenation
397-442: LGTM! Proper multi-token paymaster implementation.The method correctly:
- Uses viem's
keccak256for hashing- Employs proper ABI parameter encoding
- Handles complex parameter arrays correctly
- Uses proper signing and data concatenation
444-473: LGTM! Clean utility method implementations.The token decimal/symbol getter methods and gas estimation utility properly use viem's:
- Contract creation with proper type casting
- Contract read interface
- Public client request method
475-538: LGTM! Proper oracle interaction implementation.The methods correctly:
- Use viem contract creation and read interfaces
- Handle promise-based operations properly
- Maintain consistent caching mechanisms
- Use proper error handling patterns
589-646: LGTM! Correct Chainlink oracle implementation.The method properly:
- Uses viem contract interfaces
- Handles price calculations with
formatUnitsandparseUnits- Implements proper error handling for all promise settlements
- Uses correct bigint arithmetic operations
648-822: LGTM! Comprehensive multi-token quote implementation.The large method correctly:
- Creates viem public clients
- Uses proper contract interfaces with type casting
- Handles complex gas estimation logic
- Implements proper price fetching from various oracles
- Uses
toHexfor proper value conversion- Maintains consistent error handling patterns
The implementation is thorough and correctly migrated to viem.
824-886: LGTM! Proper multi-token paymaster signing.The method correctly:
- Creates viem clients properly
- Uses contract interfaces with proper type casting
- Handles oracle price fetching correctly
- Implements gas estimation with proper conversions using
toHexandBigInt- Maintains consistent error handling
888-982: LGTM! Comprehensive V07 multi-token implementation.The method properly:
- Handles initCode concatenation with viem's
concat- Uses proper gas limit conversions with
toHex- Implements correct contract interactions
- Handles complex user operation packing
- Returns properly formatted hex values
1117-1173: LGTM! Proper whitelist management implementation.The method correctly:
- Creates both public and wallet clients with viem
- Uses proper account creation with
privateKeyToAccount- Employs
encodeFunctionDatafor transaction data- Handles fee data properly with bigint arithmetic
- Uses proper transaction request typing with
TransactionRequest
1175-1231: LGTM! Consistent whitelist removal implementation.Similar to the whitelist addition method, this properly implements viem patterns for:
- Client creation
- Account handling
- Function encoding
- Transaction submission
- Fee handling
1233-1243: LGTM! Simple whitelist check implementation.The method correctly uses viem's contract read interface for checking whitelist status.
1245-1302: LGTM! Proper deposit implementation.The method correctly:
- Uses viem clients and account creation
- Handles balance checks with proper comparisons
- Uses
parseEtherfor amount conversion- Implements proper function encoding
- Handles transaction submission with proper typing
1304-1359: LGTM! Comprehensive paymaster deployment.The method properly:
- Creates viem clients correctly
- Handles different bytecode versions appropriately
- Uses
deployContractmethod properly- Implements consistent fee handling patterns
- Returns appropriate transaction information
1361-1415: LGTM! Proper stake addition implementation.The method correctly:
- Uses viem contract interfaces
- Implements
writeContractmethod properly- Handles value conversion with
parseEther- Maintains consistent fee handling and transaction patterns
1417-1477: LGTM! Proper Coingecko integration.The methods correctly:
- Use
formatUnitsandparseUnitsfor price calculations- Handle address normalization with
getAddress- Implement proper caching mechanisms
- Use consistent error handling patterns
The price calculation logic and caching implementation are correctly migrated to viem.
| { | ||
| "name": "arka", | ||
| "version": "4.1.2", | ||
| "version": "4.2.0", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Version bump is fine; update Node engine for Viem v2 compatibility.
Viem v2 targets modern Node runtimes. Keeping "node": ">=16.0.0" is risky and may break at runtime. Recommend raising the minimum to 18+.
Apply this change outside the selected range:
- "engines": {
- "node": ">=16.0.0"
- },
+ "engines": {
+ "node": ">=18.0.0"
+ },📝 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.
| "version": "4.2.0", | |
| "version": "4.2.0", | |
| - "engines": { | |
| - "node": ">=16.0.0" | |
| "engines": { | |
| "node": ">=18.0.0" | |
| }, |
🤖 Prompt for AI Agents
In backend/package.json around line 3, the current "version" bump left the
package.json engines requirement at an older Node minimum; update the "engines"
field to require Node 18 or newer (e.g. "node": ">=18.0.0") to ensure Viem v2
compatibility — if the engines field is absent, add it; also update any
CI/workflow files referencing the Node version to match the new minimum.
| import { toHex } from 'viem'; | ||
|
|
||
| export const UnaccountedCost = BigNumber.from("45000").toHexString(); | ||
| export const UnaccountedCost = toHex(45000n); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: BigNumber → viem toHex migration is correct.
toHex(45000n) preserves the previous hex form (0xafc8). No functional change. Consider naming this UnaccountedCostHex for clarity, but optional.
🤖 Prompt for AI Agents
In backend/src/constants/MultitokenPaymaster.ts around lines 1 to 3, the
exported constant UnaccountedCost is correctly migrated to viem's toHex(45000n)
but the name is ambiguous; rename it to UnaccountedCostHex (or add a new
UnaccountedCostHex alias and keep the old name deprecated) and update all
imports/usages across the codebase to use the new identifier (or keep the old
export forwarding to the new one) to make the hex nature explicit.
backend/src/paymaster/token.ts
Outdated
| private contract: any; | ||
| tokenAddress: Promise<string>; | ||
| paymasterAddress: string; | ||
|
|
||
| constructor(address: string, provider: providers.Provider) { | ||
| constructor(address: string, publicClient: PublicClient) { | ||
| this.paymasterAddress = address; | ||
| this.contract = new Contract(address, abi, provider) | ||
| this.tokenAddress = this.contract.token(); | ||
| this.contract = getContract({ address: address as Address, abi, publicClient }); | ||
| this.tokenAddress = this.contract.read.token(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid any for the contract; use Viem’s contract type.
Typing improves safety and autocompletion for read.* methods.
Apply:
-import {
+import {
createPublicClient,
http,
parseUnits,
toHex,
concat,
keccak256,
getContract,
getCreate2Address,
encodeDeployData,
Address,
Hex,
PublicClient,
- PrivateKeyAccount
+ PrivateKeyAccount
} from 'viem';
+import type { GetContractReturnType } from 'viem'
export class TokenPaymaster {
- private contract: any;
+ private contract: GetContractReturnType<typeof abi, PublicClient>;📝 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.
| private contract: any; | |
| tokenAddress: Promise<string>; | |
| paymasterAddress: string; | |
| constructor(address: string, provider: providers.Provider) { | |
| constructor(address: string, publicClient: PublicClient) { | |
| this.paymasterAddress = address; | |
| this.contract = new Contract(address, abi, provider) | |
| this.tokenAddress = this.contract.token(); | |
| this.contract = getContract({ address: address as Address, abi, publicClient }); | |
| this.tokenAddress = this.contract.read.token(); | |
| } | |
| import { | |
| createPublicClient, | |
| http, | |
| parseUnits, | |
| toHex, | |
| concat, | |
| keccak256, | |
| getContract, | |
| getCreate2Address, | |
| encodeDeployData, | |
| Address, | |
| Hex, | |
| PublicClient, | |
| PrivateKeyAccount | |
| } from 'viem'; | |
| import type { GetContractReturnType } from 'viem'; | |
| export class TokenPaymaster { | |
| private contract: GetContractReturnType<typeof abi, PublicClient>; | |
| tokenAddress: Promise<string>; | |
| paymasterAddress: string; | |
| constructor(address: string, publicClient: PublicClient) { | |
| this.paymasterAddress = address; | |
| this.contract = getContract({ address: address as Address, abi, publicClient }); | |
| this.tokenAddress = this.contract.read.token(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In backend/src/paymaster/token.ts around lines 32 to 40, the contract is typed
as any which loses type-safety and autocompletion for read.* methods; replace
any with Viem's Contract type (import the Contract type from 'viem') and
parameterize it with the ABI (e.g. Contract<typeof abi>) so getContract's return
is correctly typed. Also tighten tokenAddress from Promise<string> to
Promise<Address> (import Address from viem types) or the specific return type
for the token read call. Update the property declarations and constructor
assignment accordingly so contract and tokenAddress have proper Viem types
instead of any.
backend/src/paymaster/token.ts
Outdated
| async calculateTokenAmount(userOp: NotPromise<UserOperationStruct>): Promise<bigint> { | ||
| const priceMarkup = await this.contract.read.priceMarkup() | ||
| const cachedPrice = await this.contract.read.previousPrice() | ||
| const tokenDecimals = await this.contract.read.tokenDecimals(); | ||
| if (cachedPrice === 0n) { | ||
| throw new Error("ERC20Paymaster: no previous price set") | ||
| } | ||
|
|
||
| const requiredPreFund = BigNumber.from(userOp.preVerificationGas) | ||
| .add(BigNumber.from(userOp.verificationGasLimit).mul(3)) // 3 is for buffer when using paymaster | ||
| .add(BigNumber.from(userOp.callGasLimit)) | ||
| .mul(BigNumber.from(userOp.maxFeePerGas).mul(2)) | ||
| const requiredPreFund = BigInt(userOp.preVerificationGas) | ||
| + (BigInt(userOp.verificationGasLimit) * 3n) // 3 is for buffer when using paymaster | ||
| + BigInt(userOp.callGasLimit) | ||
| * (BigInt(userOp.maxFeePerGas) * 2n) | ||
| let tokenAmount = requiredPreFund | ||
| .add(BigNumber.from(userOp.maxFeePerGas).mul(40000)) // 40000 is the REFUND_POSTOP_COST constant | ||
| .mul(priceMarkup) | ||
| .mul(cachedPrice) | ||
| .div(1e6) // 1e6 is the priceDenominator constant | ||
| + (BigInt(userOp.maxFeePerGas) * 40000n) // 40000 is the REFUND_POSTOP_COST constant | ||
| * BigInt(priceMarkup) | ||
| * BigInt(cachedPrice) | ||
| / 1000000n // 1e6 is the priceDenominator constant | ||
|
|
There was a problem hiding this comment.
Critical: unit-mixing and missing parentheses in gas cost math.
Current code mixes raw gas units (preVerification/verification) with wei (callGasLimit * maxFeePerGas) and misses parentheses, producing incorrect magnitudes. Compute total gas units first, then multiply by a fee, then apply headroom.
Apply:
- const requiredPreFund = BigInt(userOp.preVerificationGas)
- + (BigInt(userOp.verificationGasLimit) * 3n) // 3 is for buffer when using paymaster
- + BigInt(userOp.callGasLimit)
- * (BigInt(userOp.maxFeePerGas) * 2n)
+ const baseFee = BigInt(userOp.maxFeePerGas)
+ const requiredGasUnits =
+ BigInt(userOp.preVerificationGas) +
+ (BigInt(userOp.verificationGasLimit) * 3n) + // buffer
+ BigInt(userOp.callGasLimit)
+ // Headroom x2 on fee (retain previous intent)
+ const requiredPreFund = requiredGasUnits * (baseFee * 2n)
- let tokenAmount = requiredPreFund
- + (BigInt(userOp.maxFeePerGas) * 40000n) // 40000 is the REFUND_POSTOP_COST constant
+ let tokenAmount = requiredPreFund
+ + (baseFee * 40000n) // REFUND_POSTOP_COST
* BigInt(priceMarkup)
* BigInt(cachedPrice)
- / 1000000n // 1e6 is the priceDenominator constant
+ / 1_000_000n // 1e6 priceDenominator📝 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.
| async calculateTokenAmount(userOp: NotPromise<UserOperationStruct>): Promise<bigint> { | |
| const priceMarkup = await this.contract.read.priceMarkup() | |
| const cachedPrice = await this.contract.read.previousPrice() | |
| const tokenDecimals = await this.contract.read.tokenDecimals(); | |
| if (cachedPrice === 0n) { | |
| throw new Error("ERC20Paymaster: no previous price set") | |
| } | |
| const requiredPreFund = BigNumber.from(userOp.preVerificationGas) | |
| .add(BigNumber.from(userOp.verificationGasLimit).mul(3)) // 3 is for buffer when using paymaster | |
| .add(BigNumber.from(userOp.callGasLimit)) | |
| .mul(BigNumber.from(userOp.maxFeePerGas).mul(2)) | |
| const requiredPreFund = BigInt(userOp.preVerificationGas) | |
| + (BigInt(userOp.verificationGasLimit) * 3n) // 3 is for buffer when using paymaster | |
| + BigInt(userOp.callGasLimit) | |
| * (BigInt(userOp.maxFeePerGas) * 2n) | |
| let tokenAmount = requiredPreFund | |
| .add(BigNumber.from(userOp.maxFeePerGas).mul(40000)) // 40000 is the REFUND_POSTOP_COST constant | |
| .mul(priceMarkup) | |
| .mul(cachedPrice) | |
| .div(1e6) // 1e6 is the priceDenominator constant | |
| + (BigInt(userOp.maxFeePerGas) * 40000n) // 40000 is the REFUND_POSTOP_COST constant | |
| * BigInt(priceMarkup) | |
| * BigInt(cachedPrice) | |
| / 1000000n // 1e6 is the priceDenominator constant | |
| async calculateTokenAmount(userOp: NotPromise<UserOperationStruct>): Promise<bigint> { | |
| const priceMarkup = await this.contract.read.priceMarkup() | |
| const cachedPrice = await this.contract.read.previousPrice() | |
| const tokenDecimals = await this.contract.read.tokenDecimals(); | |
| if (cachedPrice === 0n) { | |
| throw new Error("ERC20Paymaster: no previous price set") | |
| } | |
| // Compute total gas units, then multiply by fee (with headroom) | |
| const baseFee = BigInt(userOp.maxFeePerGas) | |
| const requiredGasUnits = | |
| BigInt(userOp.preVerificationGas) + | |
| (BigInt(userOp.verificationGasLimit) * 3n) + // buffer for paymaster | |
| BigInt(userOp.callGasLimit) | |
| // Apply 2× headroom on the fee | |
| const requiredPreFund = requiredGasUnits * (baseFee * 2n) | |
| let tokenAmount = requiredPreFund | |
| (baseFee * 40000n) // REFUND_POSTOP_COST | |
| * BigInt(priceMarkup) | |
| * BigInt(cachedPrice) | |
| / 1_000_000n // priceDenominator (1e6) | |
| // …rest of method… | |
| } |
🤖 Prompt for AI Agents
In backend/src/paymaster/token.ts around lines 49 to 66, the gas/fee math mixes
gas units and wei and is missing parentheses which yields wrong magnitudes; fix
by first summing gas units into a single BigInt totalGasUnits
(preVerificationGas + verificationGasLimit*3 + callGasLimit), then compute
totalFeeWei = totalGasUnits * BigInt(userOp.maxFeePerGas), add the fixed
REFUND_POSTOP_COST in wei (BigInt(userOp.maxFeePerGas) * 40000n) to that fee if
intended as a per-wei refund cost, next apply markup and price:
amountInTokenUnits = totalFeeWei * BigInt(priceMarkup) * BigInt(cachedPrice) /
1000000n, ensure all operands are BigInt and include any conversion using
tokenDecimals if you need to scale the token amount to token's smallest unit;
replace the current mixed-expression with these clear intermediate BigInt
variables and explicit parentheses.
backend/src/paymaster/token.ts
Outdated
| if (parseUnits('1', 6) === tokenDecimals) { | ||
| tokenAmount = tokenAmount / (10n ** 18n); | ||
| } |
There was a problem hiding this comment.
Bug: decimals check compares 1e6 to token decimals and divides by 1e18.
parseUnits('1', 6)returns 1e6 wei-like units, not the number of decimals.tokenDecimalsfrom ABI is typicallyuint8(6 or 18), often returned asnumberby Viem.- Dividing by
10^18for 6-decimal tokens is off by 1e12. The typical scale to go from 18 → 6 is10^(18-6)=10^12.
Apply:
- if (parseUnits('1', 6) === tokenDecimals) {
- tokenAmount = tokenAmount / (10n ** 18n);
- }
+ const tokenDec = typeof tokenDecimals === 'bigint' ? Number(tokenDecimals) : tokenDecimals
+ if (tokenDec === 6) {
+ tokenAmount = tokenAmount / (10n ** 12n)
+ } // 18-decimal tokens need no adjustment📝 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.
| if (parseUnits('1', 6) === tokenDecimals) { | |
| tokenAmount = tokenAmount / (10n ** 18n); | |
| } | |
| const tokenDec = typeof tokenDecimals === 'bigint' ? Number(tokenDecimals) : tokenDecimals | |
| if (tokenDec === 6) { | |
| tokenAmount = tokenAmount / (10n ** 12n) | |
| } // 18-decimal tokens need no adjustment |
🤖 Prompt for AI Agents
In backend/src/paymaster/token.ts around lines 71 to 73, the decimals check and
scaling are incorrect: replace the parseUnits comparison with a numeric check of
tokenDecimals (ensure tokenDecimals is treated as a number or converted to one),
then compute the correct BigInt scaling factor instead of hardcoding 10^18; for
example, if tokenDecimals < 18 divide tokenAmount by 10^(18 - tokenDecimals) (as
a BigInt), if tokenDecimals > 18 multiply by 10^(tokenDecimals - 18), and if
equal leave tokenAmount unchanged; make sure to use BigInt arithmetic for the
divisor/multiplier and handle possible non-number tokenDecimals safely (convert
or throw).
backend/src/paymaster/token.ts
Outdated
| const address = getCreate2Address({ | ||
| from: "0x4e59b44847b379578588920cA78FbF26c0B4956C", | ||
| salt: "0x0000000000000000000000000000000000000000000000000000000000000000", | ||
| bytecodeHash: keccak256(getPaymasterConstructor(options) as Hex) | ||
| }) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Deterministic address calc: confirm factory/salt matches actual deployment path.
Hardcoding the canonical CREATE2 factory and zero salt is fine if deployment uses the same. If your deployment tool uses a different salt or factory, this will diverge.
Would you like a helper to compute the salt from constructor args (and owner) to avoid collisions across networks?
🤖 Prompt for AI Agents
In backend/src/paymaster/token.ts around lines 188 to 193, the CREATE2 address
is computed with a hardcoded factory address and a zero salt which may not match
the actual deployment path; verify the deployed factory address and salt
generation process and update the code to derive the salt deterministically from
the paymaster constructor args (including owner/address and any nonce) and/or
use the actual factory address used by your deployer. Implement a small helper
that serializes constructor args, keccak256-hashes them to produce the salt, and
pass that salt into getCreate2Address (or replace the hardcoded factory with a
configured value) so the computed address always matches deployment across
networks.
backend/src/utils/common.ts
Outdated
| maxFeePerGas: BigInt(data.maxFeePerGas), | ||
| maxPriorityFeePerGas: BigInt(data.maxPriorityFeePerGas), | ||
| gasPrice: BigInt(data.maxFeePerGas) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Skandha gas data: safe, but validate payload format.
Casting data.maxFeePerGas and data.maxPriorityFeePerGas with BigInt(...) is fine for both hex and decimal strings. If these can be missing or zero, consider guarding against <= 0n and returning null to avoid downstream invalid fee configurations.
Apply this small guard within the selected area:
- return {
- maxFeePerGas: BigInt(data.maxFeePerGas),
- maxPriorityFeePerGas: BigInt(data.maxPriorityFeePerGas),
- gasPrice: BigInt(data.maxFeePerGas)
- }
+ const maxFeePerGas = BigInt(data.maxFeePerGas)
+ const maxPriorityFeePerGas = BigInt(data.maxPriorityFeePerGas)
+ if (maxFeePerGas <= 0n || maxPriorityFeePerGas < 0n) return null
+ return {
+ maxFeePerGas,
+ maxPriorityFeePerGas,
+ gasPrice: maxFeePerGas
+ }📝 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.
| maxFeePerGas: BigInt(data.maxFeePerGas), | |
| maxPriorityFeePerGas: BigInt(data.maxPriorityFeePerGas), | |
| gasPrice: BigInt(data.maxFeePerGas) | |
| } | |
| const maxFeePerGas = BigInt(data.maxFeePerGas) | |
| const maxPriorityFeePerGas = BigInt(data.maxPriorityFeePerGas) | |
| if (maxFeePerGas <= 0n || maxPriorityFeePerGas < 0n) return null | |
| return { | |
| maxFeePerGas, | |
| maxPriorityFeePerGas, | |
| gasPrice: maxFeePerGas | |
| } |
🤖 Prompt for AI Agents
In backend/src/utils/common.ts around lines 77 to 80, the code casts gas fields
to BigInt but does not validate them; add a guard that parses/casts maxFeePerGas
and maxPriorityFeePerGas to BigInt, then if either is missing or <= 0n return
null (to avoid creating invalid fee configurations), otherwise proceed to build
and return the object using those BigInt values (and ensure gasPrice is set
intentionally — e.g. from maxFeePerGas or a dedicated field).
| if(log) log.info('setting maxFeePerGas and maxPriorityFeePerGas as received') | ||
| const maxFeePerGas = ethers.utils.parseUnits(response.result.suggestBaseFee, 'gwei') | ||
| const fastGasPrice = ethers.utils.parseUnits(response.result.FastGasPrice, 'gwei') | ||
| const maxFeePerGas = parseUnits(response.result.suggestBaseFee, 9) | ||
| const fastGasPrice = parseUnits(response.result.FastGasPrice, 9) | ||
| return { | ||
| maxPriorityFeePerGas: fastGasPrice.sub(maxFeePerGas), | ||
| maxPriorityFeePerGas: fastGasPrice - maxFeePerGas, | ||
| maxFeePerGas, | ||
| gasPrice: maxFeePerGas, | ||
| } |
There was a problem hiding this comment.
EIP-1559 math bug: maxFeePerGas is set to base fee; gasPrice mirrors base fee.
In the Etherscan-JSON-RPC branch you set:
maxFeePerGas = suggestBaseFeemaxPriorityFeePerGas = FastGasPrice - suggestBaseFeegasPrice = maxFeePerGas(i.e., base fee)
This underprices type-2 txs and makes gasPrice inconsistent with the “fast” suggestion. Prefer:
baseFee = suggestBaseFeefast = FastGasPricepriority = max(fast - baseFee, 1 gwei)maxFeePerGas = fastgasPrice = fast(fallback for legacy/type-0)
Apply this fix:
- const maxFeePerGas = parseUnits(response.result.suggestBaseFee, 9)
- const fastGasPrice = parseUnits(response.result.FastGasPrice, 9)
- return {
- maxPriorityFeePerGas: fastGasPrice - maxFeePerGas,
- maxFeePerGas,
- gasPrice: maxFeePerGas,
- }
+ const baseFee = parseUnits(response.result.suggestBaseFee, 9)
+ const fastGasPrice = parseUnits(response.result.FastGasPrice, 9)
+ const priority = fastGasPrice > baseFee ? (fastGasPrice - baseFee) : 1_000_000_000n // 1 gwei
+ return {
+ maxPriorityFeePerGas: priority,
+ maxFeePerGas: fastGasPrice,
+ gasPrice: fastGasPrice,
+ }📝 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.
| if(log) log.info('setting maxFeePerGas and maxPriorityFeePerGas as received') | |
| const maxFeePerGas = ethers.utils.parseUnits(response.result.suggestBaseFee, 'gwei') | |
| const fastGasPrice = ethers.utils.parseUnits(response.result.FastGasPrice, 'gwei') | |
| const maxFeePerGas = parseUnits(response.result.suggestBaseFee, 9) | |
| const fastGasPrice = parseUnits(response.result.FastGasPrice, 9) | |
| return { | |
| maxPriorityFeePerGas: fastGasPrice.sub(maxFeePerGas), | |
| maxPriorityFeePerGas: fastGasPrice - maxFeePerGas, | |
| maxFeePerGas, | |
| gasPrice: maxFeePerGas, | |
| } | |
| if (log) log.info('setting maxFeePerGas and maxPriorityFeePerGas as received') | |
| const baseFee = parseUnits(response.result.suggestBaseFee, 9) | |
| const fastGasPrice = parseUnits(response.result.FastGasPrice, 9) | |
| const priority = | |
| fastGasPrice > baseFee | |
| ? fastGasPrice - baseFee | |
| : 1_000_000_000n // 1 gwei fallback | |
| return { | |
| maxPriorityFeePerGas: priority, | |
| maxFeePerGas: fastGasPrice, | |
| gasPrice: fastGasPrice, | |
| } |
🤖 Prompt for AI Agents
In backend/src/utils/common.ts around lines 99 to 106, the code treats
suggestBaseFee as maxFeePerGas and sets gasPrice to that base fee, causing
underpriced EIP-1559 txs and inconsistent gasPrice. Change names and assignments
so baseFee = parseUnits(response.result.suggestBaseFee, 9), fast =
parseUnits(response.result.FastGasPrice, 9), priority = max(fast - baseFee, 1
gwei), maxFeePerGas = fast, and gasPrice = fast (used as legacy/type-0
fallback); ensure priority uses a floor of 1 gwei and all values are BigNumber
arithmetic.
vignesha22
dismissed
their stale review
I did the necessary changes again which makes this review invalid
| if (!userOp.signature) userOp.signature = '0x'; | ||
| if (userOp.factory && userOp.factoryData) userOp.initCode = hexConcat([userOp.factory, userOp.factoryData ?? '']) | ||
| if (userOp.factory && userOp.factoryData) userOp.initCode = concat([userOp.factory as Hex, userOp.factoryData ?? '0x']) | ||
| if (!userOp.initCode) userOp.initCode = "0x"; |
There was a problem hiding this comment.
why is there a field called initCode here ?
There was a problem hiding this comment.
since on permission less the userOp when the initCode is 0x it ignores and sends the rest and on signing to maintain the structure I have included 0x if initCode or factoryData and factory isn't available
| Buffer.from(sourceDigest, 'base64'), | ||
| checkDigest | ||
| Uint8Array.from(Buffer.from(sourceDigest, 'base64')), | ||
| Uint8Array.from(checkDigest) |
There was a problem hiding this comment.
has this been tested ? @vignesha22 this will have affect on rpc-gw and backoffice also.
| Buffer.from(sourceDigest, 'base64'), | ||
| checkDigest | ||
| Uint8Array.from(Buffer.from(sourceDigest, 'base64')), | ||
| Uint8Array.from(checkDigest) |
backend/src/server.ts
Outdated
| const coingeckoId = coingeckoIds[chain][customChainlinkDeployments.indexOf(deployedPaymaster)] | ||
| const response: any = await (await fetch(`${configData.coingeckoApiUrl}${coingeckoId}`)).json(); | ||
| const price = ethers.utils.parseUnits(response[coingeckoId].usd.toString(), 8); | ||
| const price = parseUnits(response[coingeckoId].usd.toString(), 8); |
There was a problem hiding this comment.
Instead of hardcoding can we declare this as a constant?
Replaced ethers library with viem.
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit