ft: adding endpoints for withdrawing stake and deposit#234
ft: adding endpoints for withdrawing stake and deposit#234nikhilkumar1612 merged 2 commits intomasterfrom
Conversation
WalkthroughAdded withdrawal functionality: three new error messages; two Paymaster methods (withdrawDeposit, withdrawStake) that construct and submit transactions with gas fee logic and type selection; new admin and deposit HTTP routes that validate requests, resolve credentials, and invoke the Paymaster withdraw methods. Changes
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler as Route Handler
participant Paymaster
participant FeeResolver as Fee Resolver
participant Blockchain
Client->>RouteHandler: POST /withdrawDeposit (address, amount, chainId, apiKey)
RouteHandler->>RouteHandler: Validate params & epVersion, isAddress(address)
RouteHandler->>RouteHandler: Resolve credentials (AWS Secrets or unsafeMode)
RouteHandler->>RouteHandler: Select paymaster address (by epVersion & chainId)
RouteHandler->>Paymaster: withdrawDeposit(address, amount, paymasterAddr, bundlerRpc, key)
Paymaster->>FeeResolver: getGasFee() or publicClient.getGasPrice()
FeeResolver-->>Paymaster: gas params
Paymaster->>Paymaster: Choose tx type (legacy vs EIP-1559)
Paymaster->>Blockchain: submitTx(withdraw call, tx params)
Blockchain-->>Paymaster: txHash / receipt
Paymaster-->>RouteHandler: { success, txHash }
RouteHandler-->>Client: 200 OK { code, data: { txHash } }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: |
0f3d1d0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e3246a78.arka-3qg.pages.dev |
| Branch Preview URL: | https://ft-withdraw.arka-3qg.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/paymaster/index.ts`:
- Around line 1478-1498: The gas-fee check for deciding legacy vs EIP-1559 is
missing the zero-check and can produce EIP-1559 txns with maxFeePerGas === 0n;
update the conditional that decides which branch to use (the if guarding the
walletClient.writeContract call for withdrawStake) to include "||
feeData.maxFeePerGas === BigInt(0)" (same pattern used in withdrawDeposit,
deposit, deployVp) so that zero maxFeePerGas falls back to legacy; apply the
identical fix to the addStake branch as well so both addStake and withdrawStake
use the same guarded condition involving feeData.maxFeePerGas === BigInt(0) and
skipType2Txns.
- Around line 1272-1276: Update withdrawDeposit to accept an isEpv06: boolean
parameter and pick the ABI and function name based on it: use EtherspotAbiV06
and function name 'withdrawFunds' when isEpv06 is true, otherwise use
EtherspotAbiV07 and 'withdrawTo' for V07+; then call encodeFunctionData with
parseAbi([...]) using the chosen ABI and functionName (refer to withdrawDeposit,
EtherspotAbiV06, EtherspotAbiV07, withdrawFunds, withdrawTo, and
encodeFunctionData) so V06 chains call the correct function signature.
In `@backend/src/routes/admin-routes.ts`:
- Around line 612-713: Extract the duplicated credential and network resolution
logic from withdrawStake (and the similar blocks in addStake,
deployVerifyingPaymaster, deposit routes) into a shared helper like
resolveCredentialsAndNetwork that accepts (apiKey, chainId, epVersion,
apiKeyEntity, unsafeMode, client, prefixSecretId, server.config,
supportedEntrypoints) and returns { privateKey, bundlerApiKey,
supportedNetworks, networkConfig, bundlerUrl }; inside the helper perform the
SecretsManager GetSecretValueCommand flow and unsafeMode decode using
decode(apiKeyEntity.privateKey, server.config.HMAC_SECRET), set bundlerApiKey
from secrets or apiKeyEntity.bundlerApiKey, call getNetworkConfig(chainId,
supportedNetworks, supportedEPs) to build networkConfig and compute bundlerUrl
(append ?api-key= when bundler includes etherspot.io), throw or return errors
consistent with existing ErrorMessage values, then replace the duplicated blocks
in withdrawStake (and the other endpoints) with a call to this helper and use
its returned values (privateKey, bundlerUrl, etc.) when calling
paymaster.withdrawStake.
In `@backend/src/routes/deposit-route.ts`:
- Around line 157-166: The validation block around withdrawAmount (used later
with parseEther) currently allows negative numbers; update the check in the same
block that returns ReturnCode.FAILURE / ErrorMessage.INVALID_DATA to also reject
non-positive values by parsing withdrawAmount to a numeric value (e.g.,
Number(withdrawAmount) or parseFloat) and ensuring it is > 0 and finite before
proceeding; ensure this check occurs before any call to parseEther so parseEther
never receives a negative value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c14e9d2c-8f00-4f8d-8cd4-d3640045b447
📒 Files selected for processing (4)
backend/src/constants/ErrorMessage.tsbackend/src/paymaster/index.tsbackend/src/routes/admin-routes.tsbackend/src/routes/deposit-route.ts
| const encodedData = encodeFunctionData({ | ||
| abi: parseAbi(['function withdrawTo(address withdrawAddress, uint256 amount)']), | ||
| functionName: 'withdrawTo', | ||
| args: [withdrawAddress as Address, amountInWei] | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for withdrawTo in the ABI files to verify the function signature
echo "=== Checking EtherspotAbiV06 for withdrawTo ==="
rg -n "withdrawTo" backend/src/abi/EtherspotAbi.ts
echo "=== Checking EtherspotAbiV07 for withdrawTo ==="
rg -n "withdrawTo" backend/src/abi/EtherspotVerifyingSignerAbi.ts
echo "=== Checking VerifyingPaymaster ABIs ==="
rg -n "withdrawTo" backend/src/abi/VerifyingPaymasterAbi.ts
rg -n "withdrawTo" backend/src/abi/VerifyingPaymasterAbiV2.ts
rg -n "withdrawTo" backend/src/abi/VerifyingPaymasterAbiV3.tsRepository: etherspot/arka
Length of output: 308
🏁 Script executed:
# Get the withdrawTo function definitions from the ABI files
echo "=== EtherspotVerifyingSignerAbi.ts (V07) around line 451 ==="
sed -n '445,460p' backend/src/abi/EtherspotVerifyingSignerAbi.ts
echo "=== VerifyingPaymasterAbi.ts around line 410 ==="
sed -n '405,420p' backend/src/abi/VerifyingPaymasterAbi.ts
echo "=== VerifyingPaymasterAbiV2.ts around line 307 ==="
sed -n '302,318p' backend/src/abi/VerifyingPaymasterAbiV2.ts
echo "=== VerifyingPaymasterAbiV3.ts around line 307 ==="
sed -n '302,318p' backend/src/abi/VerifyingPaymasterAbiV3.ts
echo "=== Check if EtherspotAbi.ts has any withdrawal-related functions ==="
rg -n "withdraw" backend/src/abi/EtherspotAbi.ts | head -20Repository: etherspot/arka
Length of output: 34263
🏁 Script executed:
# Check the context around withdrawDeposit method to see if it has version checks
echo "=== Lines around withdrawDeposit method (1258-1320) ==="
sed -n '1250,1320p' backend/src/paymaster/index.ts
echo "=== Check how deposit() method handles versions ==="
sed -n '1180,1220p' backend/src/paymaster/index.tsRepository: etherspot/arka
Length of output: 5463
Add version parameter and use version-specific ABIs for withdrawDeposit method.
The method uses a hardcoded inline ABI assuming withdrawTo(address, uint256) exists on all paymaster contract versions, but EtherspotAbiV06 does not have this function. The deposit() method correctly handles this with an isEpv06 parameter and conditional ABI selection. Update withdrawDeposit() to:
- Add
isEpv06: booleanparameter to the method signature - Use conditional ABI selection:
isEpv06 ? EtherspotAbiV06 : EtherspotAbiV07 - Use version-specific function names:
'withdrawFunds'for V06,'withdrawTo'for V07+
Without this change, calls to withdrawDeposit() will fail on V06 chains.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/paymaster/index.ts` around lines 1272 - 1276, Update
withdrawDeposit to accept an isEpv06: boolean parameter and pick the ABI and
function name based on it: use EtherspotAbiV06 and function name 'withdrawFunds'
when isEpv06 is true, otherwise use EtherspotAbiV07 and 'withdrawTo' for V07+;
then call encodeFunctionData with parseAbi([...]) using the chosen ABI and
functionName (refer to withdrawDeposit, EtherspotAbiV06, EtherspotAbiV07,
withdrawFunds, withdrawTo, and encodeFunctionData) so V06 chains call the
correct function signature.
| let tx; | ||
| if (!feeData.maxFeePerGas || this.skipType2Txns.includes(chainId.toString())) { | ||
| tx = await walletClient.writeContract({ | ||
| address: paymasterAddress as Address, | ||
| abi: verifyingPaymasterAbi, | ||
| functionName: 'withdrawStake', | ||
| args: [withdrawAddress], | ||
| type: "legacy", | ||
| gasPrice: feeData.gasPrice ?? undefined, | ||
| }); | ||
| } else { | ||
| tx = await walletClient.writeContract({ | ||
| address: paymasterAddress as Address, | ||
| abi: verifyingPaymasterAbi, | ||
| functionName: 'withdrawStake', | ||
| args: [withdrawAddress], | ||
| maxFeePerGas: feeData.maxFeePerGas ?? undefined, | ||
| maxPriorityFeePerGas: feeData.maxPriorityFeePerGas ?? undefined, | ||
| type: "eip1559" | ||
| }); | ||
| } |
There was a problem hiding this comment.
Inconsistent gas fee check may cause EIP-1559 transactions with zero maxFeePerGas.
The condition at line 1479 is missing || feeData.maxFeePerGas === BigInt(0) which exists in other methods like withdrawDeposit (line 1293), deposit (line 1227), and deployVp (line 1360). If getGasFee returns null and publicClient.getGasPrice() returns 0n, the code will attempt to send an EIP-1559 transaction with maxFeePerGas: 0n, which will likely fail.
The same issue exists in addStake at line 1420.
🐛 Proposed fix
- if (!feeData.maxFeePerGas || this.skipType2Txns.includes(chainId.toString())) {
+ if (!feeData.maxFeePerGas || this.skipType2Txns.includes(chainId.toString()) || feeData.maxFeePerGas === BigInt(0)) {Apply the same fix to addStake at line 1420.
📝 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.
| let tx; | |
| if (!feeData.maxFeePerGas || this.skipType2Txns.includes(chainId.toString())) { | |
| tx = await walletClient.writeContract({ | |
| address: paymasterAddress as Address, | |
| abi: verifyingPaymasterAbi, | |
| functionName: 'withdrawStake', | |
| args: [withdrawAddress], | |
| type: "legacy", | |
| gasPrice: feeData.gasPrice ?? undefined, | |
| }); | |
| } else { | |
| tx = await walletClient.writeContract({ | |
| address: paymasterAddress as Address, | |
| abi: verifyingPaymasterAbi, | |
| functionName: 'withdrawStake', | |
| args: [withdrawAddress], | |
| maxFeePerGas: feeData.maxFeePerGas ?? undefined, | |
| maxPriorityFeePerGas: feeData.maxPriorityFeePerGas ?? undefined, | |
| type: "eip1559" | |
| }); | |
| } | |
| let tx; | |
| if (!feeData.maxFeePerGas || this.skipType2Txns.includes(chainId.toString()) || feeData.maxFeePerGas === BigInt(0)) { | |
| tx = await walletClient.writeContract({ | |
| address: paymasterAddress as Address, | |
| abi: verifyingPaymasterAbi, | |
| functionName: 'withdrawStake', | |
| args: [withdrawAddress], | |
| type: "legacy", | |
| gasPrice: feeData.gasPrice ?? undefined, | |
| }); | |
| } else { | |
| tx = await walletClient.writeContract({ | |
| address: paymasterAddress as Address, | |
| abi: verifyingPaymasterAbi, | |
| functionName: 'withdrawStake', | |
| args: [withdrawAddress], | |
| maxFeePerGas: feeData.maxFeePerGas ?? undefined, | |
| maxPriorityFeePerGas: feeData.maxPriorityFeePerGas ?? undefined, | |
| type: "eip1559" | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/paymaster/index.ts` around lines 1478 - 1498, The gas-fee check
for deciding legacy vs EIP-1559 is missing the zero-check and can produce
EIP-1559 txns with maxFeePerGas === 0n; update the conditional that decides
which branch to use (the if guarding the walletClient.writeContract call for
withdrawStake) to include "|| feeData.maxFeePerGas === BigInt(0)" (same pattern
used in withdrawDeposit, deposit, deployVp) so that zero maxFeePerGas falls back
to legacy; apply the identical fix to the addStake branch as well so both
addStake and withdrawStake use the same guarded condition involving
feeData.maxFeePerGas === BigInt(0) and skipType2Txns.
| server.post('/withdrawStake', async (request, reply) => { | ||
| try { | ||
| if (!request.body) return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.MISSING_PARAMS }); | ||
|
|
||
| const body: any = request.body; | ||
| const query: any = request.query; | ||
| const chainId = query['chainId']; | ||
| const apiKey = query['apiKey']; | ||
| const epVersion = body.params?.[0]; | ||
| const withdrawAddress = body.params?.[1]; | ||
|
|
||
| if (!chainId || isNaN(chainId) || !apiKey) { | ||
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_DATA }); | ||
| } | ||
|
|
||
| if(!withdrawAddress || !isAddress(withdrawAddress)) { | ||
| return reply.code(ReturnCode.FAILURE).send({error: ErrorMessage.INVALID_WITHDRAW_ADDRESS}); | ||
| } | ||
|
|
||
| if (!epVersion || (epVersion !== EPVersions.EPV_06 && epVersion !== EPVersions.EPV_07 && epVersion !== EPVersions.EPV_08)) { | ||
| return reply.code(ReturnCode.FAILURE).send({error: ErrorMessage.INVALID_EP_VERSION}); | ||
| } | ||
|
|
||
| const apiKeyEntity: APIKey | null = await server.apiKeyRepository.findOneByApiKey(apiKey); | ||
| if (!apiKeyEntity) return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_API_KEY }); | ||
|
|
||
| let verifyingPaymasters, supportedEPs; | ||
|
|
||
| if(epVersion === EPVersions.EPV_06) { | ||
| verifyingPaymasters = apiKeyEntity.verifyingPaymasters ? JSON.parse(apiKeyEntity.verifyingPaymasters) : {}; | ||
| supportedEPs = SUPPORTED_ENTRYPOINTS.EPV_06; | ||
| } else if (epVersion === EPVersions.EPV_07) { | ||
| verifyingPaymasters = apiKeyEntity.verifyingPaymastersV2 ? JSON.parse(apiKeyEntity.verifyingPaymastersV2) : {}; | ||
| supportedEPs = SUPPORTED_ENTRYPOINTS.EPV_07; | ||
| } else { | ||
| verifyingPaymasters = apiKeyEntity.verifyingPaymastersV3 ? JSON.parse(apiKeyEntity.verifyingPaymastersV3) : {}; | ||
| supportedEPs = SUPPORTED_ENTRYPOINTS.EPV_08; | ||
| } | ||
|
|
||
| if (!verifyingPaymasters[chainId]) { | ||
| return reply.code(ReturnCode.FAILURE).send( | ||
| {error: `${ErrorMessage.VP_NOT_DEPLOYED}`} | ||
| ); | ||
| } | ||
|
|
||
| let privateKey; | ||
| let bundlerApiKey = apiKey; | ||
| let supportedNetworks; | ||
|
|
||
| if (!unsafeMode) { | ||
| const AWSresponse = await client.send( | ||
| new GetSecretValueCommand({ | ||
| SecretId: prefixSecretId + apiKey, | ||
| }) | ||
| ); | ||
| const secrets = JSON.parse(AWSresponse.SecretString ?? '{}'); | ||
| if (!secrets['PRIVATE_KEY']) { | ||
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_API_KEY }); | ||
| } | ||
| if (secrets['BUNDLER_API_KEY']) { | ||
| bundlerApiKey = secrets['BUNDLER_API_KEY']; | ||
| } | ||
| privateKey = secrets['PRIVATE_KEY']; | ||
| supportedNetworks = secrets['SUPPORTED_NETWORKS']; | ||
| } else { | ||
| privateKey = decode(apiKeyEntity.privateKey, server.config.HMAC_SECRET); | ||
| supportedNetworks = apiKeyEntity.supportedNetworks; | ||
| if (apiKeyEntity.bundlerApiKey) { | ||
| bundlerApiKey = apiKeyEntity.bundlerApiKey; | ||
| } | ||
| } | ||
|
|
||
| if (server.config.SUPPORTED_NETWORKS == '' && !SupportedNetworks) { | ||
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.UNSUPPORTED_NETWORK }); | ||
| } | ||
| const networkConfig = getNetworkConfig( | ||
| chainId, | ||
| supportedNetworks ?? '', | ||
| supportedEPs | ||
| ); | ||
| if (!networkConfig) { | ||
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.UNSUPPORTED_NETWORK }); | ||
| } | ||
| let bundlerUrl = networkConfig.bundler; | ||
| if (networkConfig.bundler.includes('etherspot.io')) { | ||
| bundlerUrl = `${networkConfig.bundler}?api-key=${bundlerApiKey}`; | ||
| } | ||
|
|
||
| const tx = await paymaster.withdrawStake( | ||
| privateKey, | ||
| bundlerUrl, | ||
| withdrawAddress, | ||
| verifyingPaymasters[chainId], | ||
| chainId, | ||
| server.log | ||
| ); | ||
| return reply.code(ReturnCode.SUCCESS).send(tx); | ||
| } catch (error: any) { | ||
| request.log.error(error); | ||
| return reply.code(ReturnCode.FAILURE).send({ error: error.message ?? ErrorMessage.FAILED_TO_PROCESS }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting duplicated credential resolution logic.
The private key derivation (lines 657-682) and network configuration resolution (lines 684-698) patterns are duplicated across /withdrawStake, /addStake, /deployVerifyingPaymaster, and deposit routes. Consider extracting this into a shared helper function to improve maintainability.
The endpoint implementation is otherwise correct and follows established patterns.
♻️ Suggested helper extraction
// In a shared utils file
async function resolveCredentialsAndNetwork(
apiKey: string,
chainId: number,
epVersion: EPVersions,
apiKeyEntity: APIKey,
unsafeMode: boolean,
client: SecretsManagerClient,
prefixSecretId: string,
serverConfig: any,
supportedEntrypoints: any
) {
let privateKey: string;
let bundlerApiKey = apiKey;
let supportedNetworks = apiKeyEntity.supportedNetworks;
if (!unsafeMode) {
const AWSresponse = await client.send(
new GetSecretValueCommand({ SecretId: prefixSecretId + apiKey })
);
const secrets = JSON.parse(AWSresponse.SecretString ?? '{}');
if (!secrets['PRIVATE_KEY']) throw new Error('Invalid API key');
if (secrets['BUNDLER_API_KEY']) bundlerApiKey = secrets['BUNDLER_API_KEY'];
privateKey = secrets['PRIVATE_KEY'];
supportedNetworks = secrets['SUPPORTED_NETWORKS'];
} else {
privateKey = decode(apiKeyEntity.privateKey, serverConfig.HMAC_SECRET);
if (apiKeyEntity.bundlerApiKey) bundlerApiKey = apiKeyEntity.bundlerApiKey;
}
// ... resolve networkConfig based on epVersion
return { privateKey, bundlerApiKey, networkConfig, bundlerUrl };
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/routes/admin-routes.ts` around lines 612 - 713, Extract the
duplicated credential and network resolution logic from withdrawStake (and the
similar blocks in addStake, deployVerifyingPaymaster, deposit routes) into a
shared helper like resolveCredentialsAndNetwork that accepts (apiKey, chainId,
epVersion, apiKeyEntity, unsafeMode, client, prefixSecretId, server.config,
supportedEntrypoints) and returns { privateKey, bundlerApiKey,
supportedNetworks, networkConfig, bundlerUrl }; inside the helper perform the
SecretsManager GetSecretValueCommand flow and unsafeMode decode using
decode(apiKeyEntity.privateKey, server.config.HMAC_SECRET), set bundlerApiKey
from secrets or apiKeyEntity.bundlerApiKey, call getNetworkConfig(chainId,
supportedNetworks, supportedEPs) to build networkConfig and compute bundlerUrl
(append ?api-key= when bundler includes etherspot.io), throw or return errors
consistent with existing ErrorMessage values, then replace the duplicated blocks
in withdrawStake (and the other endpoints) with a call to this helper and use
its returned values (privateKey, bundlerUrl, etc.) when calling
paymaster.withdrawStake.
| if ( | ||
| withdrawAmount === undefined || | ||
| withdrawAmount === null || | ||
| withdrawAmount === '' || | ||
| isNaN(Number(withdrawAmount)) || | ||
| !chainId || | ||
| isNaN(chainId) | ||
| ) { | ||
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_DATA }); | ||
| } |
There was a problem hiding this comment.
Negative amount validation is missing.
The validation checks if withdrawAmount is a valid number but doesn't reject negative values. A negative withdrawal amount would be parsed by parseEther and could lead to unexpected behavior.
🛡️ Proposed fix
if (
withdrawAmount === undefined ||
withdrawAmount === null ||
withdrawAmount === '' ||
isNaN(Number(withdrawAmount)) ||
+ Number(withdrawAmount) <= 0 ||
!chainId ||
isNaN(chainId)
) {
return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_DATA });
}📝 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 ( | |
| withdrawAmount === undefined || | |
| withdrawAmount === null || | |
| withdrawAmount === '' || | |
| isNaN(Number(withdrawAmount)) || | |
| !chainId || | |
| isNaN(chainId) | |
| ) { | |
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_DATA }); | |
| } | |
| if ( | |
| withdrawAmount === undefined || | |
| withdrawAmount === null || | |
| withdrawAmount === '' || | |
| isNaN(Number(withdrawAmount)) || | |
| Number(withdrawAmount) <= 0 || | |
| !chainId || | |
| isNaN(chainId) | |
| ) { | |
| return reply.code(ReturnCode.FAILURE).send({ error: ErrorMessage.INVALID_DATA }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/routes/deposit-route.ts` around lines 157 - 166, The validation
block around withdrawAmount (used later with parseEther) currently allows
negative numbers; update the check in the same block that returns
ReturnCode.FAILURE / ErrorMessage.INVALID_DATA to also reject non-positive
values by parsing withdrawAmount to a numeric value (e.g.,
Number(withdrawAmount) or parseFloat) and ensuring it is > 0 and finite before
proceeding; ensure this check occurs before any call to parseEther so parseEther
never receives a negative value.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/CHANGELOG.md`:
- Line 3: Update the changelog line to clearly state the endpoint intent:
replace “Added endpoints to withdraw stake and deposit.” with wording that
specifies the implemented route is for withdrawing a deposit (e.g., “Added
endpoint withdrawDeposit to withdraw a deposit; not a deposit endpoint”) and
include the route name withdrawDeposit to avoid confusion with a deposit
operation.
- Around line 2-4: The markdown heading "## [4.2.3] - 2026-04-05" in
CHANGELOG.md needs blank lines before and after to satisfy MD022; edit the file
and insert a single empty line above that heading (so it is separated from
previous content) and another empty line immediately after the heading line
(before the "- Added endpoints..." list) to ensure proper heading spacing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02c66f16-8f5c-49d5-b090-c96804e82f62
📒 Files selected for processing (2)
backend/CHANGELOG.mdbackend/package.json
| ## [4.2.3] - 2026-04-05 | ||
| - Added endpoints to withdraw stake and deposit. | ||
|
|
There was a problem hiding this comment.
Fix markdown heading spacing to satisfy MD022.
Line 2 needs blank lines above and below the heading; current formatting violates markdownlint and can fail docs quality checks.
Suggested patch
# Changelog
+
## [4.2.3] - 2026-04-05
+
- Added endpoints to withdraw stake and deposit.📝 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.2.3] - 2026-04-05 | |
| - Added endpoints to withdraw stake and deposit. | |
| # Changelog | |
| ## [4.2.3] - 2026-04-05 | |
| - Added endpoints to withdraw stake and deposit. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
[warning] 2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/CHANGELOG.md` around lines 2 - 4, The markdown heading "## [4.2.3] -
2026-04-05" in CHANGELOG.md needs blank lines before and after to satisfy MD022;
edit the file and insert a single empty line above that heading (so it is
separated from previous content) and another empty line immediately after the
heading line (before the "- Added endpoints..." list) to ensure proper heading
spacing.
| @@ -1,4 +1,7 @@ | |||
| # Changelog | |||
| ## [4.2.3] - 2026-04-05 | |||
| - Added endpoints to withdraw stake and deposit. | |||
There was a problem hiding this comment.
Clarify the release note wording for endpoint intent.
Line 3 says “withdraw stake and deposit,” but the implemented route is withdrawDeposit (withdrawing deposit), not depositing. Please make the wording explicit to avoid API confusion.
Suggested wording
-- Added endpoints to withdraw stake and deposit.
+- Added endpoints to withdraw stake and withdraw deposit.📝 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.
| - Added endpoints to withdraw stake and deposit. | |
| - Added endpoints to withdraw stake and withdraw deposit. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/CHANGELOG.md` at line 3, Update the changelog line to clearly state
the endpoint intent: replace “Added endpoints to withdraw stake and deposit.”
with wording that specifies the implemented route is for withdrawing a deposit
(e.g., “Added endpoint withdrawDeposit to withdraw a deposit; not a deposit
endpoint”) and include the route name withdrawDeposit to avoid confusion with a
deposit operation.
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit
New Features
Bug Fixes / Errors
Documentation
Chores