Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', '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('^' + ".*" + ' invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, '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" + ' invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, '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('^' + ".*" + ' invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, '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); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); invoice: Use PaymentHash in raw invoice types by jgmcalpine · Pull Request #4363 · lightningdevkit/rust-lightning · GitHub
Skip to content

invoice: Use PaymentHash in raw invoice types - #4363

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash
Feb 3, 2026
Merged

invoice: Use PaymentHash in raw invoice types#4363
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
jgmcalpine:invoice-raw-use-payment-hash

Conversation

@jgmcalpine

@jgmcalpinejgmcalpine commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Closes#4292

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Refactors RawBolt11Invoice, RawDataPart, and TaggedField to use
lightning_types::payment::PaymentHash instead of bitcoin::hashes::sha256::Hash.
This improves type safety and aligns the raw invoice types with the high-level
Bolt11Invoice.

This is a follow-up to #4293 to complete the migration of invoice types to PaymentHash.

Specific changes:

  • TaggedField::PaymentHash now holds a PaymentHash.
  • InvoiceBuilder::payment_hash now accepts a PaymentHash directly.
  • Implemented Base32 traits for PaymentHash in lightning-invoice to support
    serialization.
  • Updated tests to resolve name collisions between the PaymentHash type and
    the TaggedField variant.
  • Updated downstream usage in the lightning crate (channelmanager and
    invoice_utils) to remove redundant hash conversions now that the builder
    accepts the strong type.

@ldk-reviews-bot

ldk-reviews-bot commented Jan 30, 2026

Copy link
Copy Markdown

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

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 5 times, most recently from da905d6 to f967d8aCompareJanuary 30, 2026 18:56
Comment threadlightning/src/ln/channelmanager.rs Outdated
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
let payment_hash = invoice.payment_hash();
let payment_hash =
PaymentHash(Sha256::from_byte_array(invoice.payment_hash().0).to_byte_array());

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.

Huh? Same below.

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.

Ah, good catch. That was a redundant round-trip left over from when I was refactoring the types. Simplified it to just use invoice.payment_hash() directly

@TheBlueMatt
TheBlueMatt removed the request for review from valentinewallaceJanuary 30, 2026 20:42
@codecov

codecovBot commented Jan 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.35484% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.01%. Comparing base (f9ad345) to head (d652d86).
⚠️ Report is 49 commits behind head on main.

Files with missing linesPatch %Lines
lightning-invoice/src/de.rs98.63%0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4363 +/- ##
==========================================
- Coverage 86.09% 86.01% -0.09% 
==========================================
Files 156 156 Lines 102804 102693 -111 Branches 102804 102693 -111 ==========================================
- Hits 88508 88327 -181 - Misses 11788 11858 +70 
Partials 2508 2508 
FlagCoverage Δ
tests86.01% <99.35%> (-0.09%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch 2 times, most recently from 6b9f7ec to 59b1623CompareJanuary 30, 2026 21:01

@tnulltnull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Comment threadlightning/src/ln/channelmanager.rs Outdated
Err(()) => return None,
};

let payment_hash = invoice.payment_hash();

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.

Unnecessary diff.

let mut invoice_builder = InvoiceBuilder::new(currency)
.description(description.to_string())
.payment_hash(payment_hash)
.payment_hash(PaymentHash(payment_hash.to_byte_array()))

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.

Here too we're building a sha256::Hash just to undo it.

Comment threadlightning/src/ln/invoice_utils.rs Outdated
Bolt11InvoiceDescriptionRef::Direct(&Description::new("test".to_string()).unwrap())
);
assert_eq!(invoice.payment_hash(), payment_hash);
assert_eq!(invoice.payment_hash().0, payment_hash.0);

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.

Unnecessary diff.

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.

Fixed in the latest push

@jgmcalpine

Copy link
Copy Markdown
ContributorAuthor

Note: This is a breaking change for InvoiceBuilder. Downstream consumers (like ldk-node) will need to update their builder calls to pass PaymentHash directly.

Please note that we employ a "if you break it, you keep it" rule here by now. ;)

I.e., please make sure to open a PR on the LDK Node end to clean up the breakage as soon as this lands.

Understood! I'm happy to handle the LDK Node fix. I'll keep an eye on this and open the fix PR in ldk-node after this lands.

@jgmcalpine
jgmcalpineforce-pushed the invoice-raw-use-payment-hash branch from 59b1623 to d652d86CompareFebruary 2, 2026 17:19

@TheBlueMattTheBlueMatt 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.

thanks!

@TheBlueMatt
TheBlueMatt merged commit 817ab5e into lightningdevkit:mainFeb 3, 2026
20 of 21 checks passed
Sign up for freeto 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.

Bolt11Invoice::payment_hash should return a PaymentHash

4 participants

@jgmcalpine@ldk-reviews-bot@tnull@TheBlueMatt