feat(liquidator): add one-shot liquidator package - #15
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new Bun-based Changes
sequenceDiagram
participant CLI as CLI Entry
participant Config as Config Loader
participant Runner as Runner
participant Gateway as Viem Gateway
participant Chain as Blockchain
participant Logger as Logger
CLI->>CLI: Parse CLI args
CLI->>Config: loadRuntimeConfig(env, options)
Config->>Config: Validate RPC, factory, privateKey
Config-->>CLI: RuntimeConfig
CLI->>Runner: runLiquidator(gateway, configs, options)
rect rgba(100, 150, 200, 0.5)
note over Runner,Gateway: Scan Phase
Runner->>Gateway: getAllRings(network)
Gateway->>Chain: Call factory.getAllRings()
Chain-->>Gateway: rings[]
Gateway-->>Runner: rings[]
loop For each ring
Runner->>Gateway: getPhase(network, ring)
Gateway->>Chain: Read ring.phase
Chain-->>Gateway: phase
alt phase == Active
Runner->>Gateway: getRingMembers(network, ring)
Gateway->>Chain: Call ring.getRing()
Chain-->>Gateway: members[]
loop For each member
Runner->>Gateway: isDelinquent(network, ring, member)
Gateway->>Chain: Check delinquency
Chain-->>Gateway: bool
alt isDelinquent
Runner->>Runner: Add to candidates
end
end
end
end
end
rect rgba(200, 150, 100, 0.5)
note over Runner,Gateway: Execute Phase
Runner->>Runner: Sort candidates (deterministic)
Runner->>Runner: Apply maxTxPerRun cap
loop For each candidate (within cap)
alt dryRun
Runner->>Logger: Log candidate
else
Runner->>Gateway: liquidate(network, ring, target)
Gateway->>Chain: Submit liquidate txn
Chain-->>Gateway: txHash
Runner->>Gateway: waitForReceipt(txHash)
Gateway->>Chain: Poll receipt
Chain-->>Gateway: receipt
Runner->>Runner: Update counters
end
end
end
Runner->>Logger: Log RunSummary
Runner-->>CLI: RunSummary
CLI-->>CLI: Print JSON summary + exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
liquidator/src/cli.ts (1)
50-53: Log stack traces on fatal failures to improve incident triage.Line 50–53 currently emit only the message, which drops useful context when diagnosing production issues.
🛠️ Proposed improvement
main().catch((error) => { - const message = error instanceof Error ? error.message : String(error); - console.error(`[liquidator] Fatal: ${message}`); + if (error instanceof Error) { + console.error(`[liquidator] Fatal: ${error.message}`); + if (error.stack) { + console.error(error.stack); + } + } else { + console.error(`[liquidator] Fatal: ${String(error)}`); + } process.exit(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@liquidator/src/cli.ts` around lines 50 - 53, The fatal error handler for main() currently logs only error.message; update the catch to log the full stack (or the Error object) to preserve diagnostic context: inside main().catch((error) => { ... }) build a log that includes error instanceof Error ? error.stack || error.message : String(error) (or simply console.error(error)) before calling process.exit(1), so the stack trace is emitted for triage while keeping the existing exit behavior.liquidator/.env.example (1)
2-9: Optional: reorder env keys to satisfy dotenv-linter warnings.Line 2–9 ordering triggers the current linter warnings. Reordering keeps CI/lint output clean.
🧹 Proposed reorder
# Testnet runtime -LIQUIDATOR_TESTNET_RPC_URL=https://public-node.testnet.rsk.co LIQUIDATOR_TESTNET_FACTORY_ADDRESS=0x52C37e8364290F3A5f293D6D4ef9852B2d7D0542 LIQUIDATOR_TESTNET_PRIVATE_KEY=REPLACE_WITH_TESTNET_PRIVATE_KEY +LIQUIDATOR_TESTNET_RPC_URL=https://public-node.testnet.rsk.co # Mainnet runtime -LIQUIDATOR_MAINNET_RPC_URL=https://public-node.rsk.co LIQUIDATOR_MAINNET_FACTORY_ADDRESS=0x0000000000000000000000000000000000000000 LIQUIDATOR_MAINNET_PRIVATE_KEY=REPLACE_WITH_MAINNET_PRIVATE_KEY +LIQUIDATOR_MAINNET_RPC_URL=https://public-node.rsk.co🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@liquidator/.env.example` around lines 2 - 9, Reorder the environment variable entries so they satisfy dotenv-linter ordering rules: within each section (Testnet and Mainnet) sort the keys alphabetically (e.g., LIQUIDATOR_TESTNET_FACTORY_ADDRESS, LIQUIDATOR_TESTNET_PRIVATE_KEY, LIQUIDATOR_TESTNET_RPC_URL for Testnet; LIQUIDATOR_MAINNET_FACTORY_ADDRESS, LIQUIDATOR_MAINNET_PRIVATE_KEY, LIQUIDATOR_MAINNET_RPC_URL for Mainnet) and keep the separating comment lines intact; update the block containing LIQUIDATOR_TESTNET_* and LIQUIDATOR_MAINNET_* keys accordingly so the linter warnings disappear.liquidator/src/types.ts (1)
61-74: ExtractRunSummary.perNetworkinline object into a named type.Line 67–74 uses an inline structural type that’s likely reused in tests/formatters. Naming it improves readability and reduces drift risk.
♻️ Proposed refactor
export type RunSummary = { + // consider placing near other report types + // export type RunSummaryPerNetwork = { ... } activeRings: number; delinquentTargetsFound: number; dryRun: boolean; maxTxPerRun: number; networks: NetworkKey[]; - perNetwork: Array<{ - activeRings: number; - delinquentTargetsFound: number; - network: NetworkKey; - ringsScanned: number; - txFailed: number; - txSucceeded: number; - }>; + perNetwork: RunSummaryPerNetwork[]; ringsScanned: number; skippedByCap: number; txAttempted: number; txFailed: number; txSucceeded: number; }; + +export type RunSummaryPerNetwork = { + activeRings: number; + delinquentTargetsFound: number; + network: NetworkKey; + ringsScanned: number; + txFailed: number; + txSucceeded: number; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@liquidator/src/types.ts` around lines 61 - 74, Extract the inline object used in RunSummary.perNetwork into a new exported named type (e.g., PerNetworkSummary or NetworkRunSummary), replace the inline definition in RunSummary with perNetwork: PerNetworkSummary[] and export the new type so tests/formatters can import it; update any local references to the inline shape (including tests/formatters) to use the new type name and ensure the new type includes the same fields: activeRings, delinquentTargetsFound, network, ringsScanned, txFailed, txSucceeded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@liquidator/.env.example`:
- Around line 4-9: The example env contains syntactically valid-looking private
keys (LIQUIDATOR_TESTNET_PRIVATE_KEY and LIQUIDATOR_MAINNET_PRIVATE_KEY);
replace their values with clearly non-usable placeholders (e.g.,
<YOUR_PRIVATE_KEY_HERE> or PRIVATE_KEY_PLACEHOLDER) so they cannot be
accidentally used, and keep the existing variable names
(LIQUIDATOR_TESTNET_PRIVATE_KEY, LIQUIDATOR_MAINNET_PRIVATE_KEY) so consumers
know where to insert real keys.
In `@liquidator/README.md`:
- Around line 20-29: The README currently misstates the config requirements and
env var names; update the listed environment variables and explanatory text to
match loadRuntimeConfig() behavior: make LIQUIDATOR_*_PRIVATE_KEY optional for
non-live/test runs and required only for live runs, and correct the cap variable
name to LIQUIDATOR_MAX_TX_PER_RUN (not MAX_TX_PER_RUN). Locate the sections
around the bullet list and lines 86-90 and change the wording to indicate which
keys are required for live runs only and that LIQUIDATOR_MAX_TX_PER_RUN (default
5) is the optional cap.
In `@liquidator/src/config.ts`:
- Line 1: isValidPrivateKey currently only checks hex format/length and lets
invalid secp256k1 scalars (e.g. 0x00...00) pass; move real validation earlier by
attempting to derive a signer/account from the key and rejecting on failure.
Update isValidPrivateKey to convert the hex key and call the same
account-derivation used by createViemGateway/privateKeyToAccount (or call
privateKeyToAccount directly) inside a try/catch and return false on any error
(and true only if account creation succeeds), so invalid scalars are rejected
before runtime.
In `@liquidator/src/runner.ts`:
- Around line 141-145: The flattened candidate list is biased because
scanNetwork/scanNetworks appends full per-network candidate arrays then
applyTxCap slices the combined list; change the logic to enforce the
transaction-cap per network before flattening (or interleave candidates across
networks). Concretely, inside the loop that calls scanNetwork (referencing
scanNetwork and the perNetwork/report.candidates variables) trim
report.candidates to the per-network cap computed from applyTxCap or refactor
applyTxCap to accept and enforce caps per-network (and consider
selectedNetworks("both") ordering). Ensure candidates.push only receives the
already-capped arrays so a backlog on one network cannot consume all global
slots.
- Around line 49-59: The call to gateway.getAllRings(network) is outside the
scan's error isolation so any RPC failure can abort the whole scan; wrap the
getAllRings(...) call and subsequent getAddress mapping in a try/catch (or move
it inside the existing try block used for the scan) so failures are caught
per-network, log the error (with context including the network) and set rings =
[] (and ringsScanned = 0) so the NetworkScanReport can still be returned;
reference the gateway.getAllRings, getAddress and the NetworkScanReport creation
so you modify that code path only.
---
Nitpick comments:
In `@liquidator/.env.example`:
- Around line 2-9: Reorder the environment variable entries so they satisfy
dotenv-linter ordering rules: within each section (Testnet and Mainnet) sort the
keys alphabetically (e.g., LIQUIDATOR_TESTNET_FACTORY_ADDRESS,
LIQUIDATOR_TESTNET_PRIVATE_KEY, LIQUIDATOR_TESTNET_RPC_URL for Testnet;
LIQUIDATOR_MAINNET_FACTORY_ADDRESS, LIQUIDATOR_MAINNET_PRIVATE_KEY,
LIQUIDATOR_MAINNET_RPC_URL for Mainnet) and keep the separating comment lines
intact; update the block containing LIQUIDATOR_TESTNET_* and
LIQUIDATOR_MAINNET_* keys accordingly so the linter warnings disappear.
In `@liquidator/src/cli.ts`:
- Around line 50-53: The fatal error handler for main() currently logs only
error.message; update the catch to log the full stack (or the Error object) to
preserve diagnostic context: inside main().catch((error) => { ... }) build a log
that includes error instanceof Error ? error.stack || error.message :
String(error) (or simply console.error(error)) before calling process.exit(1),
so the stack trace is emitted for triage while keeping the existing exit
behavior.
In `@liquidator/src/types.ts`:
- Around line 61-74: Extract the inline object used in RunSummary.perNetwork
into a new exported named type (e.g., PerNetworkSummary or NetworkRunSummary),
replace the inline definition in RunSummary with perNetwork: PerNetworkSummary[]
and export the new type so tests/formatters can import it; update any local
references to the inline shape (including tests/formatters) to use the new type
name and ensure the new type includes the same fields: activeRings,
delinquentTargetsFound, network, ringsScanned, txFailed, txSucceeded.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec41fb71-13ae-40d7-b354-6bf470a92e09
⛔ Files ignored due to path filters (1)
liquidator/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.gitignoreliquidator/.env.exampleliquidator/README.mdliquidator/package.jsonliquidator/src/abi.tsliquidator/src/chains.tsliquidator/src/cli-options.tsliquidator/src/cli.tsliquidator/src/config.tsliquidator/src/logger.tsliquidator/src/parse-utils.tsliquidator/src/runner.tsliquidator/src/types.tsliquidator/src/viem-gateway.tsliquidator/test/runner.test.tsliquidator/tsconfig.jsonpackage.json
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.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores