switch block explorer to the signet one and fix confirmation count#89
Merged
Conversation
📝 WalkthroughWalkthroughConfiguration updated to ignore macOS files. Multiple transaction viewer endpoints migrated from public services to local HTTP. Wallet refresh logic refactored to remove cache-first decision branches and always perform fresh sync. Market component updated for Tor address formatting, maker address normalization, and grid layout changes. Grid utility added to CSS. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/wallet/Wallet.js (2)
63-82:⚠️ Potential issue | 🟠 MajorRemove raw wallet payload logs.
These logs can expose transaction history, addresses, labels, amounts, and UTXO data in renderer logs. Keep only counts or non-identifying diagnostics.
Proposed fix
- console.log('[REFRESH] raw getTransactions response:', JSON.stringify(data, null, 2)); + console.log('[REFRESH] transactions fetched:', data.transactions?.length ?? 0); @@ - console.log('[REFRESH] raw getUtxos response:', JSON.stringify(data, null, 2)); + console.log('[REFRESH] UTXOs fetched:', data.utxos?.length ?? 0); @@ - console.log('[REFRESH] Complete summary:'); - console.log(' balance:', balance); - console.log(' transactions confirmations:', transactions?.map(t => ({ txid: typeof t.info.txid === 'object' ? t.info.txid.value : t.info.txid, confirmations: t.info.confirmations }))); - console.log(' utxo confirmations:', utxos?.map(u => ({ txid: typeof u.utxo.txid === 'object' ? u.utxo.txid.value : u.utxo.txid, confirmations: u.utxo.confirmations }))); + console.log('[REFRESH] Complete summary:', { + hasBalance: Boolean(balance), + transactionCount: transactions?.length ?? 0, + utxoCount: utxos?.length ?? 0, + });Also applies to: 366-369
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wallet/Wallet.js` around lines 63 - 82, Remove the raw wallet payload console logs that dump full JSON in fetchTransactions and fetchUtxos and replace them with non-identifying diagnostics (e.g., log only counts like number of transactions or UTXOs and success/failure), ensuring you do not print addresses, labels, amounts, or full UTXO objects; also remove or replace any other raw payload logs in this file (the similar JSON dumps referenced around the later wallet refresh code) with the same count/summary-only approach so no sensitive wallet data is written to renderer logs.
262-320:⚠️ Potential issue | 🟠 MajorDo not limit confirmation enrichment to the first 200 transactions.
A still-unspent UTXO can originate from an older transaction outside
getTransactions(200, 0), leaving its stale confirmation count unchanged. Fetch UTXOs first, then page transactions until all UTXO txids are matched, or move live confirmation enrichment into the backendgetUtxosresponse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wallet/Wallet.js` around lines 262 - 320, The current confirmation enrichment only calls window.api.taker.getTransactions(200, 0) which can miss older txids and leave UTXO confirmations stale; change the flow to call fetchUtxos() first, collect the set of UTXO txids, then page through window.api.taker.getTransactions(...) (incrementing the start/skip or page param) building txConfMap until either all UTXO txids are found or no more transactions are returned, and only then perform the enrichment logic that maps txConfMap into rawUtxos (the map/enrich code that currently checks txConfMap.get(txid) and returns updated confirmations); alternatively, move this enrichment into the backend getUtxos implementation if preferred.src/components/market/Market.js (1)
551-613:⚠️ Potential issue | 🟠 MajorDo not interpolate remote bond txids into inline JavaScript.
Maker data comes from the offerbook, so validate the txid as 64-hex and attach a click listener instead of embedding it in an
onclickstring.Proposed fix
+ const safeBondTxid = /^[0-9a-fA-F]{64}$/.test(maker.bondTxid) + ? maker.bondTxid + : ''; + modal.innerHTML = ` @@ - maker.bondTxid + safeBondTxid ? ` <div class="bg-[`#0f1419`] p-4 rounded-lg"> <p class="text-sm text-gray-400 mb-1">Bond Txid</p> <button - onclick="window.open('http://170.75.166.88:8080/tx/${maker.bondTxid}', '_blank')" + type="button" + data-bond-txid="${safeBondTxid}" class="text-cyan-400 hover:text-cyan-300 underline font-mono text-sm break-all text-left w-full" > - ${maker.bondTxid} + ${safeBondTxid} </button> </div> @@ document.body.appendChild(modal); + modal.querySelector('[data-bond-txid]')?.addEventListener('click', (event) => { + const txid = event.currentTarget.dataset.bondTxid; + window.open( + `http://170.75.166.88:8080/tx/${encodeURIComponent(txid)}`, + '_blank', + 'noopener,noreferrer' + ); + }); };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 `@src/components/market/Market.js` around lines 551 - 613, The modal currently interpolates maker.bondTxid directly into an onclick string which is unsafe; instead validate maker.bondTxid is a 64-character hex string, render the txid as plain text (no inline onclick) and after appending the modal attach a DOM click listener to the Bond Txid button that opens the explorer URL (e.g., using window.open) only when validation passes; update the code locations that build the modal HTML (where maker.bondTxid is used) and the post-append logic to perform the hex validation and event listener attachment for maker.bondTxid.src/components/wallet/UtxoList.js (1)
567-580: 🛠️ Refactor suggestion | 🟠 MajorCentralize the explorer URL and encode
txidto avoid duplication and handle special characters.The
window.openTxOnMempoolfunction and hardcoded URLhttp://170.75.166.88:8080are duplicated across at least three files (Wallet.js, UtxoList.js, and TransactionsList.js). Thetxidparameter is also used directly in string interpolation without URL encoding. Extract this into a single helper function that:
- Centralizes the explorer base URL for easier updates
- Encodes the txid with
encodeURIComponent(String(txid))to handle special characters safelyThis will also cover direct URL usages in Send.js and Market.js.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wallet/UtxoList.js` around lines 567 - 580, Extract the hardcoded explorer URL into a shared constant and helper function (e.g., create a function openExplorerTx(txid) or getExplorerUrl(txid)) and replace window.openTxOnMempool with a call to that helper; ensure the helper builds the URL using the centralized base (e.g., EXPLORER_BASE) and encodes the txid via encodeURIComponent(String(txid)), then use the same open logic (require('electron').shell.openExternal(url) with a window.open fallback) so Wallet.js, UtxoList.js, TransactionsList.js (and any direct usages in Send.js/Market.js) reuse the single implementation.
🤖 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/send/Send.js`:
- Around line 784-785: The anchor in Send.js that builds the external
transaction URL should be hardened: add rel="noopener noreferrer" to the <a>
element and ensure the txid path segment is URL-encoded (use
encodeURIComponent(txid) or encodeURIComponent(txid.trim()) where txid is
referenced) when constructing the href (the template that currently uses
${txid}); update the href construction and add the rel attribute on the same <a>
element to prevent tab-napping and broken URLs.
In `@src/components/swap/Swap.js`:
- Around line 520-524: The fetchNetworkFees function should validate the HTTP
response and the returned fee before assigning networkFeeRate: check response.ok
after fetch (and handle non-2xx by using a fallback or throwing), parse
response.json() safely, verify that data.halfHourFee exists and is a finite
positive number (e.g., Number.isFinite(+data.halfHourFee) && +data.halfHourFee >
0) before assigning to networkFeeRate, and otherwise keep the current safe
default or log/throw an error; update references to fetchNetworkFees,
networkFeeRate, and halfHourFee when applying this validation.
In `@src/components/wallet/Wallet.js`:
- Around line 517-522: The current try/catch around window.api.taker.sync()
ignores non-throwing failures; change the call to capture its return (e.g.,
const syncResult = await window.api.taker.sync()), check syncResult.success (or
equivalent) before proceeding to cache refreshed data, and if success is false
log a warning and skip the caching/update path; keep the existing catch to
handle thrown errors but ensure both failure cases (success:false and
exceptions) do not allow stale data to be cached.
---
Outside diff comments:
In `@src/components/market/Market.js`:
- Around line 551-613: The modal currently interpolates maker.bondTxid directly
into an onclick string which is unsafe; instead validate maker.bondTxid is a
64-character hex string, render the txid as plain text (no inline onclick) and
after appending the modal attach a DOM click listener to the Bond Txid button
that opens the explorer URL (e.g., using window.open) only when validation
passes; update the code locations that build the modal HTML (where
maker.bondTxid is used) and the post-append logic to perform the hex validation
and event listener attachment for maker.bondTxid.
In `@src/components/wallet/UtxoList.js`:
- Around line 567-580: Extract the hardcoded explorer URL into a shared constant
and helper function (e.g., create a function openExplorerTx(txid) or
getExplorerUrl(txid)) and replace window.openTxOnMempool with a call to that
helper; ensure the helper builds the URL using the centralized base (e.g.,
EXPLORER_BASE) and encodes the txid via encodeURIComponent(String(txid)), then
use the same open logic (require('electron').shell.openExternal(url) with a
window.open fallback) so Wallet.js, UtxoList.js, TransactionsList.js (and any
direct usages in Send.js/Market.js) reuse the single implementation.
In `@src/components/wallet/Wallet.js`:
- Around line 63-82: Remove the raw wallet payload console logs that dump full
JSON in fetchTransactions and fetchUtxos and replace them with non-identifying
diagnostics (e.g., log only counts like number of transactions or UTXOs and
success/failure), ensuring you do not print addresses, labels, amounts, or full
UTXO objects; also remove or replace any other raw payload logs in this file
(the similar JSON dumps referenced around the later wallet refresh code) with
the same count/summary-only approach so no sensitive wallet data is written to
renderer logs.
- Around line 262-320: The current confirmation enrichment only calls
window.api.taker.getTransactions(200, 0) which can miss older txids and leave
UTXO confirmations stale; change the flow to call fetchUtxos() first, collect
the set of UTXO txids, then page through window.api.taker.getTransactions(...)
(incrementing the start/skip or page param) building txConfMap until either all
UTXO txids are found or no more transactions are returned, and only then perform
the enrichment logic that maps txConfMap into rawUtxos (the map/enrich code that
currently checks txConfMap.get(txid) and returns updated confirmations);
alternatively, move this enrichment into the backend getUtxos implementation if
preferred.
🪄 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: 485f7541-311e-485b-8d02-e7c1303c71f4
📒 Files selected for processing (8)
.gitignoresrc/components/market/Market.jssrc/components/send/Send.jssrc/components/swap/Swap.jssrc/components/wallet/TransactionsList.jssrc/components/wallet/UtxoList.jssrc/components/wallet/Wallet.jssrc/styles/output.css
Comment on lines
+784
to
785
| <a href="http://170.75.166.88:8080/tx/${txid}" target="_blank" | ||
| class="text-sm font-mono text-blue-400 hover:text-blue-300 underline flex-1 truncate"> |
There was a problem hiding this comment.
Harden the external transaction link.
Add rel="noopener noreferrer" and encode the txid path segment before opening a new tab/window.
Proposed fix
- <a href="http://170.75.166.88:8080/tx/${txid}" target="_blank"
+ <a href="http://170.75.166.88:8080/tx/${encodeURIComponent(String(txid))}" target="_blank" rel="noopener noreferrer"
class="text-sm font-mono text-blue-400 hover:text-blue-300 underline flex-1 truncate">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a href="http://170.75.166.88:8080/tx/${txid}" target="_blank" | |
| class="text-sm font-mono text-blue-400 hover:text-blue-300 underline flex-1 truncate"> | |
| <a href="http://170.75.166.88:8080/tx/${encodeURIComponent(String(txid))}" target="_blank" rel="noopener noreferrer" | |
| class="text-sm font-mono text-blue-400 hover:text-blue-300 underline flex-1 truncate"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/send/Send.js` around lines 784 - 785, The anchor in Send.js
that builds the external transaction URL should be hardened: add rel="noopener
noreferrer" to the <a> element and ensure the txid path segment is URL-encoded
(use encodeURIComponent(txid) or encodeURIComponent(txid.trim()) where txid is
referenced) when constructing the href (the template that currently uses
${txid}); update the href construction and add the rel attribute on the same <a>
element to prevent tab-napping and broken URLs.
Comment on lines
520
to
524
| const response = await fetch( | ||
| 'https://mempool.space/api/v1/fees/recommended' | ||
| 'http://170.75.166.88:8080/api/v1/fees/recommended' | ||
| ); | ||
| const data = await response.json(); | ||
| networkFeeRate = data.halfHourFee; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
curl -fsS --max-time 10 'http://170.75.166.88:8080/api/v1/fees/recommended' \
| jq '{halfHourFee, fastestFee, economyFee, minimumFee}'Repository: citadel-tech/taker-app
Length of output: 145
🏁 Script executed:
#!/bin/bash
# Verify the file exists and check the exact lines referenced
if [ -f "src/components/swap/Swap.js" ]; then
echo "=== Lines 515-530 of src/components/swap/Swap.js ==="
sed -n '515,530p' src/components/swap/Swap.js | cat -n
else
echo "File not found at src/components/swap/Swap.js"
# Try alternate patterns
find . -name "Swap.js" -type f 2>/dev/null | head -5
fiRepository: citadel-tech/taker-app
Length of output: 600
🏁 Script executed:
#!/bin/bash
# Get broader context around the fetch call to see error handling
if [ -f "src/components/swap/Swap.js" ]; then
echo "=== Context: Lines 500-550 ==="
sed -n '500,550p' src/components/swap/Swap.js | cat -n
fiRepository: citadel-tech/taker-app
Length of output: 1817
Add validation before assigning the fee response.
The fetchNetworkFees() function (lines 519-527) lacks defensive checks. If the endpoint returns a non-2xx status, non-numeric halfHourFee, or missing field, networkFeeRate becomes undefined and causes NaN in calculations. Add an response.ok check and validate that halfHourFee is a finite positive number before assignment.
Proposed fix
async function fetchNetworkFees() {
try {
const response = await fetch(
'http://170.75.166.88:8080/api/v1/fees/recommended'
);
+ if (!response.ok) {
+ throw new Error(`Fee endpoint returned ${response.status}`);
+ }
const data = await response.json();
- networkFeeRate = data.halfHourFee;
+ const nextFeeRate = Number(data.halfHourFee);
+ if (!Number.isFinite(nextFeeRate) || nextFeeRate <= 0) {
+ throw new Error('Fee endpoint returned invalid halfHourFee');
+ }
+ networkFeeRate = nextFeeRate;
updateSummary();
} catch (error) {
console.error('Failed to fetch network fees:', error);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const response = await fetch( | |
| 'https://mempool.space/api/v1/fees/recommended' | |
| 'http://170.75.166.88:8080/api/v1/fees/recommended' | |
| ); | |
| const data = await response.json(); | |
| networkFeeRate = data.halfHourFee; | |
| async function fetchNetworkFees() { | |
| try { | |
| const response = await fetch( | |
| 'http://170.75.166.88:8080/api/v1/fees/recommended' | |
| ); | |
| if (!response.ok) { | |
| throw new Error(`Fee endpoint returned ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| const nextFeeRate = Number(data.halfHourFee); | |
| if (!Number.isFinite(nextFeeRate) || nextFeeRate <= 0) { | |
| throw new Error('Fee endpoint returned invalid halfHourFee'); | |
| } | |
| networkFeeRate = nextFeeRate; | |
| updateSummary(); | |
| } catch (error) { | |
| console.error('⚠️ Failed to fetch network fees:', error); | |
| } | |
| } |
🤖 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 520 - 524, The fetchNetworkFees
function should validate the HTTP response and the returned fee before assigning
networkFeeRate: check response.ok after fetch (and handle non-2xx by using a
fallback or throwing), parse response.json() safely, verify that
data.halfHourFee exists and is a finite positive number (e.g.,
Number.isFinite(+data.halfHourFee) && +data.halfHourFee > 0) before assigning to
networkFeeRate, and otherwise keep the current safe default or log/throw an
error; update references to fetchNetworkFees, networkFeeRate, and halfHourFee
when applying this validation.
Comment on lines
+517
to
522
| console.log('🔄 Syncing and fetching fresh data...'); | ||
| try { | ||
| await window.api.taker.sync(); | ||
| } catch (syncErr) { | ||
| console.warn('⚠️ Initial wallet sync failed, proceeding anyway:', syncErr.message); | ||
| } |
There was a problem hiding this comment.
Check the initial sync result before caching refreshed data.
window.api.taker.sync() can return { success: false } without throwing; this path currently proceeds silently and can cache stale data.
Proposed fix
console.log('🔄 Syncing and fetching fresh data...');
try {
- await window.api.taker.sync();
+ await syncWalletState();
} catch (syncErr) {
console.warn('⚠️ Initial wallet sync failed, proceeding anyway:', syncErr.message);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/wallet/Wallet.js` around lines 517 - 522, The current
try/catch around window.api.taker.sync() ignores non-throwing failures; change
the call to capture its return (e.g., const syncResult = await
window.api.taker.sync()), check syncResult.success (or equivalent) before
proceeding to cache refreshed data, and if success is false log a warning and
skip the caching/update path; keep the existing catch to handle thrown errors
but ensure both failure cases (success:false and exceptions) do not allow stale
data to be cached.
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
UI Changes
Wallet Improvements
Updates