App updates#72
Merged
Merged
Conversation
|
Warning Rate limit exceeded
⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds TCP reachability testing and a taker liquidity IPC exposed to the renderer, pins the coinswap-ffi repo to a branch, adds an execution plan doc, and implements a broad UI/UX overhaul across market, receive/addresses, wallet (transactions/UTXOs), swap flows, settings/first-time-setup, recovery, styles, and logs. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer (UI)
participant Preload as Preload (contextBridge)
participant Main as Main (ipcMain)
participant Net as Network (net.Socket)
Renderer->>Preload: api.testTcpPort({host,port,timeout})
Preload->>Main: invoke 'network:testTcpPort' with config
Main->>Net: create socket, socket.connect(port, host)
alt connect success
Net-->>Main: 'connect'
Main-->>Renderer: { success: true, host, port }
else error or timeout
Net-->>Main: 'error' or timeout
Main-->>Renderer: { success: false, host, port, error }
end
Note over Main,Net: socket destroyed and single resolution enforced
sequenceDiagram
participant Renderer as Renderer (UI)
participant Preload as Preload (contextBridge)
participant Main as Main (ipcMain)
participant Taker as TakerInstance
Renderer->>Preload: api.taker.checkSwapLiquidity()
Preload->>Main: invoke 'taker:checkSwapLiquidity'
Main->>Taker: ensure takerInstance exists, call getBalances()
alt taker has checkSwapLiquidity fn
Taker-->>Main: balances + checkSwapLiquidity() result (various shapes)
else
Taker-->>Main: balances only
end
Main->>Main: normalize spendable/regular/swap and compute maxSwappable (apply fallback of max(regular,swap)-3000)
Main-->>Renderer: { success: true, liquidity: { spendable, regular, swap, maxSwappable } } or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/FirstTimeSetup.js (1)
900-932:⚠️ Potential issue | 🟠 MajorRedact secrets before logging the built config.
Lines 920-928 place the RPC password, wallet password, and Tor auth password into
config, and Line 932 logs that object verbatim. That leaks credentials into renderer/devtools logs during setup.Suggested fix
- console.log('✅ Configuration built:', config); + console.log('✅ Configuration built:', { + ...config, + rpc: { + ...config.rpc, + password: config.rpc.password ? '[redacted]' : '', + }, + taker: { + ...config.taker, + tor_auth_password: config.taker.tor_auth_password + ? '[redacted]' + : undefined, + }, + wallet: { + ...config.wallet, + password: config.wallet.password ? '[redacted]' : '', + }, + });As per coding guidelines:
Never log sensitive data (passwords, private keys).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/FirstTimeSetup.js` around lines 900 - 932, The console.log prints the full config object including sensitive fields (rpc.password, wallet.password, taker.tor_auth_password) — create a sanitized copy of config before logging by removing or redacting those fields (e.g., omit rpc.password, wallet.password or replace with '[REDACTED]') and log that sanitizedConfig instead of config; update the code around the config construction and the console.log call to reference the sanitized version while leaving the original config intact for use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api1.js`:
- Around line 1347-1403: The network:testTcpPort handler currently accepts
arbitrary config.host and unvalidated config.port; update the
ipcMain.handle('network:testTcpPort') implementation to enforce loopback-only
probes and strict port validation: force host to a loopback value (e.g., allow
only '127.0.0.1' and '::1' or reject other hosts) before creating the socket,
parse the incoming port with parseInt and verify Number.isInteger(portNum) and 1
<= portNum <= 65535, and return the { success: false, error: 'Port is required'
}-style response (or a clear validation error) when parsing fails; finally call
socket.connect(portNum, host) using the validated numeric port and keep existing
finish/settled/socket.destroy logic around socket.connect and its event handlers
(symbols: ipcMain.handle('network:testTcpPort'), port/host variables,
socket.connect, finish, settled).
In `@execution.md`:
- Line 11: The proposed change incorrectly labels satoshi amounts with the
bitcoin symbol ₿; update the implementation so any conversion to satoshis
(remove or replace calls to satsToBtc and the new formatSats suggestion) uses a
correct unit label "sat" or "sats" instead of "₿". Locate usages in UtxoList.js
(satsToBtc), SwapReport.js (satsToBtc), SwapHistory.js (satsToBtc),
TransactionsList.js (formatAmount), Wallet.js, Send.js, Receive.js and Market.js
and replace display formatting with a function (e.g., formatSats) that returns
sats.toLocaleString() + ' sats' (or ' sat' for singular) and ensure any removed
satsToBtc logic is not converting units elsewhere.
In `@preload.js`:
- Line 75: The IPC bridge methods in preload.js (all handlers e.g., testTcpPort)
currently return raw ipcRenderer.invoke promises; change each to an async
function that awaits ipcRenderer.invoke inside a try/catch, returning a
normalized object like { success: true, data } on success and { success: false,
error: errorMessage } on failure; specifically update the testTcpPort entry (and
the other handlers between lines 11–75) to use async/await, wrap the invoke call
in try/catch, extract/serialize the error (e.g., error.message), and return the
normalized { success, error } shape.
In `@setup-coinswap.js`:
- Around line 24-26: The git clone currently uses runCommand(`git clone -b
${BRANCH} ${REPO_URL}`) which clones into the current working directory instead
of the intended FFI_DIR; update the clone invocation so it clones explicitly
into FFI_DIR (e.g. include ${FFI_DIR} as the target directory in the git clone
command) so NAPI_SOURCE (used later in the script) will be present under the
expected path; locate the runCommand call that references BRANCH and REPO_URL
and change it to pass the FFI_DIR as the clone destination (or run the command
with cwd set to __dirname/FFI_DIR).
- Around line 21-32: The script currently uses the mutable BRANCH constant and
runCommand to track a moving branch; replace BRANCH with an immutable identifier
(e.g., COMMIT_SHA or SIGNED_TAG) and update logic in the FFI_DIR handling to
always checkout that immutable ref: when cloning (runCommand) clone the repo
then git checkout the COMMIT_SHA (or clone with --branch if using a signed tag),
and when the repo already exists use runCommand('git fetch --all', { cwd:
FFI_DIR }) followed by runCommand(`git checkout ${COMMIT_SHA}`, { cwd: FFI_DIR
}) and runCommand(`git reset --hard ${COMMIT_SHA}`, { cwd: FFI_DIR }) instead of
tracking the branch; keep REPO_URL and FFI_DIR as-is and update only the BRANCH
symbol usage to the new COMMIT_SHA/TAG constant so builds become reproducible.
In `@src/components/market/Market.js`:
- Around line 513-524: The modal currently hardcodes mempool.space for bondTx
links (see the maker.bondTxid block and the
window.open('https://mempool.space/tx/${maker.bondTxid}', '_blank') call);
change it to use the same network-aware explorer helper used elsewhere (the code
used in UtxoList.js and AddressList.js) — e.g. call the existing function or
variable that builds explorer URLs (like getExplorerUrl or explorerBase + path)
to open the correct network-specific tx URL (e.g.
window.open(getExplorerUrl(maker.bondTxid, 'tx'), '_blank')) so the link
respects the current network.
- Around line 317-318: The call to window.api.taker.getProtocol() inside
initialize is not wrapped in try/catch and can reject, causing initialize to
abort; update initialize (or the surrounding async block) to await
window.api.taker.getProtocol() inside a try/catch, capture and log the error
(e.g., using console.error or the component logger), and ensure on failure you
set a safe default (null or fallback protocol) so the existing fallback/error
flow can continue rather than aborting initialization.
In `@src/components/receive/Receive.js`:
- Around line 145-166: detectAddressType was extended to accept a
fallbackSpendType but callers in Receive.js still call
detectAddressType(address) with no fallback, and generateNewAddress ignores the
IPC-provided result.addressType; update all local calls in this component (e.g.,
where generateNewAddress and detectAddressType are invoked) to pass the
IPC/address source type through (use the result.addressType or the component's
spend-type prop/state as the fallbackSpendType argument) so the fallback path is
used; follow the pattern in AddressList.js for wiring the fallback/type through
to detectAddressType and ensure generateNewAddress uses the exact addressType
returned by the IPC API instead of re-detecting without the fallback.
In `@src/components/recovery/Recovery.js`:
- Around line 48-52: Update the user-facing copy in the Recovery component:
replace the paragraph text "Recovery will resume in next restart" with a
grammatically correct phrase such as "Recovery will resume on the next restart",
and replace "Always ensure to not have very old pending recoveries. That can put
your funds at risk." with a clearer sentence like "Avoid having very old pending
recoveries, as they can put your funds at risk." Locate and edit these strings
in the JSX paragraphs inside the Recovery component in Recovery.js.
In `@src/components/settings/FirstTimeSetup.js`:
- Around line 288-290: The Wallet creation UI currently allows an empty
password; update FirstTimeSetup.js so creating a new wallet requires a non-empty
password: add client-side validation in the create wallet flow (e.g., in the
form submit handler such as handleCreateWallet or onSubmit/onCreate) to block
submission when the password input is empty, show a validation error message
near the "Wallet Password" field, and change the helper text so it no longer
suggests an empty password creates an unencrypted wallet; ensure the same
validation is applied to the other related create path referenced around the
other block (lines ~792-799) to enforce encryption by default.
- Around line 612-614: The ZMQ address is hardcoded to 127.0.0.1 in
getZmqAddress(), causing ZMQ checks and generated config to ignore the
user-specified RPC/host; change getZmqAddress to accept a host (e.g.,
hostOrRpcHost) and build the address with that host instead of 127.0.0.1, then
update all call sites (including the node test that performs the TCP ZMQ check
and any config-generation code) to pass the chosen RPC/host value so
remote/containerized bitcoind hosts are used for ZMQ checks and config
generation (ensure references to getZmqAddress, the node ZMQ test function, and
the config write/generation functions are updated consistently).
In `@src/components/wallet/TransactionsList.js`:
- Around line 289-297: The current totalSwaps calculation (using
allTransactions.filter(...) and getTransactionType) counts swap-classified
wallet transactions, not unique coinswaps; either rename the metric to something
like "Swap Transactions" to reflect it's counting transactions, or change the
source to count distinct swaps by grouping transactions by a swap identifier
from your swap reports (e.g., use swapReports or swaps array if available) and
compute unique swap IDs instead of counting transactions; update the place that
renders the "Total Swaps" card to use the new metric name or the deduplicated
swap count and replace references to totalSwaps accordingly.
In `@src/components/wallet/UtxoList.js`:
- Around line 151-153: The spendable filter is using getSpendTypeDisplay labels
instead of the canonical utxo.spendable flag; update the logic where spendable
UTXOs are counted/filtered (e.g., the spendableCount calculation that filters
allUtxos and the similar block referenced at lines 172-177) to use
utxo.spendable (e.g., !!u.spendable) as the source of truth rather than
comparing getSpendTypeDisplay(u.spendInfo?.spendType); ensure all other
occurrences that determine "Spendable" status use the utxo.spendable property.
In `@src/styles/output.css`:
- Around line 1234-1236: The stylesheet contains a compiled rule (.leading-none
{ --tw-leading: 1; line-height: 1; }) that triggers the Stylelint rule
declaration-empty-line-before; fix by either (A) regenerating the compiled CSS
from the Tailwind source so the generated output no longer violates linting, or
(B) stop linting the compiled artifact by updating the stylelint config to
ignore the generated file (or add a file-level disable comment) so lint runs
only against source CSS; locate the offending rule by searching for the
.leading-none selector and the declaration-empty-line-before violation and apply
one of these two fixes.
---
Outside diff comments:
In `@src/components/settings/FirstTimeSetup.js`:
- Around line 900-932: The console.log prints the full config object including
sensitive fields (rpc.password, wallet.password, taker.tor_auth_password) —
create a sanitized copy of config before logging by removing or redacting those
fields (e.g., omit rpc.password, wallet.password or replace with '[REDACTED]')
and log that sanitizedConfig instead of config; update the code around the
config construction and the console.log call to reference the sanitized version
while leaving the original config intact for use.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9cb4b74b-9db1-46b6-bcdc-b664b55c0d6e
📒 Files selected for processing (13)
api1.jsexecution.mdpreload.jssetup-coinswap.jssrc/components/market/Market.jssrc/components/receive/AddressList.jssrc/components/receive/Receive.jssrc/components/recovery/Recovery.jssrc/components/settings/FirstTimeSetup.jssrc/components/wallet/TransactionsList.jssrc/components/wallet/UtxoList.jssrc/components/wallet/Wallet.jssrc/styles/output.css
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/swap/SwapReport.js (1)
37-38:⚠️ Potential issue | 🟠 MajorKeep a fallback when
totalFundingTxsis absent.Older reports won't have this new field, so the summary card now renders
0even whenfundingTxidsByHopormakersCountis present. Derive the count during normalization instead of treating the new field as required.Also applies to: 655-660
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/swap/SwapReport.js` around lines 37 - 38, The code currently sets totalFundingTxs from swapReport.totalFundingTxs or swapReport.total_funding_txs with a fallback to 0, which causes older reports to show 0 even when funding data exists; update the normalization logic (the block assigning totalFundingTxs) to compute a fallback count when the explicit field is missing by deriving it from available structures such as fundingTxidsByHop (sum lengths of arrays/sets per hop) or makersCount (use makersCount when fundingTxidsByHop is absent), i.e., if swapReport.totalFundingTxs and swapReport.total_funding_txs are both falsy, compute totalFundingTxs = sum over Object.values(swapReport.fundingTxidsByHop).map(length) or fallback to swapReport.makersCount, then use that computed value instead of 0.
♻️ Duplicate comments (1)
src/styles/output.css (1)
1263-1266:⚠️ Potential issue | 🟠 MajorStylelint blocker is still present in generated CSS.
declaration-empty-line-beforeis still violated at Line 1265 and Line 1293, so CI remains red. Since this is generated Tailwind output, fix this at the source generation step or exclude this compiled artifact from Stylelint checks.Also applies to: 1291-1294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/output.css` around lines 1263 - 1266, The generated CSS contains an empty line before the declaration in the .leading-none block which violates the declaration-empty-line-before rule; either change the generation step so the compiler/emitter no longer emits an empty line before declarations for utility blocks like .leading-none, or exclude the compiled artifact from Stylelint by adding an ignore (e.g., stylelintIgnorePatterns for generated CSS) or inserting a top-of-file directive to disable the rule for the generated output (e.g., a generated-file-only /* stylelint-disable declaration-empty-line-before */), and ensure the fix applies to all similar blocks (e.g., the other instance around the same utility).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/settings/Settings.js`:
- Around line 356-377: renderConnectionResults currently interpolates
result.label and result.message directly into innerHTML, allowing HTML/JS
injection from untrusted RPC input (see testBitcoindConnection). Fix it by
avoiding direct innerHTML for user-controlled strings: build the DOM nodes via
document.createElement and set their textContent (or run strings through a
strict HTML-escaping utility) before appending to resultDiv; update the code
paths in renderConnectionResults that reference result.label/result.message to
use the safe text-setting approach so no raw HTML is ever injected.
In `@src/components/swap/Swap.js`:
- Around line 424-429: The SwapReportComponent call is only passing
result.report.report so top-level metadata (swapId, amount, status, timestamps)
is dropped; change the props to include the top-level report object too — e.g.
spread result.report first and then spread result.report.report (or vice‑versa
if nested keys should be overridden) and keep the explicit overrides for
protocol/protocolVersion/isTaproot; update the call at
module.SwapReportComponent(...) to pass {...result.report,
...result.report.report, protocol: result.report.protocol ?? 'v1', isTaproot:
result.report.isTaproot ?? false, protocolVersion: result.report.protocolVersion
?? 1} so swapId/amount/status/timestamps reach SwapReportComponent.
- Around line 880-885: The warning text in Swap.js currently says "Recommended
minimum hop = 2" which implies a 1-maker swap (unsafe); update the message in
the warning block (the JSX that renders the yellow alert in Swap.js) to
recommend a safe configuration—either state "Recommended minimum makers = 2" or
"Recommended minimum hops = 3" (so it no longer directs users to a
1-maker/unsafe setup) and ensure the displayed rationale still matches the
corrected recommendation.
- Around line 294-315: fetchSwapLiquidity uses walletBalances as a fallback
before balances are guaranteed, causing maxSwappableAmount to remain 0 if
checkSwapLiquidity fails; modify fetchSwapLiquidity to ensure balances are
loaded first by awaiting the existing balance fetcher (e.g., call/await
fetchBalance() or a shared ensureBalancesLoaded helper) before computing the
fallback, then compute maxSwappableAmount from walletBalances.regular/swap minus
the buffer; update the same pattern at the other occurrences (around the
references at 1343-1348 and 1392-1395) to avoid using uninitialized
walletBalances.
In `@src/components/swap/SwapHistory.js`:
- Around line 225-237: The current code spreads only result.report.report into
fullReport, discarding top-level metadata (swapId, status, amount, timestamps)
returned by window.api.swapReports.get; update the construction of fullReport in
the row.addEventListener click handler so it merges the top-level result.report
first and then overlays result.report.report (e.g., { ...result.report,
...result.report.report, protocol: ..., isTaproot: ..., protocolVersion: ... })
before calling module.SwapReportComponent(container, fullReport) to ensure
identifiers and summary fields are preserved.
---
Outside diff comments:
In `@src/components/swap/SwapReport.js`:
- Around line 37-38: The code currently sets totalFundingTxs from
swapReport.totalFundingTxs or swapReport.total_funding_txs with a fallback to 0,
which causes older reports to show 0 even when funding data exists; update the
normalization logic (the block assigning totalFundingTxs) to compute a fallback
count when the explicit field is missing by deriving it from available
structures such as fundingTxidsByHop (sum lengths of arrays/sets per hop) or
makersCount (use makersCount when fundingTxidsByHop is absent), i.e., if
swapReport.totalFundingTxs and swapReport.total_funding_txs are both falsy,
compute totalFundingTxs = sum over
Object.values(swapReport.fundingTxidsByHop).map(length) or fallback to
swapReport.makersCount, then use that computed value instead of 0.
---
Duplicate comments:
In `@src/styles/output.css`:
- Around line 1263-1266: The generated CSS contains an empty line before the
declaration in the .leading-none block which violates the
declaration-empty-line-before rule; either change the generation step so the
compiler/emitter no longer emits an empty line before declarations for utility
blocks like .leading-none, or exclude the compiled artifact from Stylelint by
adding an ignore (e.g., stylelintIgnorePatterns for generated CSS) or inserting
a top-of-file directive to disable the rule for the generated output (e.g., a
generated-file-only /* stylelint-disable declaration-empty-line-before */), and
ensure the fix applies to all similar blocks (e.g., the other instance around
the same utility).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 85922db1-ddf9-4a71-8c84-04e07af1d7d8
📒 Files selected for processing (8)
api1.jspreload.jssrc/components/log/Log.jssrc/components/settings/Settings.jssrc/components/swap/Swap.jssrc/components/swap/SwapHistory.jssrc/components/swap/SwapReport.jssrc/styles/output.css
💤 Files with no reviewable changes (1)
- src/components/log/Log.js
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/swap/Swap.js (1)
204-233:⚠️ Potential issue | 🟠 MajorRe-filter makers when the real protocol arrives.
The page filters offers with
currentProtocolbefore it asks the backend for the active protocol, then updatescurrentProtocolwithout recomputingavailableMakers. If local state is stale or the cached list came from a different protocol, the maker count, summary, and fee math stay wrong until a full refresh.As per coding guidelines, "Multi-hop routing for swaps".
Also applies to: 1443-1450
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/swap/Swap.js` around lines 204 - 233, The code maps goodMakers into availableMakers and applies protocol filtering using the local currentProtocol before the backend-sourced active protocol is known, so when currentProtocol is later updated the list stays stale; fix by separating the protocol-specific filtering into a reusable step and re-applying it after you obtain/update the real protocol from window.api.taker.getOffers (or after you set currentProtocol), e.g., compute the full availableMakers list from goodMakers first (using the mapping logic shown), then filter that list based on currentProtocol (using the same Taproot vs Legacy condition) whenever currentProtocol is set/updated so availableMakers, maker counts and fee summaries always reflect the currentProtocol.
♻️ Duplicate comments (4)
src/components/settings/Settings.js (1)
448-450:⚠️ Potential issue | 🔴 CriticalDon't interpolate config fields into preview
innerHTML.
rpcUser,rpcPass, and the derived endpoint strings are written straight into the preview panes withinnerHTML. A crafted value from the form or persisted config can execute script inside a renderer that already exposes privileged IPC APIs. Build both previews withtextContent/ DOM nodes (or strict escaping) instead.As per coding guidelines, "RPC credential management security".
Also applies to: 454-496
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/Settings.js` around lines 448 - 450, The preview construction is vulnerable because Settings.js writes user-supplied RPC fields and endpoint strings directly into innerHTML (see zmqPreview, rawblock, rawtx and the RPC fields rpcUser, rpcPass and derived endpoint strings), so replace all innerHTML-based preview rendering with safe text-only rendering: build the preview output using textContent or createTextNode/DOM elements for each line (or apply strict escaping) instead of injecting HTML, and ensure any existing blocks that set innerHTML (including the similar code around lines 454-496) are updated to use the same text-safe rendering approach.src/styles/output.css (1)
1263-1265:⚠️ Potential issue | 🟡 MinorThis generated CSS still fails the active Stylelint rule.
declaration-empty-line-beforeis still triggered on.leading-noneand.tracking-wide, so CI will keep failing until this file is regenerated or excluded from lint.Also applies to: 1291-1293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles/output.css` around lines 1263 - 1265, The generated stylesheet still violates the Stylelint rule `declaration-empty-line-before` for the classes `.leading-none` and `.tracking-wide`; fix by regenerating the CSS from the source (re-run your build/preprocessor that emits src/styles/output.css) so the generated output conforms to the updated style rules, or update lint config to ignore this generated file (or specifically disable the `declaration-empty-line-before` rule for generated CSS) so CI stops failing; locate the offending rules in the generated file by searching for `.leading-none` and `.tracking-wide` to verify the regenerated/ignored file resolves the failure.api1.js (1)
1401-1457:⚠️ Potential issue | 🟠 MajorKeep the TCP probe loopback-only and validate the port before
socket.connect().This handler still accepts arbitrary hosts and forwards
config.portstraight intosocket.connect(). That keeps a generic TCP probing primitive exposed to the renderer and still allows malformed port values to fail outside the intended{ success, error }path.As per coding guidelines, "Validate all user inputs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api1.js` around lines 1401 - 1457, The TCP probe must be restricted to loopback and validate ports before calling socket.connect: in the ipcMain.handle('network:testTcpPort', ...) handler, enforce host to loopback only (accept only '127.0.0.1' or '::1' or else override/return an error) and validate config.port by parsing to an integer (use parseInt/Number.isInteger) and ensure it's in the 1–65535 range; if invalid, call the same resolve path (finish) with { success: false, host, port, error: 'Invalid port' }. Also ensure you only call socket.connect(...) with the validated numeric port and loopback host (do not pass user-provided host directly). Use the existing finish/settled/socket.destroy flow for all early returns so behavior stays consistent.src/components/swap/Swap.js (1)
683-687:⚠️ Potential issue | 🟠 MajorA zero
maxSwappableAmountcurrently disables the over-limit check.The
&& maxSwappableAmount > 0guard means a zero-balance wallet—or a still-loading liquidity check—can enter any positive amount without tripping validation, so the start button stays enabled until the backend rejects it.Based on learnings, "Show loading states during async operations" and "Provide clear error messages to users".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/swap/Swap.js` around lines 683 - 687, The current over-limit check in Swap.js (using swapAmount, maxSwappableAmount, and warnings) incorrectly skips validation when maxSwappableAmount is 0; remove the `&& maxSwappableAmount > 0` guard so the check always runs (e.g., if swapAmount > maxSwappableAmount push the warning), and additionally surface a distinct warning or loading state when maxSwappableAmount is null/undefined or a special loader flag (e.g., isMaxSwappableLoading) is true so the UI disables the start button while liquidity is loading; update any UI logic that enables the start action to consider these warnings/loading flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/swap/Swap.js`:
- Around line 488-490: The UI currently treats availableMakers.slice(0,
getNumberOfMakers()) as the "selected" route (function getSelectedMakers), which
is inaccurate because the backend chooses the actual route and fees; update the
UI to either (A) stop labeling the first N offers as "Selected Makers" by
renaming getSelectedMakers to getTopCandidateMakers (or similar) and change all
UI labels that rely on it, or (B) prefer the authoritative route returned by the
backend (e.g., use the swap/route response field such as route, selectedMakers
or fees) to populate the selected-makers list and fee estimate; specifically
modify getSelectedMakers, any code paths that compute the fee estimate, and
components that render the "Selected Makers" list so they either use the
backend-provided route object or are relabeled as "Top N candidates" until the
backend route is available. Ensure fee estimation logic (where fees are derived)
reads from the backend route/fees rather than availableMakers.slice(...).
- Around line 309-316: The code uses || when copying liquidity fields
(maxSwappableAmount and walletBalances.spendable/regular/swap) which treats 0 as
falsy and keeps stale cached values; change those fallbacks to nullish checks
(use ??) so actual zero balances from data.liquidity override previous
values—update the assignments for maxSwappableAmount and the properties in
walletBalances where data.liquidity.spendable/regular/swap are used.
In `@src/components/swap/SwapHistory.js`:
- Around line 225-239: The click handler on each row currently calls
window.api.swapReports.get(swapId) and dynamically imports './SwapReport.js'
without error handling; wrap the entire handler body (the IPC call and the
dynamic import/usage of module.SwapReportComponent) in a try-catch, use await
for the import (e.g., const module = await import('./SwapReport.js')) or attach
a .catch to handle import errors, and on any failure update the UI (container)
to show an error message or clear loading state; reference the row click
listener, window.api.swapReports.get, import('./SwapReport.js'), container and
fullReport when making these changes and mirror the try-catch pattern used by
loadSwapHistory().
In `@src/components/swap/SwapReport.js`:
- Around line 427-438: getProtocolInfoLines() currently always returns
V2-specific text; change it to branch on the actual swap protocol (use
report.protocol or an isV2Swap boolean passed in/derived) and return
protocol-appropriate HTML (V2 copy for Musig2/HTLC and V1/neutral copy for
legacy) so legacy reports don't show V2 claims. Update the getProtocolInfoLines
implementation to accept or access report.protocol/isV2Swap and render the two
alternative blocks (or a neutral comparison) accordingly; also apply the same
fix to the other similar block around the second occurrence (the section noted
at lines ~619-625) to keep both places consistent.
---
Outside diff comments:
In `@src/components/swap/Swap.js`:
- Around line 204-233: The code maps goodMakers into availableMakers and applies
protocol filtering using the local currentProtocol before the backend-sourced
active protocol is known, so when currentProtocol is later updated the list
stays stale; fix by separating the protocol-specific filtering into a reusable
step and re-applying it after you obtain/update the real protocol from
window.api.taker.getOffers (or after you set currentProtocol), e.g., compute the
full availableMakers list from goodMakers first (using the mapping logic shown),
then filter that list based on currentProtocol (using the same Taproot vs Legacy
condition) whenever currentProtocol is set/updated so availableMakers, maker
counts and fee summaries always reflect the currentProtocol.
---
Duplicate comments:
In `@api1.js`:
- Around line 1401-1457: The TCP probe must be restricted to loopback and
validate ports before calling socket.connect: in the
ipcMain.handle('network:testTcpPort', ...) handler, enforce host to loopback
only (accept only '127.0.0.1' or '::1' or else override/return an error) and
validate config.port by parsing to an integer (use parseInt/Number.isInteger)
and ensure it's in the 1–65535 range; if invalid, call the same resolve path
(finish) with { success: false, host, port, error: 'Invalid port' }. Also ensure
you only call socket.connect(...) with the validated numeric port and loopback
host (do not pass user-provided host directly). Use the existing
finish/settled/socket.destroy flow for all early returns so behavior stays
consistent.
In `@src/components/settings/Settings.js`:
- Around line 448-450: The preview construction is vulnerable because
Settings.js writes user-supplied RPC fields and endpoint strings directly into
innerHTML (see zmqPreview, rawblock, rawtx and the RPC fields rpcUser, rpcPass
and derived endpoint strings), so replace all innerHTML-based preview rendering
with safe text-only rendering: build the preview output using textContent or
createTextNode/DOM elements for each line (or apply strict escaping) instead of
injecting HTML, and ensure any existing blocks that set innerHTML (including the
similar code around lines 454-496) are updated to use the same text-safe
rendering approach.
In `@src/components/swap/Swap.js`:
- Around line 683-687: The current over-limit check in Swap.js (using
swapAmount, maxSwappableAmount, and warnings) incorrectly skips validation when
maxSwappableAmount is 0; remove the `&& maxSwappableAmount > 0` guard so the
check always runs (e.g., if swapAmount > maxSwappableAmount push the warning),
and additionally surface a distinct warning or loading state when
maxSwappableAmount is null/undefined or a special loader flag (e.g.,
isMaxSwappableLoading) is true so the UI disables the start button while
liquidity is loading; update any UI logic that enables the start action to
consider these warnings/loading flags.
In `@src/styles/output.css`:
- Around line 1263-1265: The generated stylesheet still violates the Stylelint
rule `declaration-empty-line-before` for the classes `.leading-none` and
`.tracking-wide`; fix by regenerating the CSS from the source (re-run your
build/preprocessor that emits src/styles/output.css) so the generated output
conforms to the updated style rules, or update lint config to ignore this
generated file (or specifically disable the `declaration-empty-line-before` rule
for generated CSS) so CI stops failing; locate the offending rules in the
generated file by searching for `.leading-none` and `.tracking-wide` to verify
the regenerated/ignored file resolves the failure.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4263f020-48c4-46c7-9d7b-0d5f71024a0c
📒 Files selected for processing (8)
api1.jspreload.jssrc/components/log/Log.jssrc/components/settings/Settings.jssrc/components/swap/Swap.jssrc/components/swap/SwapHistory.jssrc/components/swap/SwapReport.jssrc/styles/output.css
💤 Files with no reviewable changes (1)
- src/components/log/Log.js
Comment on lines
+427
to
+438
| function getProtocolInfoLines() { | ||
| return ` | ||
| <div> | ||
| <p class="mb-1"><strong>Save Money:</strong> Lesser Fees than V1 swaps.</p> | ||
| <p><strong>Efficient:</strong> Combined tapscript with Musig2 + HTLC leaves.</p> | ||
| </div> | ||
| <div> | ||
| <p class="mb-1"><strong>Anonymity Set — Legacy:</strong> All P2WSH UTXOs.</p> | ||
| <p><strong>Anonymity Set — Taproot:</strong> All Taproot Single Sig UTXOs.</p> | ||
| </div> | ||
| `; | ||
| } |
There was a problem hiding this comment.
Make the protocol-details block depend on the actual swap protocol.
getProtocolInfoLines() always renders V2-specific copy (“Lesser Fees than V1”, “Musig2 + HTLC leaves”), so legacy reports show technically wrong details. Branch on report.protocol / isV2Swap, or switch this section to neutral comparison copy.
Also applies to: 619-625
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/swap/SwapReport.js` around lines 427 - 438,
getProtocolInfoLines() currently always returns V2-specific text; change it to
branch on the actual swap protocol (use report.protocol or an isV2Swap boolean
passed in/derived) and return protocol-appropriate HTML (V2 copy for Musig2/HTLC
and V1/neutral copy for legacy) so legacy reports don't show V2 claims. Update
the getProtocolInfoLines implementation to accept or access
report.protocol/isV2Swap and render the two alternative blocks (or a neutral
comparison) accordingly; also apply the same fix to the other similar block
around the second occurrence (the section noted at lines ~619-625) to keep both
places consistent.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/settings/Settings.js (2)
445-496:⚠️ Potential issue | 🟡 MinorKeep the regtest preview consistent with the actual template.
The preview now reuses the user-editable RPC port for
[regtest], so the default signet settings make the on-screen reference showrpcport=38332. The copy-to-clipboard template still hardcodes18442, so the two sources disagree.Also applies to: 657-701
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/Settings.js` around lines 445 - 496, The regtest preview in fullPreview.innerHTML is showing a user-editable rpc port (via rpcPort) that disagrees with the copy-to-clipboard template which hardcodes 18442; update the regtest block in fullPreview.innerHTML to use the same canonical default as the copy template (replace rpcport=${rpcPort} with rpcport=18442 or reference a single shared constant like rpcPortDefault) so both the on-screen preview and the clipboard template remain consistent; change the occurrence inside fullPreview.innerHTML (and the similar block around lines 657-701) to use the same source of truth.
747-793:⚠️ Potential issue | 🟠 MajorAdd abort timeouts to the RPC and REST fetch calls.
The test flow uses
Promise.allSettled()to wait on two RPC calls and a REST fetch with no timeout mechanism. On a blackholed host/port, these promises will hang indefinitely, leaving the button stuck in the "Testing..." state and blocking the UI. UseAbortControllerwith a reasonable timeout (e.g., 10 seconds) to prevent users from being stuck.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/Settings.js` around lines 747 - 793, makeRPCCall currently waits indefinitely on fetch; add an AbortController-based timeout (e.g., 10s) to cancel the request and avoid UI hangs: inside makeRPCCall create an AbortController, start a timer (setTimeout) that calls controller.abort() after 10_000ms, pass controller.signal to fetch, clear the timer after fetch completes, and catch aborts to throw a clear timeout error (e.g., "RPC request timed out"). Apply the same pattern to the REST fetch function(s) used in this settings module so both RPC and REST calls use AbortController timeouts and surface user-friendly timeout errors.
♻️ Duplicate comments (3)
api1.js (1)
1401-1457:⚠️ Potential issue | 🟠 MajorRestrict
network:testTcpPortto loopback and validate the port beforesocket.connect().This handler takes renderer-controlled
host/portand passes them straight tonet.Socket, so the setup helper can be used as a generic TCP probe. Invalid or out-of-range ports can also throw/reject before you return{ success: false, error }, which breaks the API contract.As per coding guidelines,
**/*.js: "Validate all user inputs";{main.js,api1.js,preload.js}: "Return { success: boolean, error?: string } from API handlers".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api1.js` around lines 1401 - 1457, The network:testTcpPort IPC handler currently forwards renderer-controlled host/port directly into net.Socket and calls socket.connect without validating inputs or restricting host; update the handler to only allow loopback addresses (e.g., '127.0.0.1' and '::1') or normalize unspecified host to loopback, validate that port is a number within 1–65535 (reject non-numeric or out-of-range values), and perform these checks before calling socket.connect; also wrap socket.connect in a try/catch (or otherwise handle synchronous exceptions) and include actual error details in the resolved { success: false, error } responses so the API contract ({ success: boolean, error?: string }) is always preserved.src/components/swap/Swap.js (2)
507-520:⚠️ Potential issue | 🟠 MajorFee estimation is still tied to a guessed route.
Renaming the list to “Top Maker Candidates” fixed the label, but
calculateFees()still prices the swap fromavailableMakers.slice(0, getNumberOfMakers()). The backend chooses the actual route later, so the displayed maker fee, total fee, receive amount, and the manual-mode amount derived from those fees can still be materially wrong. Use backend-selected route/fee data for these calculations, or keep this estimate out of validation until the real route exists.
As per coding guidelines, "Fee estimation accuracy for Bitcoin transactions" and "Multi-hop routing for swaps".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/swap/Swap.js` around lines 507 - 520, calculateFees currently estimates maker and network fees using the UI-side candidate list (getTopCandidateMakers / getNumberOfHops) which can differ from the backend-selected route; update calculateFees to prefer the backend-provided route/fee data (e.g., a backendRoute or selectedRoute object returned by the swap API) for makerFee, networkFee, hops and refund locktimes, and only fall back to getTopCandidateMakers/getNumberOfHops when no backend route exists; also stop using availableMakers.slice(0, getNumberOfMakers()) or similar UI-only lists for validation and manual-mode amount calculations—use the backend route fees or mark the estimate as provisional until the real route is available.
895-900:⚠️ Potential issue | 🟡 MinorThe warning still recommends the unsafe hop count.
2 hopsmeans1 maker, which is the exact case this banner says can deanonymize the user. The recommendation should be3 hops (2 makers)or2 makers minimum.💡 Minimal text fix
- Warning: If swap with only one maker, the maker can deanonymize you. Recommended minimum hop = 2. + Warning: A single-maker swap can deanonymize you. Recommended minimum: 3 hops (2 makers).As per coding guidelines, "Multi-hop routing for swaps".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/swap/Swap.js` around lines 895 - 900, In the Swap component's warning banner (the text string starting with "Warning: If swap with only one maker..."), update the recommendation to avoid recommending "2 hops"—change it to "3 hops (2 makers)" or explicitly state "minimum 2 makers" so the banner reads that at least 2 makers (3 hops) are recommended; edit the message in Swap.js accordingly to replace "Recommended minimum hop = 2." with the corrected phrasing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/swap/Swap.js`:
- Around line 103-117: The estimator still uses a hardcoded mainnet
mempool.space endpoint; update fetchNetworkFees() to select the
mempool/mempool-like endpoint from the active network/backend (use the
currentNetwork and/or currentProtocol variables) instead of always calling
mainnet, and ensure all places that compute total fee, ETA and receive amount
use the fetched rates from fetchNetworkFees() (replace hardcoded mempool.space
usage in fee calculation blocks and any calls that compute
totalFee/receiveAmount/maxSwappableAmount so they read network-aware rates).
Ensure fetchNetworkFees() builds the URL from currentNetwork mapping
(signet/testnet/mainnet) or delegates to the configured backend, and propagate
its result to the functions that compute fees/ETA.
- Around line 388-429: Wrap the await loadSwapHistory() call inside
renderSwapHistorySection() in a try/catch so failures don't abort the swap
screen: catch errors from loadSwapHistory() and on error log/handle it, set
swapHistory = [] (or a safe default) and compute stats =
summarizeSwapHistory(swapHistory) (or a default stats object), then render an
inline error/empty state into swapHistoryStats and swapHistoryContainer via
buildSwapHistoryMarkup so the main swap view continues to load; ensure
subsequent code that attaches click handlers to elements from
swapHistoryContainer only runs when rows exist and still references
viewSwapReport(swapId) safely.
- Around line 114-119: The code is loading currentProtocol from localStorage
before fetching makers, which can be stale; call the authoritative
window.api.taker.getProtocol() (or await taker.getProtocol()) before the initial
fetchMakers() so currentProtocol reflects the wallet, update currentProtocol
from that result, then re-run fetchMakers() or refilter the offerbook whenever
the protocol value changes; apply the same change to the other initialization
spots referenced (around the blocks identified by the
currentProtocol/localStorage logic at lines ~214-233 and the later
reconciliation at ~1443-1447) so makers, candidate lists, and fee estimates are
always computed against the authoritative protocol.
- Around line 683-687: The warning logic currently skips when maxSwappableAmount
=== 0 causing a positive swapAmount to bypass validation; update the check in
the Swap component so that the warning about exceeding swappable balance always
runs (remove the "&& maxSwappableAmount > 0" guard) and instead rely on a
separate "balancesLoaded" or similar flag to avoid startup flicker; ensure you
update references to swapAmount, maxSwappableAmount and warnings to reflect the
always-validated condition and add or use an existing balancesLoaded boolean to
gate UI enabling (e.g., "Start Coinswap") rather than suppressing the warning
when maxSwappableAmount is zero.
In `@src/components/swap/SwapHistory.js`:
- Around line 72-82: loadSwapHistory currently hides real failures by returning
[] on both IPC errors and on { success: false } responses; change it so failures
are surfaced instead of collapsed: in loadSwapHistory (and where you call
window.api.swapReports.getAll) detect when result.success is false and throw or
return a failure object containing the error message/details from the IPC reply
(include the returned error text if present), and also rethrow or propagate any
caught exceptions instead of swallowing them; preserve successful behavior by
mapping result.reports via normalizeSwapReport only when result.success is true.
Update callers (components like SwapHistory.js and other listed files) to handle
the thrown error or the failure object and display a clear error message to the
user rather than showing the empty-state UI.
In `@src/components/swap/SwapReport.js`:
- Around line 454-483: The layout fixes: currently the grid uses a fixed class
"md:grid-cols-4" causing extra nodes to wrap and arrows to mis-align; update the
container generation around nodes (the element using md:grid-cols-4) to create a
dynamic number of columns based on nodes.length (or switch to a single-row
flex/inline-grid approach) so the full node list and arrow elements rendered in
the nodes.map stay on one row; adjust the mapping logic that creates the node
blocks (refer to nodes, maker-node, and the arrow SVG blocks) so you compute
columns = nodes.length (or set grid-template-columns: repeat(columns, 1fr)
inline) and ensure the arrow elements remain between nodes without wrapping.
---
Outside diff comments:
In `@src/components/settings/Settings.js`:
- Around line 445-496: The regtest preview in fullPreview.innerHTML is showing a
user-editable rpc port (via rpcPort) that disagrees with the copy-to-clipboard
template which hardcodes 18442; update the regtest block in
fullPreview.innerHTML to use the same canonical default as the copy template
(replace rpcport=${rpcPort} with rpcport=18442 or reference a single shared
constant like rpcPortDefault) so both the on-screen preview and the clipboard
template remain consistent; change the occurrence inside fullPreview.innerHTML
(and the similar block around lines 657-701) to use the same source of truth.
- Around line 747-793: makeRPCCall currently waits indefinitely on fetch; add an
AbortController-based timeout (e.g., 10s) to cancel the request and avoid UI
hangs: inside makeRPCCall create an AbortController, start a timer (setTimeout)
that calls controller.abort() after 10_000ms, pass controller.signal to fetch,
clear the timer after fetch completes, and catch aborts to throw a clear timeout
error (e.g., "RPC request timed out"). Apply the same pattern to the REST fetch
function(s) used in this settings module so both RPC and REST calls use
AbortController timeouts and surface user-friendly timeout errors.
---
Duplicate comments:
In `@api1.js`:
- Around line 1401-1457: The network:testTcpPort IPC handler currently forwards
renderer-controlled host/port directly into net.Socket and calls socket.connect
without validating inputs or restricting host; update the handler to only allow
loopback addresses (e.g., '127.0.0.1' and '::1') or normalize unspecified host
to loopback, validate that port is a number within 1–65535 (reject non-numeric
or out-of-range values), and perform these checks before calling socket.connect;
also wrap socket.connect in a try/catch (or otherwise handle synchronous
exceptions) and include actual error details in the resolved { success: false,
error } responses so the API contract ({ success: boolean, error?: string }) is
always preserved.
In `@src/components/swap/Swap.js`:
- Around line 507-520: calculateFees currently estimates maker and network fees
using the UI-side candidate list (getTopCandidateMakers / getNumberOfHops) which
can differ from the backend-selected route; update calculateFees to prefer the
backend-provided route/fee data (e.g., a backendRoute or selectedRoute object
returned by the swap API) for makerFee, networkFee, hops and refund locktimes,
and only fall back to getTopCandidateMakers/getNumberOfHops when no backend
route exists; also stop using availableMakers.slice(0, getNumberOfMakers()) or
similar UI-only lists for validation and manual-mode amount calculations—use the
backend route fees or mark the estimate as provisional until the real route is
available.
- Around line 895-900: In the Swap component's warning banner (the text string
starting with "Warning: If swap with only one maker..."), update the
recommendation to avoid recommending "2 hops"—change it to "3 hops (2 makers)"
or explicitly state "minimum 2 makers" so the banner reads that at least 2
makers (3 hops) are recommended; edit the message in Swap.js accordingly to
replace "Recommended minimum hop = 2." with the corrected phrasing.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c702d202-8183-4c2f-99cc-83ecc3df0b3a
📒 Files selected for processing (8)
api1.jspreload.jssrc/components/log/Log.jssrc/components/settings/Settings.jssrc/components/swap/Swap.jssrc/components/swap/SwapHistory.jssrc/components/swap/SwapReport.jssrc/styles/output.css
💤 Files with no reviewable changes (1)
- src/components/log/Log.js
Comment on lines
+103
to
+117
| let currentNetwork = 'signet'; | ||
| let walletBalances = { | ||
| spendable: 0, | ||
| regular: 0, | ||
| swap: 0, | ||
| contract: 0, | ||
| fidelity: 0, | ||
| }; | ||
| let maxSwappableAmount = 0; | ||
| let balanceLoadPromise = null; | ||
|
|
||
| try { | ||
| const config = JSON.parse(localStorage.getItem('coinswap_config') || '{}'); | ||
| currentProtocol = config.protocol || currentProtocol; | ||
| currentNetwork = config.network || currentNetwork; |
There was a problem hiding this comment.
currentNetwork is only half-wired into the estimator.
These changes make ETA network-aware, but fetchNetworkFees() still hardcodes the mainnet mempool.space endpoint. On the default signet path, that misprices the network fee, total fee, and receive amount. Pull fee rates from the active network/backend instead of always using mainnet recommendations.
As per coding guidelines, "Fee estimation accuracy for Bitcoin transactions".
Also applies to: 492-495, 507-513
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/swap/Swap.js` around lines 103 - 117, The estimator still uses
a hardcoded mainnet mempool.space endpoint; update fetchNetworkFees() to select
the mempool/mempool-like endpoint from the active network/backend (use the
currentNetwork and/or currentProtocol variables) instead of always calling
mainnet, and ensure all places that compute total fee, ETA and receive amount
use the fetched rates from fetchNetworkFees() (replace hardcoded mempool.space
usage in fee calculation blocks and any calls that compute
totalFee/receiveAmount/maxSwappableAmount so they read network-aware rates).
Ensure fetchNetworkFees() builds the URL from currentNetwork mapping
(signet/testnet/mainnet) or delegates to the configured backend, and propagate
its result to the functions that compute fees/ETA.
This was referenced Mar 25, 2026
Merged
Closed
This was referenced Mar 27, 2026
Merged
Merged
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
UI/UX Changes
Documentation
Chores
Style