Skip to content

refactor: use channelPoint as uniqueId by replacing colon with hyphen#1362

Open
yemmyharry wants to merge 2 commits intojamaljsr:masterfrom
yemmyharry:fix/batch-channel-display
Open

refactor: use channelPoint as uniqueId by replacing colon with hyphen#1362
yemmyharry wants to merge 2 commits intojamaljsr:masterfrom
yemmyharry:fix/batch-channel-display

Conversation

@yemmyharry
Copy link
Copy Markdown

Closes #1361

Description

This PR fixes a bug where channels opened in a single batch transaction (sharing the same funding TXID but different output indices) were not all displaying in the Polar graph designer. Previously, the uniqueId generation for channels relied on a truncated version of the channelPoint (last 12 chars) , which caused collisions for batch outputs like txid:0 and txid:1 , resulting in only one channel being rendered.

Steps to Test

  1. Create a network with 3 LND nodes (Alice, Bob, Carol)
  2. From an external wallet connected to Bob, batch-open channels to both Alice and Carol simultaneously (using LND's PSBT-based channel batching via openChannelStream)
  3. Mine 3 blocks to confirm
  4. Observe the graph designer

Screenshots

[Only if applicable]

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Feb 28, 2026

Greptile Summary

This PR fixes a channel collision bug in the LND channel mapper where batch-opened channels sharing the same funding TXID (but different output indices) were rendered as a single channel in the graph designer, because the old uniqueId was derived from the last 12 characters of the txid alone (dropping the vout index). The fix replaces the : separator in the raw channelPoint string with -, using the full txid-vout value as the uniqueId.

Key changes:

  • Removes the private txid() helper that stripped the output index from channelPoint.
  • Updates mapOpenChannel and mapPendingChannel to set uniqueId to chan.channelPoint.replace(':', '-'), ensuring txid:0 and txid:1 map to distinct IDs (txid-0 and txid-1).
  • Note: String.prototype.replace with a string argument only replaces the first occurrence of :, which is correct here since a Bitcoin txid is a hex string (no colons), so there is always exactly one colon in a channelPoint. The fix is functionally sound.
  • No unit test file (mappers.spec.ts) exists, and none was added to verify the fixed behavior or prevent regressions.
  • Eclair and CLightning services retain their existing channelId.slice(-12) approach, which is unrelated to the LND channelPoint format and unaffected by this change.

Confidence Score: 4/5

  • Safe to merge — the fix is correct and targeted, with minimal risk of regression.
  • The logic change is small, well-scoped, and directly addresses the described collision. The channelPoint format guarantees exactly one colon, so replace(':', '-') is reliable. The only gap is missing unit test coverage for the mapper functions.
  • No files require special attention beyond the noted absence of unit tests in src/lib/lightning/lnd/mappers.ts.

Important Files Changed

Filename Overview
src/lib/lightning/lnd/mappers.ts Replaces the collision-prone 12-char truncated txid with the full channelPoint (colon replaced by hyphen) as the uniqueId — correctly fixes batch-channel display bug; no unit tests added.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[LND RPC returns Channel\nchannelPoint = 'txid:vout'] --> B{mapOpenChannel /\nmapPendingChannel}
    B --> C["uniqueId = channelPoint.replace(':', '-')\ne.g. 'abc123...txid-0'"]
    C --> D[updateLinksAndPorts in chart.ts]
    D --> E["links[uniqueId] created/updated\nports[uniqueId] created/updated"]
    E --> F[Each batch channel gets its own\ndistinct link in the graph]

    style C fill:#d4edda,stroke:#28a745
    
    subgraph OLD ["Old (broken) logic"]
      G["txid(channelPoint).slice(-12)\ne.g. both txid:0 and txid:1\nproduced same last-12-chars ID"]
      G --> H["links[sameId] — second channel\noverwrites first → only 1 link shown"]
    end
Loading

Last reviewed commit: bb91e47

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (711755f) to head (e74a7b3).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1362   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          211       211           
  Lines         7014      7013    -1     
  Branches      1398      1348   -50     
=========================================
- Hits          7014      7013    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@Abdulkbk Abdulkbk left a comment

Choose a reason for hiding this comment

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

tACK 🚀, the fix works.

A few notes:
PR title should be "fix" not refactor per conventional commits.

Also, the newly added test file mappers.spec.ts for the two "unique IDs for batch-opened channels" directly validates the fix and is welcome. It would be easier to review if you moved the specific test to the previous commit and then added the unrelated tests in this commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Batch-opened channels not fully displayed in graph designer

2 participants