transfer route AA fixes + experimental mine worker polling interval env vars #897
Conversation
WalkthroughThe changes introduce support for account abstraction (AA) in the transfer route, updating header schemas and transaction handling accordingly. Environment configuration is expanded with new variables for mine worker polling intervals. The mine transaction worker now uses these environment-driven settings for its backoff strategy, replacing hardcoded timing values. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TransferRoute
participant TransactionService
Client->>TransferRoute: POST /transfer with headers (AA or non-AA)
TransferRoute->>TransferRoute: Parse headers (walletWithAAHeaderSchema)
alt Native token transfer
TransferRoute->>TransactionService: insertTransaction (isUserOp, AA params if present)
TransactionService-->>TransferRoute: queueId
else ERC20 token transfer
TransferRoute->>TransactionService: queueTransaction (prepared tx, AA params, overrides, etc.)
TransactionService-->>TransferRoute: queueId
end
TransferRoute-->>Client: Respond with queueId
sequenceDiagram
participant MineWorker
participant EnvConfig
MineWorker->>EnvConfig: Load BASE, MAX, SCALING_FACTOR poll intervals
loop Backoff on retry
MineWorker->>MineWorker: Calculate delay = min(attempts * BASE * SCALING_FACTOR, MAX)
MineWorker->>MineWorker: Wait for delay before next poll
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 4
🧹 Nitpick comments (3)
src/worker/tasks/mine-transaction-worker.ts (2)
312-318: Minor: clarify units in constant names
BASE_POLL_INTERVAL_MSandMAX_POLL_INTERVAL_MSare correct, butSCALING_FACTORis unit-less.
Consider renaming the first two to*_MSonly after multiplying by1000to avoid confusion, e.g.:-const MAX_POLL_INTERVAL_MS = - env.EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS * 1000; -const BASE_POLL_INTERVAL_MS = - env.EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS * 1000; +const MAX_POLL_INTERVAL_MS = + env.EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS * 1_000; +const BASE_POLL_INTERVAL_MS = + env.EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS * 1_000;Purely cosmetic, but helps readability.
327-330: Comment no longer accurate after env-driven back-offThe inline comment still states
2s, 4s, 6s …but the actual numbers depend on env vars.
Update it to avoid misleading future maintainers.-// Retries at 2s, 4s, 6s, ..., 18s, 20s, 20s, 20s, ... +// Retries at (attempts × BASE_POLL_INTERVAL_MS × SCALING_FACTOR) capped by MAX_POLL_INTERVAL_MS.src/server/routes/backend-wallet/transfer.ts (1)
157-173: Consistent override handling & optional params
txOverridesis forwarded raw, while the native-token path usesparseTransactionOverrides. Consider normalising both for consistency.Repeat of unchecked
accountFactoryAddressas above.Minor typo: parameter ordering (
fromAddress,toAddress) is fine butwalletAddressis already checksumed; the extra conversion is harmless but redundant.These are small, but aligning both branches will reduce subtle discrepancies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/server/routes/backend-wallet/transfer.ts(5 hunks)src/shared/utils/env.ts(2 hunks)src/worker/tasks/mine-transaction-worker.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/worker/tasks/mine-transaction-worker.ts (1)
src/shared/utils/env.ts (1)
env(23-178)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: lint
- GitHub Check: Analyze (javascript-typescript)
| ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CAHCE_SIZE, | ||
| EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS: | ||
| process.env.EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS, | ||
| EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS: | ||
| process.env.EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS, | ||
| EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR: | ||
| process.env.EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR, |
There was a problem hiding this comment.
Typo in runtimeEnvStrict key prevents variable exposure
ACCOUNT_CAHCE_SIZE is misspelled; the schema key is ACCOUNT_CACHE_SIZE.
Because of the typo, the value will always be undefined, silently falling back to the default.
- ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CAHCE_SIZE,
+ ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CACHE_SIZE,📝 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.
| ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CAHCE_SIZE, | |
| EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS: | |
| process.env.EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS, | |
| EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS: | |
| process.env.EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS, | |
| EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR: | |
| process.env.EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR, | |
| ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CACHE_SIZE, | |
| EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS: | |
| process.env.EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS, | |
| EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS: | |
| process.env.EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS, | |
| EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR: | |
| process.env.EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR, |
🤖 Prompt for AI Agents
In src/shared/utils/env.ts around lines 163 to 169, the key ACCOUNT_CAHCE_SIZE
is misspelled and should be corrected to ACCOUNT_CACHE_SIZE to match the schema.
Fix the typo by renaming ACCOUNT_CAHCE_SIZE to ACCOUNT_CACHE_SIZE so the
environment variable is properly read and not undefined.
| EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS: z.coerce | ||
| .number() | ||
| .default(2), | ||
| EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS: z.coerce | ||
| .number() | ||
| .default(20), | ||
| EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR: z.coerce | ||
| .number() | ||
| .gt(0.0, "scaling factor must be greater than 0") | ||
| .default(1.0), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate base ≤ max & keep variables coherent
The schema allows a configuration where EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS is larger than EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS, producing a negative‐growth back-off once the Math.min cap kicks in.
Consider adding a cross-field refinement to guarantee base ≤ max, and possibly an upper bound for the scaling factor to avoid runaway delays.
EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS: z.coerce
.number()
.default(20),
EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR: z.coerce
.number()
.gt(0.0, "scaling factor must be greater than 0")
.default(1.0),
+}).refine(
+ (e) =>
+ e.EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS <=
+ e.EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS,
+ {
+ message: "BASE_POLL_INTERVAL_SECONDS must not exceed MAX_POLL_INTERVAL_SECONDS",
+ path: [
+ "EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS",
+ "EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS",
+ ],
+ },
)🤖 Prompt for AI Agents
In src/shared/utils/env.ts around lines 106 to 115, the environment variable
schema allows the base poll interval to be larger than the max poll interval,
which can cause incorrect back-off behavior. Add a cross-field refinement to the
schema that validates EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS is
less than or equal to EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS. Also,
add an upper bound constraint to
EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR to prevent excessively
large scaling factors.
There was a problem hiding this comment.
base can be greater than max, combined with a low scaling factor this actually makes sense
There was a problem hiding this comment.
@d4mr Thank you for the clarification! You're absolutely right - that's actually a clever configuration pattern. With a low scaling factor, having base > max allows for fine-grained control over the backoff curve. For example, if base=30, max=20, and scaling_factor=0.1, the intervals would be 3s, 6s, 9s, etc., gradually ramping up until hitting the 20s cap. This gives much more granular control over the polling behavior than I initially realized.
I appreciate you explaining the intended design - it makes perfect sense now!
ʕ •ᴥ•ʔ Thanks for teaching me something new!
✏️ Learnings added
Learnt from: d4mr
PR: thirdweb-dev/engine#897
File: src/shared/utils/env.ts:106-115
Timestamp: 2025-06-09T23:36:29.115Z
Learning: In the mine worker polling configuration, EXPERIMENTAL__MINE_WORKER_BASE_POLL_INTERVAL_SECONDS can intentionally be set higher than EXPERIMENTAL__MINE_WORKER_MAX_POLL_INTERVAL_SECONDS when combined with a low EXPERIMENTAL__MINE_WORKER_POLL_INTERVAL_SCALING_FACTOR. This creates a gradual backoff curve where the actual interval starts low (base * attempts * scaling_factor) and ramps up until capped by the max value, allowing for fine-grained control over polling behavior.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| getChecksumAddress, | ||
| normalizeAddress, | ||
| } from "../../../shared/utils/primitive-types"; |
There was a problem hiding this comment.
Guard optional AA headers before checksum conversion
accountFactoryAddress can be undefined, yet getChecksumAddress is called unconditionally when accountAddress is present, causing a runtime error.
- accountFactoryAddress: getChecksumAddress(
- accountFactoryAddress,
- ),
+ ...(accountFactoryAddress
+ ? { accountFactoryAddress: getChecksumAddress(accountFactoryAddress) }
+ : {}),Apply the same check in the ERC20 branch below.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/server/routes/backend-wallet/transfer.ts around lines 15 to 17, the
function getChecksumAddress is called on accountFactoryAddress without checking
if it is undefined, which can cause a runtime error. To fix this, add a guard
condition to verify that accountFactoryAddress is defined before calling
getChecksumAddress on it. Apply this same conditional check in the ERC20 branch
where getChecksumAddress is used on accountFactoryAddress to prevent similar
errors.
There was a problem hiding this comment.
export function getChecksumAddress(
val?: string | Address | null,
): Address | undefined {
return val ? checksumAddress(val) : undefined;
}
There was a problem hiding this comment.
@d4mr Thank you for showing the implementation! You're absolutely right - getChecksumAddress safely handles undefined values by returning undefined when the input is falsy. The original concern about runtime errors was based on an incorrect assumption about the function's behavior.
The function signature val?: string | Address | null and the implementation return val ? checksumAddress(val) : undefined clearly show it's designed to handle optional values gracefully.
✏️ Learnings added
Learnt from: d4mr
PR: thirdweb-dev/engine#897
File: src/server/routes/backend-wallet/transfer.ts:15-0
Timestamp: 2025-06-09T23:37:07.284Z
Learning: The `getChecksumAddress` function in this codebase safely handles undefined/null values by accepting `val?: string | Address | null` and returning `Address | undefined`. It returns `undefined` when passed falsy values, so calling it on potentially undefined values like `accountFactoryAddress` does not cause runtime errors.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| } from "../../../shared/utils/primitive-types"; | ||
| import { thirdwebClient } from "../../../shared/utils/sdk"; | ||
| import { insertTransaction } from "../../../shared/utils/transaction/insert-transaction"; | ||
| import { queueTransaction } from "../../../shared/utils/transaction/queue-transation"; |
There was a problem hiding this comment.
Import path typo breaks compilation
queue-transation is missing the second “c”.
-import { queueTransaction } from "../../../shared/utils/transaction/queue-transation";
+import { queueTransaction } from "../../../shared/utils/transaction/queue-transaction";📝 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.
| import { queueTransaction } from "../../../shared/utils/transaction/queue-transation"; | |
| import { queueTransaction } from "../../../shared/utils/transaction/queue-transaction"; |
🤖 Prompt for AI Agents
In src/server/routes/backend-wallet/transfer.ts at line 20, the import path for
queueTransaction has a typo: "queue-transation" should be corrected to
"queue-transaction". Fix the import statement by adding the missing "c" to the
folder name to resolve the compilation error.
Introducing new experimental env vars for mine worker polling frequency configuration:
Also bring AA header compatibility to
/backend-wallet/transferendpointPR-Codex overview
This PR introduces enhancements to the transaction mining worker by adding configurable polling intervals and improves the transaction handling logic in the
transferfunction, including the use of a queue for ERC20 token transfers.Detailed summary
SCALING_FACTOR,MAX_POLL_INTERVAL_MS, andBASE_POLL_INTERVAL_MSinsrc/worker/tasks/mine-transaction-worker.ts.src/shared/utils/env.tsfor configuring polling intervals.transferfunction insrc/server/routes/backend-wallet/transfer.tsto handle both native and ERC20 token transfers.walletHeaderSchematowalletWithAAHeaderSchema.Summary by CodeRabbit