[custom channels 4/5]: Extract PART4 from mega staging branch #9095

guggero · 2024-09-12T16:18:37Z

Extracts part 4 from #8960.

coderabbitai · 2024-09-12T16:18:43Z

Important

Review skipped

Auto reviews are limited to specific labels.

Labels to auto review (1)

llm-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-- I pushed a fix in commit <commit_id>, please review it.
-- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
-- @coderabbitai generate unit testing code for this file.
-- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
-- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
-- @coderabbitai read src/utils.ts and generate unit testing code.
-- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

github-actions · 2024-09-12T16:47:27Z

Pull reviewers stats

Stats of the last 30 days for lnd:

User	Total reviews	Time to review	Total comments
guggero 🥇	27 ▀▀▀	11h 17m	37 ▀▀
yyforyongyu 🥈	22 ▀▀	16h	37 ▀▀
Roasbeef 🥉	9 ▀	2d 19h 35m	20 ▀
ellemouton	7 ▀	13h 24m	13 ▀
bitromortac	5 ▀	6d 18h 31m ▀	50 ▀▀
ziggie1984	5 ▀	5d 13h 31m ▀	17 ▀
Crypt-iQ	4	13h 28m	7
bhandras	3	10m	0
ProofOfKeags	3	5d 6h 54m ▀	38 ▀▀
hieblmi	3	2h 55m	3
morehouse	2	7d 2h 27m ▀	9
alexbosworth	1	3d 21h 20m ▀	1
aakselrod	1	4d 2h 7m ▀	2
dstadulis	1	5d 7h 41m ▀	1
GeorgeTsagk	1	11d 15h 22m ▀▀	3
carlaKC	1	17h 11m	0
MPins	1	5d 8h 9m ▀	2

guggero · 2024-09-13T13:58:16Z

Ready for full review!

guggero · 2024-09-13T16:07:47Z

Please be aware that if the dependent part3 is updated and this isn't rebased immediately, it will look like there are more commits than before. The start commit for this part is always invoices: add invoice htlc interceptor service.

Will push a rebased version as soon as I've addressed the CI failures.

ellemouton

Second pass looking gooood! I like the changes to the interceptor 🙏

Think this is pretty much g2g.

invoices/modification_interceptor.go

invoices/invoiceregistry.go

invoices/modification_interceptor.go

bhandras

Really clean PR! I'm not as confident reviewing the second part (aux closer) but looks overall very good 🚀 I wonder why all itests fail though?

invoices/modification_interceptor.go

invoices/modification_interceptor_test.go

bhandras · 2024-09-17T17:24:00Z

invoices/invoiceregistry.go

