[custom channels]: merge custom channel staging branch into master

[guggero]

trafficstars

Fixes https://github.com/lightningnetwork/lnd/issues/8769.

This is the cleaned up code of previous PRs that all went into the 0-19-staging branch and represents all the changes needed to get custom channels into lnd.

Reviewers please read

All commits have been pre-reviewed in previous PRs by at least one person. We are looking for a second round review, mostly with focus on safety and stability when running with non-custom channels.

The idea is that reviewers get assigned to a certain range of commits within this PR. We decided to not do multiple PRs to keep rebase effort minimal (and so we have all context in a single PR).

The following parts exist (see commit message prefixes marking the start of a part):

• -PART1--: General tapscript leaf awareness and aux leaf store
  • Assigned reviewers: @ProofOfKeags
  • 🛠️ Status: Ready for full second round review
  • EXTRACTED from this PR into: https://github.com/lightningnetwork/lnd/pull/9030
• -PART2--: Custom records on HTLCs, SCID alias management, TLV traffic shaper
  • Assigned reviewers: @bitromortac @ProofOfKeags
  • :yellow_circle: Status: Ready for full review
• -PART3--: Aux funding and RPC/CLI custom data integration
  • Assigned reviewers: @Crypt-iQ
  • :yellow_circle: Status: Ready for full review
• -PART4--: Invoice HTLC modifier and aux channel closer
  • Assigned reviewers: @ellemouton @bhandras
  • :yellow_circle: Status: Ready for full review
• -PART5--: Force close sweeping
  • Assigned reviewers: @yyforyongyu
  • 🛠️ Status: Ready for full review, review in progress...

Known TODOs

This PR is still in draft, but once we assigned reviewers, they can start loading context on the code.

The following TODOs definitely need to be addressed before this can be taken out of draft:

• All items in https://github.com/lightningnetwork/lnd/issues/8769 (inlined below for easier overview):
  • [x] https://github.com/lightningnetwork/lnd/pull/8509#discussion_r1606975757 (collision and range of custom local SCID aliases)
  • [x] https://github.com/lightningnetwork/lnd/pull/8509#discussion_r1588119871 (error handling for unknown custom local SCID alias)
  • [x] https://github.com/lightningnetwork/lnd/pull/8509#issuecomment-2120858347 (collision detection and validation of base SCID)
  • [x] https://github.com/lightningnetwork/lnd/pull/8660#discussion_r1608641948 (implement and test HTLC custom record persistence)
  • [x] https://github.com/lightningnetwork/lnd/pull/8660#discussion_r1609843872 (unit test coverage of all added custom record functionality)
  • [ ] https://github.com/lightningnetwork/lnd/pull/8660#discussion_r1606720575 (persistence of custom records across restarts when sending payments)
  • [x] https://github.com/lightningnetwork/lnd/pull/8632#discussion_r1612091616 (mock implementation for aux components and basic unit test coverage)
  • [X] https://github.com/lightningnetwork/lnd/pull/8771#discussion_r1616920049 (add more fields to invoicesrpc.HtlcModifier
    • Marked as done because no additional fields were required for the custom channel functionality.
  • [x] revert itest change in https://github.com/lightningnetwork/lnd/pull/8908/commits/4f2c75f62047579f249ed5914042a0cc6337f342 to properly test amount modification
    • Need to use invoice acceptor to be able to pull this off, so create invoice for amount X, actually send X*2 (which'll be in the MPP amount to expect), intercept HTLC to modify it back to X then use invoice acceptor to settle with just X and ignore MPP total amount)
• [X] Formatting and general commit order cleanup of Part 5
• [x] https://github.com/lightningnetwork/lnd/pull/8960#discussion_r1717776047 (remove hardcoded sweep output amount)

[coderabbitai[bot]]

[!IMPORTANT]

Review skipped

Auto reviews are limited to specific labels.

:label: 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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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[bot]]

Pull reviewers stats

Stats of the last 30 days for lnd:

	User	Total reviews	Time to review	Total comments
	guggero 🥇	14 ▀▀	2d 20h 48m	43 ▀▀
	yyforyongyu 🥈	13 ▀▀	1d 13h 22m	22 ▀
	bhandras 🥉	6 ▀	9h 54m	5
	Roasbeef	6 ▀	6h 49m	9
	ProofOfKeags	6 ▀	20d 6h 33m ▀▀▀	79 ▀▀▀
	ellemouton	6 ▀	13d 18h 18m ▀▀	46 ▀▀
	morehouse	4 ▀	10d 6h 18m ▀	15 ▀
	GeorgeTsagk	3	17h 29m	3
	bitromortac	2	17d 18h 49m ▀▀	8
	carlaKC	2	20h 22m	10
	ziggie1984	2	3d 20h 46m ▀	8
	Webbdlee74	1	1d 6h 57m	0

