Open
Conversation
📝 WalkthroughWalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Prompt as Prompt Logic
participant Signer as Transaction Signer
participant Provider as JsonRpcProvider
participant API as Stakekit API
User->>Prompt: Provide inputs
Prompt->>Signer: Build transaction(s)
Signer->>Provider: Fetch chainId, nonce, fee data
Provider-->>Signer: Return chainId, nonce, fee data
Signer->>Signer: Sign -> {signed, isEvmTx}
alt isEvmTx == true
Signer->>Provider: Broadcast signed tx
Provider-->>Signer: transactionHash
Signer->>API: Notify with transactionHash
API-->>User: Confirmation
else isEvmTx == false
Signer->>API: Submit signedPayload
API-->>User: Confirmation
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can get early access to new features in CodeRabbit.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
recipes/borrow.ts (1)
562-575: Add an explicit same-chain guard when reusing one provider/nonce stream.
providerandbaseNonceare initialized from the first EVM tx and reused for the whole batch. Add a chain consistency check before signing to fail fast if a mixed-chain batch ever appears.Also applies to: 604-607
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@recipes/borrow.ts` around lines 562 - 575, The code initializes provider and baseNonce from firstEvmTx and then reuses them for signing; add an explicit same-chain guard so any transaction in transactions that will be signed with provider/baseNonce is verified to have the same chainId as firstEvmTx before signing (e.g., check tx.chainId === firstEvmTx.chainId) and throw or return an error immediately if a mismatch is found; update both the signing loop/check site(s) that reference provider/baseNonce (the block using provider/getTransactionCount and the later signing path around lines 604-607) to perform this validation so mixed-chain batches fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@recipes/borrow.ts`:
- Around line 540-549: The current fee-population logic can mix legacy and
EIP-1559 fields and must be changed so we only fill missing fields: in the block
that calls provider.getFeeData() (referencing txData and provider.getFeeData),
detect three cases — if neither txData.gasPrice nor txData.maxFeePerGas is set,
populate EIP-1559 fields from feeData or fallback to gasPrice; if
txData.gasPrice is already present, do nothing (leave legacy path untouched); if
txData.maxFeePerGas exists but txData.maxPriorityFeePerGas is missing, only set
maxPriorityFeePerGas from feeData.maxPriorityFeePerGas — never set both gasPrice
and maxFeePerGas on the same txData object.
- Around line 424-441: The enum/options and boolean prompts (where
prop.enum/prop.options is truthy and where type === "boolean") always assign
result[name] even when isRequired is false; change both prompts to offer an
explicit "Skip" option for optional fields (e.g., add a choice with a distinct
undefined/empty value) and update the assignment to only set result[name] when
response.value is present or isRequired is true (same pattern used by the
array/input branches). Locate the Enquirer.prompt calls (the select block using
choices/initial and the confirm block) and: for enums/options add a "Skip"
choice when isRequired is false, and for booleans replace or augment the confirm
with a three-way select (Yes/No/Skip) or equivalent so a skip can return
undefined; then guard the result[name] assignment with the conditional check
(response.value || isRequired).
---
Nitpick comments:
In `@recipes/borrow.ts`:
- Around line 562-575: The code initializes provider and baseNonce from
firstEvmTx and then reuses them for signing; add an explicit same-chain guard so
any transaction in transactions that will be signed with provider/baseNonce is
verified to have the same chainId as firstEvmTx before signing (e.g., check
tx.chainId === firstEvmTx.chainId) and throw or return an error immediately if a
mismatch is found; update both the signing loop/check site(s) that reference
provider/baseNonce (the block using provider/getTransactionCount and the later
signing path around lines 604-607) to perform this validation so mixed-chain
batches fail fast.
Comment on lines
424
to
441
| if (prop.enum || prop.options) { | ||
| const baseChoices = prop.options || (prop.enum as string[]); | ||
| const skipChoice = "<skip>"; | ||
| const choices = isRequired ? baseChoices : [skipChoice, ...baseChoices]; | ||
| const choices = prop.options || (prop.enum as string[]); | ||
| const response: any = await Enquirer.prompt({ | ||
| type: "select", | ||
| name: "value", | ||
| message, | ||
| choices, | ||
| initial: prop.default, | ||
| } as any); | ||
| if (!isRequired && response.value === skipChoice) continue; | ||
| result[name] = response.value; | ||
| } else if (type === "boolean") { | ||
| const response: any = await Enquirer.prompt({ | ||
| type: "confirm", | ||
| name: "value", | ||
| message, | ||
| initial: prop.default as boolean, | ||
| initial: prop.default || false, | ||
| } as any); | ||
| result[name] = response.value; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In enquirer@2.4.1, do selectandconfirm prompts support a true "no selection" result without adding a custom skip option?
💡 Result:
No.
selectalways has a focused choice, andselectedis “the currently focused choice” (for single-select). So submitting the prompt necessarily returns some choice (unless you add a dedicated “Skip/None” choice whosevalueyou treat as empty). [2]confirmis defined to return onlytrueorfalse, not an empty/undefined “no answer” state. [1]
If you want a true “no selection” outcome, you need either (a) an explicit skip/none option in the choices, or (b) treat prompt cancellation (e.g., Ctrl+C leading to a rejected promise) as “no selection” in your app logic.
Sources: [1] [2]
🏁 Script executed:
# Check the file size first
wc -l recipes/borrow.tsRepository: stakekit/api-recipes
Length of output: 86
🏁 Script executed:
# Read the context around lines 424-441
sed -n '400,450p' recipes/borrow.ts | cat -nRepository: stakekit/api-recipes
Length of output: 2223
🏁 Script executed:
# Search for 'isRequired' variable in the borrow.ts file
rg 'isRequired' recipes/borrow.ts -B 3 -A 3Repository: stakekit/api-recipes
Length of output: 1083
🏁 Script executed:
# Get the broader function context to understand prop structure
sed -n '380,460p' recipes/borrow.ts | cat -nRepository: stakekit/api-recipes
Length of output: 3360
Optional enum/boolean inputs cannot be truly omitted despite being optional fields.
The code unconditionally assigns values to result[name] for both enum/options (line 54) and boolean (line 62) fields, even when isRequired is false. This forces optional fields to always receive a value, preventing callers from genuinely omitting them. In contrast, array and input fields correctly check if (response.value || isRequired) before assignment, allowing optional fields to be skipped.
Since enquirer's select and confirm prompts don't natively support "no selection" results, add a "Skip" option for optional fields:
Suggested fix
if (prop.enum || prop.options) {
const choices = prop.options || (prop.enum as string[]);
+ if (!isRequired) choices.push("Skip");
const response: any = await Enquirer.prompt({
type: "select",
name: "value",
message,
choices,
initial: prop.default,
} as any);
- result[name] = response.value;
+ if (response.value !== "Skip") result[name] = response.value;
} else if (type === "boolean") {
const response: any = await Enquirer.prompt({
- type: "confirm",
+ type: isRequired ? "confirm" : "select",
name: "value",
message,
- initial: prop.default || false,
+ ...(isRequired
+ ? { initial: prop.default ?? false }
+ : { choices: [true, false, "Skip"], initial: "Skip" }),
} as any);
- result[name] = response.value;
+ if (response.value !== "Skip") result[name] = response.value;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@recipes/borrow.ts` around lines 424 - 441, The enum/options and boolean
prompts (where prop.enum/prop.options is truthy and where type === "boolean")
always assign result[name] even when isRequired is false; change both prompts to
offer an explicit "Skip" option for optional fields (e.g., add a choice with a
distinct undefined/empty value) and update the assignment to only set
result[name] when response.value is present or isRequired is true (same pattern
used by the array/input branches). Locate the Enquirer.prompt calls (the select
block using choices/initial and the confirm block) and: for enums/options add a
"Skip" choice when isRequired is false, and for booleans replace or augment the
confirm with a three-way select (Yes/No/Skip) or equivalent so a skip can return
undefined; then guard the result[name] assignment with the conditional check
(response.value || isRequired).
…gning to fail in ethers v6
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
recipes/borrow.ts (1)
424-441:⚠️ Potential issue | 🟠 MajorOptional enum/boolean inputs are still effectively required.
Line 433 and Line 441 always assign
result[name], so optional fields cannot actually be omitted.Suggested patch
if (prop.enum || prop.options) { - const choices = prop.options || (prop.enum as string[]); + const choices = [...(prop.options || (prop.enum as string[]))]; + if (!isRequired) choices.push("__skip__"); const response: any = await Enquirer.prompt({ type: "select", name: "value", message, choices, initial: prop.default, } as any); - result[name] = response.value; + if (response.value !== "__skip__") result[name] = response.value; } else if (type === "boolean") { const response: any = await Enquirer.prompt({ - type: "confirm", + type: isRequired ? "confirm" : "select", name: "value", message, - initial: prop.default || false, + ...(isRequired + ? { initial: prop.default ?? false } + : { + choices: [ + { name: "Yes", value: true }, + { name: "No", value: false }, + { name: "Skip", value: "__skip__" }, + ], + initial: 2, + }), } as any); - result[name] = response.value; + if (response.value !== "__skip__") result[name] = response.value;In enquirer 2.x, can `select` or `confirm` return an actual undefined/no-selection result without adding an explicit "Skip/None" choice?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@recipes/borrow.ts` around lines 424 - 441, The enum/boolean prompts always assign result[name], making optional fields required; change the logic so optional properties (check the prop optional/required flag) only set result[name] when the user actually chooses a value: for enum/options prompts (the block checking prop.enum || prop.options) add a "Skip" or "None" choice when the property is optional and only assign result[name] if response.value !== "SKIP" (or undefined), and for boolean prompts replace the confirm (which always yields true/false) with a select that offers ["Yes","No","Skip"] for optional booleans and map those to true/false/undefined before conditionally assigning result[name]; update the branches using Enquirer.prompt and the assignments to result[name] accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@recipes/borrow.ts`:
- Around line 572-587: The current logic pins a single JsonRpcProvider and
baseNonce to firstEvmTx.chainId (variables provider and baseNonce) and then
reuses nonceOffset for all EVM transactions, which breaks when subsequent EVM
txs target different chainIds; change this to maintain per-chain maps: create a
Map<number, JsonRpcProvider> (using getRpcUrl and JsonRpcProvider) and a
Map<number, number> for baseNonce plus a Map<number, number> for nonceOffset
(keyed by tx.chainId), lazily initializing provider and baseNonce for a chain
the first time you see an EVM transaction (inspect each tx in transactions with
the same EVM check used for firstEvmTx), and when assigning nonces or
broadcasting use the provider/baseNonce/nonceOffset for tx.chainId instead of
the single provider/baseNonce/nonceOffset; optionally add a fail-fast guard if
provider creation or nonce fetch fails for any chain.
---
Duplicate comments:
In `@recipes/borrow.ts`:
- Around line 424-441: The enum/boolean prompts always assign result[name],
making optional fields required; change the logic so optional properties (check
the prop optional/required flag) only set result[name] when the user actually
chooses a value: for enum/options prompts (the block checking prop.enum ||
prop.options) add a "Skip" or "None" choice when the property is optional and
only assign result[name] if response.value !== "SKIP" (or undefined), and for
boolean prompts replace the confirm (which always yields true/false) with a
select that offers ["Yes","No","Skip"] for optional booleans and map those to
true/false/undefined before conditionally assigning result[name]; update the
branches using Enquirer.prompt and the assignments to result[name] accordingly.
Comment on lines
+572
to
+587
| let provider: JsonRpcProvider | undefined; | ||
| let baseNonce: number | undefined; | ||
| const firstEvmTx = transactions.find( | ||
| (tx) => | ||
| tx.signablePayload && | ||
| tx.status !== TransactionStatus.CONFIRMED && | ||
| tx.status !== TransactionStatus.SKIPPED && | ||
| (tx.signingFormat === SigningFormat.EVM_TRANSACTION || tx.signingFormat === undefined), | ||
| ); | ||
| if (firstEvmTx) { | ||
| const rpcUrl = getRpcUrl(firstEvmTx.chainId); | ||
| provider = new JsonRpcProvider(rpcUrl); | ||
| baseNonce = await provider.getTransactionCount(wallet.address, "pending"); | ||
| console.log(` On-chain nonce: ${baseNonce} (chain ${firstEvmTx.chainId})`); | ||
| } | ||
| let nonceOffset = 0; |
There was a problem hiding this comment.
Provider/nonce context is pinned to the first EVM transaction chain.
Line 583 initializes a single provider from firstEvmTx.chainId, then Line 614 reuses that nonce context for all later transactions. If any later EVM tx has a different chainId, nonce and broadcast context can be wrong.
Fail-fast guard (minimal fix)
let provider: JsonRpcProvider | undefined;
let baseNonce: number | undefined;
+ const evmChainIds = new Set(
+ transactions
+ .filter(
+ (tx) =>
+ tx.signablePayload &&
+ tx.status !== TransactionStatus.CONFIRMED &&
+ tx.status !== TransactionStatus.SKIPPED &&
+ (tx.signingFormat === SigningFormat.EVM_TRANSACTION || tx.signingFormat === undefined),
+ )
+ .map((tx) => tx.chainId),
+ );
+
+ if (evmChainIds.size > 1) {
+ throw new Error("Mixed EVM chainIds in one action are not supported with single-provider nonce sequencing");
+ }
+
const firstEvmTx = transactions.find(Also applies to: 614-623
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@recipes/borrow.ts` around lines 572 - 587, The current logic pins a single
JsonRpcProvider and baseNonce to firstEvmTx.chainId (variables provider and
baseNonce) and then reuses nonceOffset for all EVM transactions, which breaks
when subsequent EVM txs target different chainIds; change this to maintain
per-chain maps: create a Map<number, JsonRpcProvider> (using getRpcUrl and
JsonRpcProvider) and a Map<number, number> for baseNonce plus a Map<number,
number> for nonceOffset (keyed by tx.chainId), lazily initializing provider and
baseNonce for a chain the first time you see an EVM transaction (inspect each tx
in transactions with the same EVM check used for firstEvmTx), and when assigning
nonces or broadcasting use the provider/baseNonce/nonceOffset for tx.chainId
instead of the single provider/baseNonce/nonceOffset; optionally add a fail-fast
guard if provider creation or nonce fetch fails for any chain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added
Description of new functionality, feature, or content that has been added in this pull request.
Changed
Description of the modifications made to existing functionality, feature, or content in this pull request. This could include changes to code, CI, documentation, etc.
Summary by CodeRabbit
New Features
Bug Fixes
Style