Skip to content

manual maker selection #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
keraliss merged 2 commits into from
May 13, 2026
Merged

manual maker selection #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9fdc1b3
manual maker selection
keraliss May 8, 2026
a0c1268
making coderabbit happy
keraliss May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions api1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1453,7 +1453,7 @@ function registerCoinswapHandlers() {
// Start coinswap
ipcMain.handle(
'coinswap:start',
async (event, { amount, makerCount, outpoints, password }) => {
async (event, { amount, makerCount, outpoints, password, selectedMakerAddresses }) => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
try {
if (!api1State.takerInstance) {
return { success: false, error: 'Taker not initialized' };
Expand All @@ -1464,6 +1464,14 @@ function registerCoinswapHandlers() {
return { success: false, error: 'Invalid amount' };
}

if (
selectedMakerAddresses != null &&
(!Array.isArray(selectedMakerAddresses) ||
selectedMakerAddresses.some((a) => typeof a !== 'string' || !a.trim()))
) {
return { success: false, error: 'Invalid selectedMakerAddresses: must be an array of non-empty strings' };
}

Comment on lines +1467 to +1474
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Validation looks good; consider also enforcing length and uniqueness.

The added type/shape validation correctly addresses the prior concern. As an optional hardening, the handler does not currently enforce that selectedMakerAddresses.length is at least makerCount and does not deduplicate. A renderer with a stale UI state (or any other IPC caller) could submit fewer unique entries than makerCount, which would then surface as a worker-side failure rather than a clean error here.

♻️ Optional tightening 
         if (
           selectedMakerAddresses != null &&
           (!Array.isArray(selectedMakerAddresses) ||
             selectedMakerAddresses.some((a) => typeof a !== 'string' || !a.trim()))
         ) {
           return { success: false, error: 'Invalid selectedMakerAddresses: must be an array of non-empty strings' };
         }
+
+        if (Array.isArray(selectedMakerAddresses) && selectedMakerAddresses.length > 0) {
+          const unique = new Set(selectedMakerAddresses.map((a) => a.trim()));
+          if (unique.size !== selectedMakerAddresses.length) {
+            return { success: false, error: 'selectedMakerAddresses contains duplicates' };
+          }
+          if (unique.size < makerCount) {
+            return {
+              success: false,
+              error: `Need ${makerCount} makers, only ${unique.size} selected`,
+            };
+          }
+        }
📝 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
if (
selectedMakerAddresses != null &&
(!Array.isArray(selectedMakerAddresses) ||
selectedMakerAddresses.some((a) => typeof a !== 'string' || !a.trim()))
) {
return { success: false, error: 'Invalid selectedMakerAddresses: must be an array of non-empty strings' };
}
if (
selectedMakerAddresses != null &&
(!Array.isArray(selectedMakerAddresses) ||
selectedMakerAddresses.some((a) => typeof a !== 'string' || !a.trim()))
) {
return { success: false, error: 'Invalid selectedMakerAddresses: must be an array of non-empty strings' };
}
if (Array.isArray(selectedMakerAddresses) && selectedMakerAddresses.length > 0) {
const unique = new Set(selectedMakerAddresses.map((a) => a.trim()));
if (unique.size !== selectedMakerAddresses.length) {
return { success: false, error: 'selectedMakerAddresses contains duplicates' };
}
if (unique.size < makerCount) {
return {
success: false,
error: `Need ${makerCount} makers, only ${unique.size} selected`,
};
}
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api1.js` around lines 1467 - 1474, The validator for selectedMakerAddresses
should also enforce that, when provided, the array contains at least makerCount
entries and that entries are unique and non-empty; update the handler logic
around selectedMakerAddresses and makerCount to (1) check
selectedMakerAddresses.length >= makerCount, (2) deduplicate entries (e.g., via
a Set) and ensure the deduplicated count >= makerCount, and (3) return a clear
error like 'selectedMakerAddresses must contain at least {makerCount} unique
non-empty strings' if those checks fail.

const protocol = api1State.protocolVersion || 'v1';
const protocolName = protocol === 'v2' ? 'Taproot' : 'P2WSH';
const swapId = `swap_${Date.now()}_${Math.random().toString(36).substring(7)}`;
Expand Down Expand Up @@ -1571,7 +1579,7 @@ function registerCoinswapHandlers() {
});

const worker = new Worker(path.join(__dirname, 'coinswap-worker.js'), {
workerData: { amount, makerCount, outpoints, config },
workerData: { amount, makerCount, outpoints, selectedMakerAddresses, config },
});

api1State.activeSwaps.set(swapId, {
Expand Down
3 changes: 2 additions & 1 deletion coinswap-worker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { parentPort, workerData } = require('worker_threads');
try {
const coinswapNapi = require('coinswap-napi');

const { amount, makerCount, outpoints, config } = workerData;
const { amount, makerCount, outpoints, selectedMakerAddresses, config } = workerData;
const protocol = config.protocol || 'v1';
const normalizedProtocol = protocol === 'v2' ? 'Taproot' : 'Legacy';
const protocolName =
Expand Down Expand Up @@ -58,6 +58,7 @@ const { parentPort, workerData } = require('worker_threads');
sendAmount: amount,
makerCount: makerCount,
manuallySelectedOutpoints: outpoints || undefined,
preferredMakers: selectedMakerAddresses && selectedMakerAddresses.length > 0 ? selectedMakerAddresses : undefined,
};

console.log(`🔄 Syncing offerbook in swap worker before prepare...`);
Expand Down
27 changes: 10 additions & 17 deletions src/components/market/Market.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,14 @@ export function Market(container) {
}
}

function formatTorEndpoint(address, start = 6, end = 0) {
function formatTorEndpoint(address, start = 8, end = 6) {
if (!address || typeof address !== 'string') return 'unknown';

const separatorIndex = address.lastIndexOf(':');
if (separatorIndex === -1) return address;
const host = (separatorIndex !== -1 ? address.slice(0, separatorIndex) : address).replace(/\.onion$/i, '');

const host = address.slice(0, separatorIndex).replace(/\.onion$/i, '');
const port = address.slice(separatorIndex + 1);

if (host.length <= start + end + 3) {
return `${host}:${port}`;
}

return end > 0 ? `${host.slice(0, start)}..${host.slice(-end)}:${port}` : `${host.slice(0, start)}..:${port}`;
if (host.length <= start + end + 3) return host;
return `${host.slice(0, start)}...${host.slice(-end)}`;
}

// Check sync state every second
Expand Down Expand Up @@ -92,12 +86,11 @@ export function Market(container) {
const addr = item.address;
let fullAddress;
if (typeof addr === 'string') {
fullAddress = addr.includes(':') ? addr : `${addr}:6102`;
fullAddress = addr;
} else {
const addressObj = addr || {};
const onionAddr = addressObj.onion_addr || '';
const port = addressObj.port || '6102';
fullAddress = `${onionAddr}:${port}`;
const host = addr?.onion_addr || '';
const portSuffix = addr?.port ? `:${addr.port}` : '';
fullAddress = host || portSuffix ? `${host}${portSuffix}` : '';
}
Comment on lines 88 to 94
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Edge case: port-only object can still produce a leading-colon string.

The new logic correctly avoids the bare ":" from the previous code when both fields are missing, but if addr.onion_addr is falsy while addr.port is truthy, the result becomes ":<port>" (e.g., ":6102"). That string then flows into the table title/viewFidelityBond lookup and formatTorEndpoint (which would treat the entire :port as host after slicing on the last :).

Suggest gating on onion_addr being present rather than on either field:

🛡️ Proposed guard 
     if (typeof addr === 'string') {
       fullAddress = addr;
     } else {
-      const host = addr?.onion_addr || '';
-      const portSuffix = addr?.port ? `:${addr.port}` : '';
-      fullAddress = host || portSuffix ? `${host}${portSuffix}` : '';
+      const host = addr?.onion_addr || '';
+      const portSuffix = host && addr?.port ? `:${addr.port}` : '';
+      fullAddress = host ? `${host}${portSuffix}` : '';
     }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/market/Market.js` around lines 88 - 94, The current
construction of fullAddress can produce a leading ":<port>" when addr.onion_addr
is falsy but addr.port is truthy; update the logic in Market.js where
fullAddress is computed (variable fullAddress, input addr with fields onion_addr
and port) to only build an address when onion_addr is present: if typeof addr
=== 'string' keep it, else if addr.onion_addr is truthy concatenate onion_addr
with an optional `:${port}`; otherwise set fullAddress to an empty string so
downstream lookups (viewFidelityBond) and formatTorEndpoint never receive a
port-only string.


// Handle null offers (unresponsive makers)
Expand Down Expand Up @@ -770,7 +763,7 @@ export function Market(container) {
.map(
(maker) => {
return `
<div class="grid grid-cols-7 gap-4 p-4 hover:bg-[#242d3d] transition-colors">
<div class="grid gap-4 p-4 hover:bg-[#242d3d] transition-colors" style="grid-template-columns: 2fr 1fr 1fr 1fr 1fr 1fr 1fr">
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Inline grid-template-columns duplicated between header and rows.

The same 7-column template string is repeated in the header (line 867) and on every row render (line 766). Extracting it into a single constant avoids drift if the layout is adjusted later (e.g., adding a column).

Also applies to: 867-867

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/market/Market.js` at line 766, Extract the repeated "2fr 1fr
1fr 1fr 1fr 1fr 1fr" grid-template string into a single constant (e.g., const
GRID_TEMPLATE_COLUMNS = "2fr 1fr 1fr 1fr 1fr 1fr 1fr") at the top of the Market
component and replace the inline style usages in the row JSX and the header JSX
with style={{ gridTemplateColumns: GRID_TEMPLATE_COLUMNS }} (or use the constant
in a style object) so both the row div (the element with class "grid gap-4 p-4
hover:bg-[`#242d3d`] transition-colors") and the header render reference the same
constant.


<div class="text-gray-300 font-mono text-sm truncate" title="${maker.address}">${formatTorEndpoint(maker.address)}</div>
<div class="text-green-400">${maker.baseFee}</div>
Expand Down Expand Up @@ -871,7 +864,7 @@ export function Market(container) {



<div class="grid grid-cols-7 gap-4 bg-[#FF6B35] p-4 text-xs">
<div class="grid gap-4 bg-[#FF6B35] p-4 text-xs" style="grid-template-columns: 2fr 1fr 1fr 1fr 1fr 1fr 1fr">
<div class="font-semibold">Tor Address</div>
<div class="font-semibold">Base Fee</div>
<div class="font-semibold">% Fee Rate</div>
Expand Down
14 changes: 12 additions & 2 deletions src/components/settings/FirstTimeSetup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export function FirstTimeSetupModal(container, onComplete) {

<div id="rpc-test-result" class="hidden"></div>

<div class="bg-yellow-500/10 border border-yellow-500/30 rounded-lg p-4">
<div id="node-setup-info" class="hidden bg-yellow-500/10 border border-yellow-500/30 rounded-lg p-4">
<div class="flex items-start gap-3 text-sm text-yellow-400">
${iconInfo}
<p>
Expand Down Expand Up @@ -485,7 +485,7 @@ export function FirstTimeSetupModal(container, onComplete) {

<div id="tor-test-result" class="hidden"></div>

<div class="bg-blue-500/10 border border-blue-500/30 rounded-lg p-4">
<div id="tor-setup-info" class="hidden bg-blue-500/10 border border-blue-500/30 rounded-lg p-4">
<div class="flex items-start gap-3 text-xs text-blue-400">
${iconInfo}
<p>
Expand Down Expand Up @@ -986,6 +986,9 @@ export function FirstTimeSetupModal(container, onComplete) {
];

renderConnectionResults(resultDiv, results);
const nodeFailed = results.some((r) => !r.ok);
const infoDiv = modal.querySelector('#node-setup-info');
if (infoDiv) infoDiv.classList.toggle('hidden', !nodeFailed);
} catch (error) {
console.error('RPC test failed:', error);

Expand All @@ -1000,6 +1003,8 @@ export function FirstTimeSetupModal(container, onComplete) {
: error.message,
},
]);
const infoDiv = modal.querySelector('#node-setup-info');
if (infoDiv) infoDiv.classList.remove('hidden');
}

btn.textContent = originalText;
Expand Down Expand Up @@ -1090,6 +1095,7 @@ export function FirstTimeSetupModal(container, onComplete) {
window.api.testTcpPort({ host: '127.0.0.1', port: controlPort }),
]);

const torFailed = !socksResult?.success || !controlResult?.success;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

torFailed placement is inconsistent with the RPC path — minor style nit.

In the RPC path nodeFailed is derived from the results array after renderConnectionResults (line 989 follows line 988). Here torFailed is computed from raw API values before renderConnectionResults (line 1098 precedes line 1099). Both are semantically equivalent, but aligning the two paths would improve readability and make future maintenance clearer.

♻️ Suggested alignment 
-      const torFailed = !socksResult?.success || !controlResult?.success;
       renderConnectionResults(resultDiv, [
         {
           label: 'SOCKS Port',
           ok: Boolean(socksResult?.success),
           message: socksResult?.success
             ? `Port ${socksPort} reachable`
             : socksResult?.error,
         },
         {
           label: 'Control Port',
           ok: Boolean(controlResult?.success),
           message: controlResult?.success
             ? `Port ${controlPort} reachable`
             : controlResult?.error,
         },
       ]);
+      const torFailed = !socksResult?.success || !controlResult?.success;
       const infoDiv = modal.querySelector('#tor-setup-info');
       if (infoDiv) infoDiv.classList.toggle('hidden', !torFailed);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/settings/FirstTimeSetup.js` at line 1098, Move the torFailed
calculation to the same place as nodeFailed for consistency: instead of
computing const torFailed = !socksResult?.success || !controlResult?.success
before calling renderConnectionResults, compute torFailed from the rendered
results array after renderConnectionResults (similar to how nodeFailed is
derived). Update the logic that uses torFailed to reference the newly computed
variable location and keep the original socksResult/controlResult usage only for
passing into renderConnectionResults.

renderConnectionResults(resultDiv, [
{
label: 'SOCKS Port',
Expand All @@ -1106,6 +1112,8 @@ export function FirstTimeSetupModal(container, onComplete) {
: controlResult?.error,
},
]);
const infoDiv = modal.querySelector('#tor-setup-info');
if (infoDiv) infoDiv.classList.toggle('hidden', !torFailed);
} catch (error) {
console.error('Tor test failed:', error);

Expand All @@ -1116,6 +1124,8 @@ export function FirstTimeSetupModal(container, onComplete) {
message: error.message || String(error),
},
]);
const infoDiv = modal.querySelector('#tor-setup-info');
if (infoDiv) infoDiv.classList.remove('hidden');
}

btn.textContent = originalText;
Expand Down
Loading