[Roasbeef]

The following parts exist (see commit message prefixes marking the start of a part):

This prefix seems to have been dropped in the latest rebase?

[guggero]

This prefix seems to have been dropped in the latest rebase?

Oops, I haven't actually pushed it yet... It's now up to date and also rebased on top of the first refactor work in the channel state machine.

[guggero]

I addressed some of the TODOs and fixes the unit and integration test failures. Also added release notes that cover all new functionality of this PR.

I added a "Status" section to each part of this PR in the main PR body, will update that whenever status changes. That means part 1, 3 and 5 are fully ready for review, @ProofOfKeags, @Crypt-iQ, @yyforyongyu.

[Roasbeef]

I pulled in the msg router specific comments from this PR into the original PR adding it: https://github.com/lightningnetwork/lnd/pull/8520

Ideally we can get this one in quickly, then rebase this over it.

[Roasbeef]

@ProofOfKeags

Ok so the biggest thing I want to challenge is the idea that we should use an Option for the tapscript root. IIRC we always have a tapscript root, irrespective of the presence of any aux leaves

We don't always have a tapscript root. When we do musig2 today for taproot channels, we don't have that extra tweak, so it's effectively BIP 86. This ensures that the only way the output can be spent is via the musig2 multi-sig with no script path.

Only with the new aux chan added here is the concept of a tapscript root for the funding output relevant.

We may not have any aux leaves and so I think it makes sense for that to be an option but we can probably save ourselves the effort and just compute the BIP86 root during initialization rather than computing it at the end and having to thread through optional arguments everywhere.

FWIW, the BIP 86 root is basically just nil, so we either need root != nil checks everywhere, or we thread through the option as is.

[ProofOfKeags]

FWIW, the BIP 86 root is basically just nil

Why don't we just compute the BIP86 value and drop it into that structure so that we can unify the types without needing the Option? I'm trying to push for symmetry/uniformity and I think there is natural symmetry here since there is always a value for this. I just think in the nil case you're computing it later?

[guggero]

Why don't we just compute the BIP86 value and drop it into that structure so that we can unify the types without needing the Option? I'm trying to push for symmetry/uniformity and I think there is natural symmetry here since there is always a value for this. I just think in the nil case you're computing it later?

The BIP86 script root is an empty byte slice: https://github.com/btcsuite/btcd/blob/master/txscript/taproot.go#L290 We can't put that into a chainhash.Hash, unless we use the all-zero hash. But then we need another distinction somewhere, which IMO is more dangerous.

[guggero]

Rebased after https://github.com/lightningnetwork/lnd/pull/8520 and addressed some first comments.

[guggero]

I extracted some commits into https://github.com/lightningnetwork/lnd/pull/9025 as requested.

All TODOs except for a single one (will just be an additional itest) are addressed, so all parts are now officially ready for full review.

@saubyk I'll update the PR body with the status regularly, please always look at that first.

[guggero]

@ProofOfKeags and @yyforyongyu: I believe I have addressed all of your comments to part 1 and have extracted those commits into https://github.com/lightningnetwork/lnd/pull/9030 as requested.

Please do the second round of review there. I hope the comments here are sufficiently resolved so it's not too much of a hassle to now have a new PR...

[guggero]

@ProofOfKeags and @bitromortac, I've addressed all your comments on part 2 (except for those being tracked as a TODO in the new PR) and extracted it into a separate PR for round 2: https://github.com/lightningnetwork/lnd/pull/9049

Please hold off on the second round of review until I take it out of draft.

[guggero]

@ellemouton (and @bhandras), I addressed all your comments for part 4.

[guggero]

I didn't get to addressing all comments before my break. I resolved those that are addressed.

[Roasbeef]

do we need to create new witness sizes in input/size.go?

No, nothing changes re the witness size.

[Roasbeef]

Cool think all my comments are addressed - just need to fix the merge conflict to let the CI run.

Ok, fixed the last merge conflict and also an issue in the way I updated MaxFeeRateAllowed. We're green now!