') + ')', '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); } })(); })(); Reject invalid parameters instead of silently dropping them by benthecarman · Pull Request #182 · lightningdevkit/ldk-server · GitHub
Skip to content

Reject invalid parameters instead of silently dropping them - #182

Open
benthecarman wants to merge 2 commits into
lightningdevkit:mainfrom
benthecarman:cli-silent
Open

Reject invalid parameters instead of silently dropping them#182
benthecarman wants to merge 2 commits into
lightningdevkit:mainfrom
benthecarman:cli-silent

Conversation

@benthecarman

@benthecarmanbenthecarman commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Previously, onchain_send silently fell back to the node's default fee estimation when given an invalid fee_rate_sat_per_vb (e.g., overflows), and bolt12_receive silently dropped the quantity parameter when amount_msat was not set. Both cases now return an error instead.

Matching CLI-side validation added for both cases.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 7, 2026•
edited
Loading

Copy link
Copy Markdown

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@benthecarman
benthecarman requested a review from tnullApril 7, 2026 20:43
tnull
tnull reviewed Apr 8, 2026
View reviewed changes
Comment threadldk-server-cli/src/main.rs Outdated
},
Commands::OnchainSend { address, amount, send_all, fee_rate_sat_per_vb } => {
if let Some(rate) = fee_rate_sat_per_vb {
// Mirrors FeeRate::from_sat_per_vb: sat/vb * 1000/4 = sat/kwu

@tnulltnullApr 8, 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.

If we get into that territory, let's just take a dependency on bitcoin and use bitcoin::FeeRate?

@benthecarmanbenthecarmanApr 15, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Just removed the cli side validation. Too hit this would have to enter something greater than u64::MAX / 250 which is super unlikely a user enters anyways and don't want to add the whole rust-bitcoin dep tree to the cli just for that.

Comment threadldk-server-cli/src/main.rs
Commands::Bolt12Receive { description, amount, expiry_secs, quantity } => {
if quantity.is_some() && amount.is_none() {
handle_error_msg(
"quantity can only be set for fixed-amount offers (amount must be provided)",

@tnulltnullApr 8, 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.

Debatable, @TheBlueMatt previously wanted to drop quantity from the LDK Node API (lightningdevkit/ldk-node#642, but was closed as abandoned). Matt, do you still think we should?

benthecarmanand others added 2 commits April 15, 2026 12:44
@benthecarman@claude
Return error for invalid onchain send fee rate
734358b
Previously, an invalid fee_rate_sat_per_vb (overflows) was silently
collapsed into None via .and_then(), causing the node to substitute
its default fee estimation without informing the user. Now the server
returns an InvalidRequestError if the fee rate overflows.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@benthecarman@claude
Error when quantity set without amount in bolt12_receive
d9b4876
Previously, setting quantity without amount_msat silently produced a
variable-amount offer with no quantity constraint. Now both the server
and CLI reject this misconfiguration with a clear error message.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tnulltnulltnull left review comments

At least 1 approving review is required to merge this pull request.

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

@benthecarman@ldk-reviews-bot@tnull