+		// If this is the first HTLC for the invoice, we don't actually
+		// have the invoice in the database yet. We'll create a new and
+		// empty invoice to pass to the interceptor.
+		existingInvoice = Invoice{


Why do we intercept in this case? Given that the normal flow of execution is that we return a new fail resolution (see below after the UpdateInvoice call).

Ah, you're right. I wondered why some unit tests failed before, that's why I added the empty invoice. But makes sense to just return the failure resolution directly.

lntest/harness.go

lnwallet/channel.go

guggero · 2024-09-18T13:25:13Z

Addressed all comments and also fixed the CI. Ready for re-review.

ellemouton

Great work! LGTM 🚢

bhandras

LGTM 🥇

bhandras · 2024-09-18T19:04:27Z

invoices/invoiceregistry.go

+
+		// If the invoice was not found, return a failure resolution
+		// with an invoice not found result.
+		return NewFailResolution(


This commit introduces a new invoice htlc interceptor service that intercepts invoice HTLCs during their settlement phase. It forwards HTLCs to a subscribed client to determine their settlement outcomes. This commit also introduces an interface to facilitate integrating the interceptor with other packages.

This commit updates the invoice registry to utilize the settlement interceptor during the invoice settlement routine. It allows the interceptor to capture the invoice, providing interception clients an opportunity to determine the settlement outcome.

This commit initiates the invoice settlement interceptor during the main server startup, assigning it a handle within the server.

This commit integrates the HTLC modifier service into the invoices RPC server.

This commit introduces a singleton invoice HTLC modifier RPC server and an endpoint to activate it. The server interfaces with the internal invoice HTLC modifier interpreter, handling the marshalling between RPC types and internal formats.

This commit enhances the itest LND node harness to include support for the new `HtlcModifier` RPC endpoint. At the same time we move another method to the correct file.

With this commit we encode the custom records as a TLV stream into the custom channel data field of the invoice HTLC. This allows the custom data parser to parse those records and replace it with human-readable JSON on the RPC interface.

This commit introduces a basic integration test for the invoice HTLC modifier. The test covers scenarios where an invoice is settled with a payment that is less than the invoice amount, facilitated by the invoice HTLC modifier.

In this commit, we move to add the internal key to the delivery addr. This way, we give the aux chan closer the extra information it may need to properly augment the normal co-op close process.

With this commit we populate additional information about the close outputs (including potential custom channel data) in the close update RPC message. This will allow custom channels to find out how the additional close outputs look like on chain and what data they might commit to. We also hook up the aux custom data formatter, so it can format the custom channel data to JSON.

guggero added no-changelog custom chans labels Sep 12, 2024

guggero added this to the v0.18.4 milestone Sep 12, 2024

guggero mentioned this pull request Sep 12, 2024

[custom channels 5/5]: merge custom channel staging branch into master #8960

Open

guggero force-pushed the extract-part3-from-staging-branch branch from 5e33608 to 2199ea9 Compare September 13, 2024 13:56

guggero force-pushed the extract-part4-from-staging-branch branch from 8612201 to 3efea30 Compare September 13, 2024 13:56

guggero marked this pull request as ready for review September 13, 2024 13:57

guggero requested review from bhandras and ellemouton September 13, 2024 13:57

guggero force-pushed the extract-part4-from-staging-branch branch from 3efea30 to 5dccc8c Compare September 13, 2024 14:18

guggero force-pushed the extract-part3-from-staging-branch branch from 2199ea9 to 49a8371 Compare September 13, 2024 15:57

guggero force-pushed the extract-part4-from-staging-branch branch from 5dccc8c to f3a0959 Compare September 13, 2024 16:36

ellemouton reviewed Sep 16, 2024

View reviewed changes

invoices/modification_interceptor.go Show resolved Hide resolved

invoices/invoiceregistry.go Show resolved Hide resolved

invoices/modification_interceptor.go Outdated Show resolved Hide resolved

invoices/modification_interceptor.go Outdated Show resolved Hide resolved

guggero force-pushed the extract-part4-from-staging-branch branch from f3a0959 to 0a439cf Compare September 16, 2024 14:03

guggero requested a review from ellemouton September 16, 2024 14:05

bhandras reviewed Sep 17, 2024

View reviewed changes

guggero force-pushed the extract-part3-from-staging-branch branch 2 times, most recently from 3dc1947 to 8cb20e7 Compare September 18, 2024 10:03

guggero force-pushed the extract-part4-from-staging-branch branch from 0a439cf to 440a7b6 Compare September 18, 2024 12:46

guggero requested a review from bhandras September 18, 2024 13:24

ellemouton approved these changes Sep 18, 2024

View reviewed changes

guggero force-pushed the extract-part3-from-staging-branch branch 2 times, most recently from 0626d8c to 6a0d859 Compare September 18, 2024 15:46

guggero force-pushed the extract-part4-from-staging-branch branch from 440a7b6 to cc1a720 Compare September 18, 2024 15:46

guggero force-pushed the extract-part3-from-staging-branch branch from 6a0d859 to f093d3d Compare September 18, 2024 16:06

guggero force-pushed the extract-part4-from-staging-branch branch 2 times, most recently from a80d505 to 8acb279 Compare September 18, 2024 17:08

guggero force-pushed the extract-part3-from-staging-branch branch from f093d3d to 52e50d8 Compare September 18, 2024 17:08

bhandras approved these changes Sep 18, 2024

View reviewed changes

ffranr and others added 10 commits September 19, 2024 09:21

lnd: initialize invoice settlement interceptor at server startup

481dfe2

This commit initiates the invoice settlement interceptor during the main server startup, assigning it a handle within the server.

invoicesrpc: add HTLC modifier to invoices RPC server

008d265

This commit integrates the HTLC modifier service into the invoices RPC server.

lntest: add HtlcModifier support to node RPC harness

0c6a155

This commit enhances the itest LND node harness to include support for the new `HtlcModifier` RPC endpoint. At the same time we move another method to the correct file.

lnwire: add MergedCopy method to CustomRecords

83923c7

multi: pass UpdateAddHtlc message custom records to invoice modifier

bbae714

itest: add basic invoice HTLC modifier integration test

9a972e1

This commit introduces a basic integration test for the invoice HTLC modifier. The test covers scenarios where an invoice is settled with a payment that is less than the invoice amount, facilitated by the invoice HTLC modifier.

guggero changed the base branch from extract-part3-from-staging-branch to master September 19, 2024 07:22

guggero force-pushed the extract-part4-from-staging-branch branch from 8acb279 to 8f5a527 Compare September 19, 2024 07:22

Roasbeef and others added 10 commits September 19, 2024 10:18

lnwire: add CustomRecords to shutdown message

099f556

lnwallet: add FundingBlob method to LightningChannel

7b396f4

lnwallet: add ability to add extra co-op close outputs

517608c

lnwallet: add ability to do custom sort for coop close txn

117a144

lnwallet/chancloser: add new AuxChanCloser interface

7ff251c

lnwallet/chancloser: add aux chan closer, use in coop flow

8d651b9

lnwallet: modify CoopCloseBalance to not depend on chan commit

625d426

server+peer: init peer struct w/ AuxChanCloser if present

44ab7e6

peer: decorate delivery addr w/ internal key

8dee76a

In this commit, we move to add the internal key to the delivery addr. This way, we give the aux chan closer the extra information it may need to properly augment the normal co-op close process.

guggero force-pushed the extract-part4-from-staging-branch branch from 8f5a527 to e0b4601 Compare September 19, 2024 08:18

guggero merged commit 4ff7d77 into master Sep 19, 2024
23 of 27 checks passed

guggero deleted the extract-part4-from-staging-branch branch September 19, 2024 09:46

guggero mentioned this pull request Sep 19, 2024

Add new RPCs introduced in lnd v0.18.4 lightninglabs/lndclient#192

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[custom channels 4/5]: Extract PART4 from mega staging branch #9095

[custom channels 4/5]: Extract PART4 from mega staging branch #9095

guggero commented Sep 12, 2024 •

edited

Loading

coderabbitai bot commented Sep 12, 2024 •

edited

Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

github-actions bot commented Sep 12, 2024

guggero commented Sep 13, 2024

guggero commented Sep 13, 2024 •

edited

Loading

ellemouton left a comment

bhandras left a comment

bhandras Sep 17, 2024

guggero Sep 18, 2024

guggero commented Sep 18, 2024

ellemouton left a comment

bhandras left a comment

bhandras Sep 18, 2024

[custom channels 4/5]: Extract PART4 from mega staging branch #9095

[custom channels 4/5]: Extract PART4 from mega staging branch #9095

Conversation

guggero commented Sep 12, 2024 • edited Loading

coderabbitai bot commented Sep 12, 2024 • edited Loading

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

github-actions bot commented Sep 12, 2024

Pull reviewers stats

guggero commented Sep 13, 2024

guggero commented Sep 13, 2024 • edited Loading

ellemouton left a comment

Choose a reason for hiding this comment

bhandras left a comment

Choose a reason for hiding this comment

bhandras Sep 17, 2024

Choose a reason for hiding this comment

guggero Sep 18, 2024

Choose a reason for hiding this comment

guggero commented Sep 18, 2024

ellemouton left a comment

Choose a reason for hiding this comment

bhandras left a comment

Choose a reason for hiding this comment

bhandras Sep 18, 2024

Choose a reason for hiding this comment

guggero commented Sep 12, 2024 •

edited

Loading

coderabbitai bot commented Sep 12, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

guggero commented Sep 13, 2024 •

edited

Loading