') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Do not record unilateral closes as payments by tankyleo · Pull Request #979 · lightningdevkit/ldk-node · GitHub
Skip to content

Do not record unilateral closes as payments - #979

Merged
tnull merged 1 commit into
lightningdevkit:mainfrom
tankyleo:2026-07-filter-out-unilateralclose
Jul 14, 2026
Merged

Do not record unilateral closes as payments#979
tnull merged 1 commit into
lightningdevkit:mainfrom
tankyleo:2026-07-filter-out-unilateralclose

Conversation

@tankyleo

@tankyleotankyleo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Unilateral closes are not onchain payments. The real on-chain credits to our onchain balance after a unilateral close happen on Sweep and Claim transactions.

Also, UnilateralClose payments currently never graduate from the Pending state, as no BDK events are emitted when these transactions confirm.

Co-Authored-By: HAL 9000

@ldk-reviews-bot

ldk-reviews-bot commented Jul 14, 2026•
edited
Loading

Copy link
Copy Markdown

👋 I see @jkczyz was un-assigned.
If you'd like another reviewer assignment, please click here.

@tankyleo
tankyleo requested a review from tnullJuly 14, 2026 06:18
@tankyleo

tankyleo commented Jul 14, 2026•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Continued from #660

The original motivation for this change: commitment transactions in 0FC channels have zero-fees and zero-amount according to BDK (since they don't interact with the on-chain wallet), so these were not recorded in the payment store. I then failed the assertion in the channel_full_cycle test that asserts that UnilateralClose was present in the payment store.

Hmm, these don't directly interact with the onchain wallet, but wouldn't it still make sense to record them as otherwise users would have a 'gap' between channel funding and the resulting Sweep transaction? If we keep them that could allow them to track the funds end-to-end, on each step?

Would have to think more about the overall purpose of payment store, as a user I think I would find it quite confusing for there to be a zero-amount inbound payment, with a non-zero fee. In the case of zero-fee commitment channels, these would be inbound payment for zero-amount, zero-fee (but the entry could still allow us to track the movement of funds).

Also note that currently UnilteralClose only gets recorded for outbound channels, as BDK can tracks the funding output and assigns a non-zero fee only in this case. This makes sense as for inbound channels, the fee of the commitment transaction comes from the counterparty's balance, not our balance.

@tankyleo
tankyleo requested a review from jkczyzJuly 14, 2026 06:23
@tankyleotankyleo mentioned this pull request Jul 14, 2026
Merged
tnull
tnull previously approved these changes Jul 14, 2026
View reviewed changes

@tnulltnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, makes sense to me, though I still think it somehow would be nice to show users the intermediate transaction state somehow. But you're probably right that the payment store is not the right place to do it.

Comment threadtests/common/mod.rs Outdated
"node_b should remain persisted in node_a peer store after locally-initiated force-close"
);
assert_any_node_has_onchain_tx_type(
assert_no_node_has_onchain_tx_type(

@tnulltnullJul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Can we just reuse assert_all_nodes_have_onchain_tx_type and invert the predicate?

@tankyleotankyleoJul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks I amended the commit below

@tankyleo
tankyleoforce-pushed the 2026-07-filter-out-unilateralclose branch from 72a5940 to d3919d2CompareJuly 14, 2026 15:58
@tankyleo
tankyleo requested a review from tnullJuly 14, 2026 15:59
@tankyleo

tankyleo commented Jul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

Hmm @tnull let me know what you think of this "unknown splice funding txid" CI failure in Rust Tests. So far on my machine I've been unable to reproduce with

RUSTFLAGS="--cfg cycle_tests --cfg tokio_unstable" cargo test channel_full_cycle_0conf_0reserve`

@tnull

tnull commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Hmm @tnull let me know what you think of this "unknown splice funding txid" CI failure in Rust Tests. So far on my machine I've been unable to reproduce with

RUSTFLAGS="--cfg cycle_tests --cfg tokio_unstable" cargo test channel_full_cycle_0conf_0reserve`

I think it's a flake that I have yet to investigate, so may not be related to this PR.

tnull
tnull reviewed Jul 14, 2026
View reviewed changes
Comment threadsrc/wallet/mod.rs Outdated
LdkTransactionType::UnilateralClose { .. } => {
log_trace!(
self.logger,
"Not recording unilateral close broadcast {} as a payment",

@tnulltnullJul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bit odd to log not to do something, especially since we don't do so for all the other variants. Maybe just drop the log?

@tankyleotankyleoJul 14, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done

@tankyleo
Do not record unilateral closes as payments
b34fb66
Unilateral closes are not onchain payments. The real on-chain credits to
our onchain balance after a unilateral close happen on `Sweep` and
`Claim` transactions.
Also, `UnilateralClose` payments previously never graduated from the
`Pending` state, as no BDK events are emitted when these transactions
confirm.
Co-Authored-By: HAL 9000
@tankyleo
tankyleoforce-pushed the 2026-07-filter-out-unilateralclose branch from d3919d2 to b34fb66CompareJuly 14, 2026 18:19
@tankyleo
tankyleo requested a review from tnullJuly 14, 2026 18:20
@tnull
tnull merged commit 150f370 into lightningdevkit:mainJul 14, 2026
17 of 23 checks passed
@tankyleo
tankyleo removed the request for review from jkczyzJuly 14, 2026 22:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tnulltnulltnull approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tankyleo@ldk-reviews-bot@tnull