Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 44 minutes and 16 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughA new auto-merchant-bot service has been added to automatically process bridge trades. The implementation includes GitHub Actions CI/CD, complete bot application with API client, cross-chain (EVM/Stellar) authentication and locking mechanisms, plus backend trade-filtering enhancements to support the bot's query requirements. Changes
Sequence DiagramsequenceDiagram
participant Bot as Auto-Merchant-Bot
participant API as Backend API
participant AuthChain as Auth Chain (EVM)
participant TradeChain as Trade Chain (EVM/Stellar)
Bot->>Bot: Load config, init EVM & Stellar credentials
Bot->>API: POST /v1/auth/challenge (EVM)
API-->>Bot: Challenge payload
Bot->>AuthChain: Sign with SIWE
AuthChain-->>Bot: Signed message + signature
Bot->>API: POST /v1/auth/login (SIWE)
API-->>Bot: JWT tokens
Bot->>API: POST /v1/auth/challenge (Stellar)
API-->>Bot: Challenge transaction XDR
Bot->>Bot: Sign with Stellar keypair
Bot->>API: POST /v1/auth/link (Stellar signed)
API-->>Bot: Link response
loop Poll every POLL_INTERVAL_MS
Bot->>API: GET /v1/trades/all (status=ACTIVE)
API-->>Bot: List of active trades
alt Per trade (with per-chain gating)
Bot->>API: POST /v1/trades/{id}/lock
API-->>Bot: Lock response + on-chain params
alt EVM
Bot->>TradeChain: lockForOrder (contract call)
TradeChain-->>Bot: txHash
else Stellar
Bot->>TradeChain: lock_for_order (Soroban call)
TradeChain-->>Bot: txHash (after polling)
end
Bot->>API: POST /v1/trades/{id}/confirm (txHash)
API-->>Bot: Confirm response
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![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 (1)
apps/backend-relayer/src/providers/viem/viem.service.ts (1)
68-91:⚠️ Potential issue | 🟠 MajorFix incompatible use of
evmRpcApiKeyand update deprecated Polygon endpoint.The code uses the same
env.evmRpcApiKeyfor incompatible provider authentication schemes:
- GetBlock (Sepolia): requires network-specific tokens embedded in the URL (format:
https://go.getblock.io/<TOKEN>/)- Alchemy (Polygon Amoy, Optimism Sepolia): requires generic API keys
A single env var cannot satisfy both. When a key type mismatches a provider,
rpc_urlremains empty and silently falls back to the default publichttp()transport, causing rate-limited/unreliable service without obvious errors.Additionally:
- Line 71: GetBlock URL is missing the trailing slash — should be
https://go.getblock.io/${env.evmRpcApiKey}/- Line 85: The endpoint
polygon-mumbai.g.alchemy.comis deprecated (April 2024) — must usepolygon-amoy.g.alchemy.comRecommend introducing per-provider env vars (e.g.,
EVM_RPC_GETBLOCK_TOKEN,EVM_RPC_ALCHEMY_KEY) or full per-chain RPC URLs as already done for Hedera.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend-relayer/src/providers/viem/viem.service.ts` around lines 68 - 91, The sepolia/polygonAmoy/optimismSepolia selection logic incorrectly reuses env.evmRpcApiKey for incompatible providers and uses a deprecated Polygon endpoint and missing trailing slash; update the mapping so sepolia uses a GetBlock-specific env (e.g., EVM_RPC_GETBLOCK_TOKEN or a full RPC URL) and polygonAmoy/optimismSepolia use an Alchemy-specific env (e.g., EVM_RPC_ALCHEMY_KEY or full RPC URL) instead of env.evmRpcApiKey; ensure the GetBlock URL includes the trailing slash (`https://go.getblock.io/${GETBLOCK_TOKEN}/`) and change the Polygon Alchemy host to polygon-amoy.g.alchemy.com, and add explicit checks that set rpc_url only when the provider-specific env is present (otherwise log/error and avoid silently falling back to public http()) in the branch handling sepolia, polygonAmoy, and optimismSepolia.
🧹 Nitpick comments (5)
apps/auto-merchant-bot/src/config.ts (1)
14-15: Inconsistent validation betweenSIGN_DOMAINandSIGN_URI.
SIGN_URIis validated as URL whileSIGN_DOMAINaccepts any string. If these feed SIWE/EIP-4361 signing, a malformed domain will surface as an on-chain/signature verification failure rather than a clear config error. Consider a regex or non-empty constraint onSIGN_DOMAIN.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/auto-merchant-bot/src/config.ts` around lines 14 - 15, SIGN_DOMAIN is currently overly permissive compared to SIGN_URI; tighten its validation in the config schema (the zod object with SIGN_DOMAIN and SIGN_URI) to reject empty or malformed domains. Change SIGN_DOMAIN from z.string().optional() to something like z.string().nonempty().regex(<domainRegex>).optional() (or remove .optional() if you require it), using a suitable domain regex (or a hostname validator) so domain values that feed SIWE/EIP-4361 are validated consistently alongside SIGN_URI.apps/auto-merchant-bot/package.json (1)
7-10:mainandbinpoint at.tssources.
main: "src/index.ts"andbin.auto-merchant-bot: "bin/cli.ts"only work because the package is private and always executed viatsx(dev) or the DockerfileCMD ["node", "dist/bin/cli.js"]. If anything ever resolves this package via Node's module loader (e.g. another workspace importing it, ornpm link), both entries will fail. Consider pointing them at the built artifacts underdist/once the package leaves its internal-only state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/auto-merchant-bot/package.json` around lines 7 - 10, The package.json currently points runtime entries at TypeScript sources ("main": "src/index.ts" and "bin.auto-merchant-bot": "bin/cli.ts"); update these to point at the built JS artifacts (e.g. "main": "dist/index.js" and "bin.auto-merchant-bot": "dist/bin/cli.js") so Node’s module loader and npm consumers can resolve the package outside the private/tsx workflow, and ensure the build step outputs those dist files before publishing.apps/auto-merchant-bot/src/run.ts (1)
96-130: Poll loop has no error backoff or shutdown signal handling.On repeated failures (backend 5xx, DNS flap, etc.) the loop retries every
POLL_INTERVAL_MSwith no jitter or exponential backoff, which can hammer the backend during outages. Additionally,while (true)never observesSIGTERM/SIGINT, so container stops rely on force-kill; in-flighthandleTradepromises (including awaitedtx.wait()/Stellar polling) get terminated mid-flight.Consider adding a simple exponential backoff on the catch branch and an
AbortController/signal flag flipped byprocess.on('SIGTERM', …)that the loop respects before the nextsleep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/auto-merchant-bot/src/run.ts` around lines 96 - 130, The poll loop (while (true) with listTrades, sleep(cfg.POLL_INTERVAL_MS), handleTrade, inflight, gates) must respect shutdown and avoid tight retries: add an AbortController/signal that is set on process.on('SIGTERM'|'SIGINT') and checked at the top of each tick (and before sleeping) so the loop breaks and the controller.signal is passed into handleTrade so in-flight operations can abort; implement an exponential backoff with jitter in the catch branch (e.g., backoffMillis that doubles up to a max and resets on success) instead of immediately sleeping the fixed cfg.POLL_INTERVAL_MS, and ensure backoff is cleared when a successful poll occurs and that gates[kind].busy/inflight cleanup still runs in finally.apps/auto-merchant-bot/src/api/client.ts (1)
106-128: Minor: refresh-token fallback treats transient failures as session loss.The inner
try { await this.doRefresh(...) } catch { this.clearTokens() }swallows every refresh error — including network timeouts / 5xx — and then falls through to the fullreauthFn()(SIWE + SEP-10 + new signatures). That's safe but wasteful on flaky networks and obscures the real error.Consider only clearing tokens on 401/400 from the refresh endpoint, and re-throwing transport errors so the caller sees the original failure instead of a cascading re-auth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/auto-merchant-bot/src/api/client.ts` around lines 106 - 128, In recoverAuth, change the inner catch around await this.doRefresh(this.refresh) so it inspects the refresh error instead of swallowing all errors: if the error indicates a 400/401 response from the refresh endpoint then call this.clearTokens() and continue to run this.reauthFn(), otherwise re-throw the original error so transport/timeouts/5xx bubble up to the caller; keep the surrounding finally that sets this.reauthInFlight = null and keep setTokens(this.setTokens) on success. Reference recoverAuth, doRefresh, clearTokens, reauthFn, reauthInFlight and setTokens when implementing the conditional error handling.apps/auto-merchant-bot/src/chain/evm.ts (1)
14-22: Redundant provider/chainId validation and resource leak on every trade.
run.tsalready validates the chainId once at startup (line 47) and constructs aJsonRpcProvider+Walletavailable inhandleTrade(line 139). However,evmLockForOrderre-creates aJsonRpcProvideron every call (line 14), callsgetNetwork()again (line 15), and never callsprovider.destroy(), leaving polling timers and network connections open in this long-running bot.Pass
evmWalletdirectly toevmLockForOrderinstead ofrpcUrl/privateKey, eliminating the need to re-validate chainId or recreate provider/wallet per trade. Alternatively, addprovider.destroy()in afinallyblock before returning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/auto-merchant-bot/src/chain/evm.ts` around lines 14 - 22, The evmLockForOrder function recreates a JsonRpcProvider and Wallet on every call and re-checks chainId, leaking provider resources; update evmLockForOrder to accept the already-constructed evmWallet (or a Wallet+provider pair) from run.ts/handleTrade instead of rpcUrl/privateKey so it no longer calls new JsonRpcProvider() or getNetwork(), or if you must keep creating a provider ensure you call provider.destroy() in a finally block before returning; adjust call sites in run.ts/handleTrade to pass evmWallet and remove redundant chainId validation inside evmLockForOrder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-merchant-bot-deploy.yml:
- Around line 69-81: The curl invocation uses the -f flag which causes curl to
exit on HTTP >=400 and prevents capturing the response and appended http_code;
remove the -f from the curl options in the block that sets response (leave -sS
or adjust flags as desired) so response, body and status variables are
populated, and keep the existing status check using response/body/status and the
if [ "${status}" -ge 400 ] branch to emit the ::error::Dokploy returned HTTP
${status} and exit 1; update only the curl options in that block (the line that
assigns response=$(curl ...)) to drop -f.
In `@apps/auto-merchant-bot/.env.example`:
- Around line 21-26: The STELLAR_NETWORK_PASSPHRASE value is unquoted and may be
mis-parsed by some dotenv/shell implementations, and the SIWE comment is
truncated/has an unclosed parenthesis; fix by quoting the passphrase value for
STELLAR_NETWORK_PASSPHRASE (e.g., wrap the entire passphrase in double quotes)
and complete/close the SIWE comment text and its parenthesis so the comment
reads as a full sentence (refer to the STELLAR_NETWORK_PASSPHRASE env var and
the SIWE envelope comment block).
In `@apps/auto-merchant-bot/Dockerfile`:
- Around line 10-23: The runtime stage in the Dockerfile runs as root; add the
non-root user before the container start by inserting USER node in the runtime
stage just prior to CMD to run the process as the provided node user (affects
the runtime stage where ENV NODE_ENV=production, WORKDIR
/app/apps/auto-merchant-bot and CMD ["node","dist/bin/cli.js"] are declared);
ensure the USER node line is placed after copying artifacts and before CMD so
the runtime executes under the node user.
In `@apps/auto-merchant-bot/src/api/auth.ts`:
- Around line 100-113: The declared passphrase parameter in loginStellar is
misleading because it is ignored (void passphrase) and the actual network is
chosen from the server challenge in signStellar; either remove the passphrase
parameter from loginStellar (and all callers) to avoid confusion, or wire it
through: add an expectedPassphrase argument to loginStellar and pass it into
signStellar, then update signStellar to compare the challenge's
c.networkPassphrase against expectedPassphrase and throw a clear error on
mismatch before signing; reference the loginStellar and signStellar functions
and the challenge field c.networkPassphrase when making the change.
In `@apps/auto-merchant-bot/src/chain/scval.ts`:
- Around line 6-22: Remove the duplicate function by keeping a single
validator/decoder (pick one name, e.g., hex32OnlyToBuffer) that uses HEX32_RE
and Buffer.from(hex.slice(2), "hex"); delete the other (hex32ToBuffer). Update
all callers (hex32ToContractId, bytesN, any other usages) to call the remaining
function and ensure the error message on validation is the desired one; keep
StrKey.encodeContract(hex32OnlyToBuffer(hex)) in hex32ToContractId as before.
In `@apps/auto-merchant-bot/src/chain/stellar.ts`:
- Around line 63-71: When handling FAILED in the loop that calls
server.getTransaction (checking rpc.Api.GetTransactionStatus in the function
that uses sent.hash), include the diagnostic payload from the response in the
thrown Error instead of dropping it; specifically attach got.resultXdr directly
(it is already a base64 string) and avoid calling .toXDR("base64") on it. Update
the throw in the FAILED branch to include got.resultXdr (and optionally a short
JSON fallback of the response if resultXdr is missing) so operators get the
TransactionResult payload for debugging.
In `@apps/auto-merchant-bot/src/config.ts`:
- Around line 18-21: DRY_RUN currently defaults to true via DRY_RUN:
z.enum(["0","1","true","false"]).default("1").transform(...), which causes the
bot to run in dry-run mode unless explicitly overridden; either change the safe
default to "0" (update the .default(...) on DRY_RUN) so production requires
explicit opt-in to dry-run, or if keeping the current default, add a clear
inline comment next to DRY_RUN and a README note calling out the production
expectation and required env values so operators won't accidentally leave real
locking disabled; ensure the change is applied to the DRY_RUN definition and any
associated documentation.
In `@apps/auto-merchant-bot/src/run.ts`:
- Around line 158-181: When an on-chain lock succeeds but confirmTrade(trade.id,
...) fails, persist a retry record (e.g., a longer-lived in-memory map
retryConfirmMap: Map<tradeId, txHash>) right after successful evmLockForOrder or
stellarLockForOrder and before calling confirmTrade; ensure run()/handleTrade
checks retryConfirmMap at start and, if present, skips the on-chain lock path
and only retries confirmTrade with backoff until it succeeds, and only remove
the entry once confirmTrade returns success (do not clear inflight/gate until
confirmTrade is confirmed or the retry record is moved to a durable store), so
reference evmLockForOrder, stellarLockForOrder, confirmTrade, run()/handleTrade,
inflight and the chain gate when implementing this change.
---
Outside diff comments:
In `@apps/backend-relayer/src/providers/viem/viem.service.ts`:
- Around line 68-91: The sepolia/polygonAmoy/optimismSepolia selection logic
incorrectly reuses env.evmRpcApiKey for incompatible providers and uses a
deprecated Polygon endpoint and missing trailing slash; update the mapping so
sepolia uses a GetBlock-specific env (e.g., EVM_RPC_GETBLOCK_TOKEN or a full RPC
URL) and polygonAmoy/optimismSepolia use an Alchemy-specific env (e.g.,
EVM_RPC_ALCHEMY_KEY or full RPC URL) instead of env.evmRpcApiKey; ensure the
GetBlock URL includes the trailing slash
(`https://go.getblock.io/${GETBLOCK_TOKEN}/`) and change the Polygon Alchemy
host to polygon-amoy.g.alchemy.com, and add explicit checks that set rpc_url
only when the provider-specific env is present (otherwise log/error and avoid
silently falling back to public http()) in the branch handling sepolia,
polygonAmoy, and optimismSepolia.
---
Nitpick comments:
In `@apps/auto-merchant-bot/package.json`:
- Around line 7-10: The package.json currently points runtime entries at
TypeScript sources ("main": "src/index.ts" and "bin.auto-merchant-bot":
"bin/cli.ts"); update these to point at the built JS artifacts (e.g. "main":
"dist/index.js" and "bin.auto-merchant-bot": "dist/bin/cli.js") so Node’s module
loader and npm consumers can resolve the package outside the private/tsx
workflow, and ensure the build step outputs those dist files before publishing.
In `@apps/auto-merchant-bot/src/api/client.ts`:
- Around line 106-128: In recoverAuth, change the inner catch around await
this.doRefresh(this.refresh) so it inspects the refresh error instead of
swallowing all errors: if the error indicates a 400/401 response from the
refresh endpoint then call this.clearTokens() and continue to run
this.reauthFn(), otherwise re-throw the original error so transport/timeouts/5xx
bubble up to the caller; keep the surrounding finally that sets
this.reauthInFlight = null and keep setTokens(this.setTokens) on success.
Reference recoverAuth, doRefresh, clearTokens, reauthFn, reauthInFlight and
setTokens when implementing the conditional error handling.
In `@apps/auto-merchant-bot/src/chain/evm.ts`:
- Around line 14-22: The evmLockForOrder function recreates a JsonRpcProvider
and Wallet on every call and re-checks chainId, leaking provider resources;
update evmLockForOrder to accept the already-constructed evmWallet (or a
Wallet+provider pair) from run.ts/handleTrade instead of rpcUrl/privateKey so it
no longer calls new JsonRpcProvider() or getNetwork(), or if you must keep
creating a provider ensure you call provider.destroy() in a finally block before
returning; adjust call sites in run.ts/handleTrade to pass evmWallet and remove
redundant chainId validation inside evmLockForOrder.
In `@apps/auto-merchant-bot/src/config.ts`:
- Around line 14-15: SIGN_DOMAIN is currently overly permissive compared to
SIGN_URI; tighten its validation in the config schema (the zod object with
SIGN_DOMAIN and SIGN_URI) to reject empty or malformed domains. Change
SIGN_DOMAIN from z.string().optional() to something like
z.string().nonempty().regex(<domainRegex>).optional() (or remove .optional() if
you require it), using a suitable domain regex (or a hostname validator) so
domain values that feed SIWE/EIP-4361 are validated consistently alongside
SIGN_URI.
In `@apps/auto-merchant-bot/src/run.ts`:
- Around line 96-130: The poll loop (while (true) with listTrades,
sleep(cfg.POLL_INTERVAL_MS), handleTrade, inflight, gates) must respect shutdown
and avoid tight retries: add an AbortController/signal that is set on
process.on('SIGTERM'|'SIGINT') and checked at the top of each tick (and before
sleeping) so the loop breaks and the controller.signal is passed into
handleTrade so in-flight operations can abort; implement an exponential backoff
with jitter in the catch branch (e.g., backoffMillis that doubles up to a max
and resets on success) instead of immediately sleeping the fixed
cfg.POLL_INTERVAL_MS, and ensure backoff is cleared when a successful poll
occurs and that gates[kind].busy/inflight cleanup still runs in finally.
🪄 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: cb0b171b-7748-4722-a7bb-1927f526863d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.github/workflows/auto-merchant-bot-deploy.ymlapps/auto-merchant-bot/.env.exampleapps/auto-merchant-bot/.gitignoreapps/auto-merchant-bot/Dockerfileapps/auto-merchant-bot/README.mdapps/auto-merchant-bot/bin/cli.tsapps/auto-merchant-bot/docker-compose.yamlapps/auto-merchant-bot/package.jsonapps/auto-merchant-bot/src/api/auth.tsapps/auto-merchant-bot/src/api/client.tsapps/auto-merchant-bot/src/api/trades.tsapps/auto-merchant-bot/src/chain/ad-manager-abi.tsapps/auto-merchant-bot/src/chain/address.tsapps/auto-merchant-bot/src/chain/evm.tsapps/auto-merchant-bot/src/chain/scval.tsapps/auto-merchant-bot/src/chain/stellar.tsapps/auto-merchant-bot/src/config.tsapps/auto-merchant-bot/src/index.tsapps/auto-merchant-bot/src/logger.tsapps/auto-merchant-bot/src/run.tsapps/auto-merchant-bot/tsconfig.jsonapps/backend-relayer/src/modules/trades/dto/trade.dto.tsapps/backend-relayer/src/modules/trades/trade.service.tsapps/backend-relayer/src/providers/viem/viem.service.tsapps/frontend/types/trades.ts
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
Release Notes
New Features
Infrastructure & Deployment