Design Doc for Dynamic Commitments

[yyforyongyu]

This PR is created to decide the design of dynamic commitments. It will be updated with a README file once finalized, and merged to the side branch yy-dyn.

A new RPC UpdateChannelParams

A new RPC endpoint UpdateChannelParams is defined.

• Initially, the name UpgradeChannel was chosen; however, given that it's also possible to downgrade channel types, and we are focusing on channel params here, a more descriptive name is chosen.
• Details are hidden from users - they only need to know it's an RPC to update channel params, while under the hood, quiescence negotiation and dynamic commitment are performed automatically.
• The RPC will be built on rpcserver, which is sent to the Switch. The Switch will be responsible for finding the link and starting the process.

TODO: We may consider moving channel-related operations into a new RPC server as the proto file is already huge, and it's difficult for LLMs to digest.

DynCommit as an update log

Similar to how we process update_fee, the dyn_commit, which is the agreed update from the dynamic commitment negotiation, will be treated as an update log and persisted on disk. Pro and cons,
Pros,

• It fits our current commitment update logic perfectly - a commitment dance will be performed once there's a pending local/remote update.
• No extra bucket needed to persist this msg.
• Reestablishment stays largely unchanged.
  Cons,
• A median to large size refactor may be needed to interface paymentDescriptor - we could choose to use it still as-is, but it will be ugly and prone to bugs.
• The dynamic commitment flow is not as clear as the alternative (see below). We will perform negotiation in one place, and execution in htlcManager, which can be hard to follow and debug. The commitment dance has many assumptions that are not made for dynamic commitments, such as there's no need to check for HTLCs or circuits when processing the commitment update in dyn. There's also no need to use the PendingCommitTicker as quiescence has its own timeout logic. Finally, to exit quiescence in this mixed flow is not as clear as we process all dyn-related operations in one place.

Interface Updater

A new interface, Updater, is introduced under htlcswitch/dyn, and is accessed by channelLink. When an RPC request or a dyn_propose msg is received, the link will invoke Updater to start the dynamic commitment flow. The overall flow is shown in the following diagram:

    +-------+                                     +-------+
    |       |--(1)--- stfu ---------------------->|       |
    |       |(Proposes entering quiescence)       |       |
    |       |                                     |       |
    |       |<-(2)----------------------- stfu ---|       |
    |       |       (Agrees; channel is now quiet)|       |
    |       |                                     |       |
    |       |--(3)--- dyn_propose --------------->|       |
    |       |(Proposes new channel terms)         |       |
    |       |                                     |       |
    |       |<-(4)-------------------- dyn_ack ---|       |
    |       |             (Agrees to terms w/ sig)|       |
    |       |                                     |       |
    |       |--(5)--- dyn_commit_sig ------------>|       |
    |   A   |(Bundles proposal and sig, signs B's |   B   |
    |       |new commitment)                      |       |
    |       |                                     |       |
    |       |<-(6)------------- revoke_and_ack ---|       |
    |       |            (B revokes its old state)|       |
    |       |                                     |       |
    |       |<-(7)----------------- commit_sig ---|       |
    |       |           (Signs A's new commitment)|       |
    |       |                                     |       |
    |       |--(8)--- revoke_and_ack ------------>|       |
    |       |(A revokes its old state)            |       |
    |       |                                     |       |
    |       |  (Channel operates with new terms)  |       |
    +-------+                                     +-------+

There are three main tasks,

1. Enter the quiescent state when dyn starts, and exit the quiescent state when dyn ends.
2. Dyn negotiation, which handles dyn_propose, dyn_ack and dyn_reject.
3. Dyn execution, which handles dyn_commit, commit_sig and revoke_and_ack. Basically it's a simplified commitment dance.

Based on the scope of Updater's responsibility, we have two ways to design it.

Monolith Approach

We can view the dynamic commitment as a whole and let Updater manage the above three tasks, which was tried in the PoC. This approach is nice as it gives an almost linear flow of the process, making it easier to understand where we are in the flow. It should be easier to maintain and debug. However, given that the dyn execution is just a simplified commitment dance, it does mean we won't take advantage of the existing commitment dance flow. It also means DynUpdater needs access to several subsystems,

• Quiescer to initiate and terminate quiescence.
• lnpeer.Peer to send dyn-related msgs.
• *lnwallet.LightningChannel to update the channel config (ChannelConfig).
• *lnwallet.LightningChannel to persist DynCommit on disk,
  • ReceiveDynCommit
  • SendDynCommit
• *lnwallet.LightningChannel to process the commit_sig and revoke_and_ack,
  • ReceiveNewCommitment
  • SignNextCommitment
  • RevokeCurrentCommitment
  • ReceiveRevocation
• TowerClient to backup the state.

Modulized Approach

The Updater will only be responsible for the negotiation task. The link will handle quiescence initialization and termination, and its current commitment dance will be used for the dyn_commit. This approach aligns with the idea of treating dyn_commit as an update log; it's also nice as the Updater only needs to have access to lnpeer.Peer to send dyn-related msgs, as the rest of the operations are handled in the link.
This does mean we may lose some clarity about the overflow of the dynamic commitment. The link also needs to be stateful so it knows we are in the middle of a dynamic commitment.

Gemini's view

Gemini favors the Monolith approach even after I offered to refactor htlcManager to have a cleaner commitment update flow, here's the full text. The conclusion is,

While the refactoring approach is appealing from a code-purity perspective, I still lean towards the Monolith Updater Approach as the more pragmatic and safer initial implementation.

Here's the key reasoning: Isolate, then integrate.

Implement Safely First: Build the dynamic commitment feature in a self-contained Updater. This minimizes the impact on the existing, battle-tested htlcManager and allows you to deliver the feature with higher confidence and lower risk. The "duplicated" logic is minimal and specific to the new, simpler context.

Evaluate for Refactoring Later: Once dynamic commitments are implemented, working, and tested, you can then make a more informed decision about refactoring. At that point, you will have two concrete examples of a commitment dance. The patterns for abstraction will be much clearer, and the refactoring effort can be a separate, dedicated project with its own testing and rollout plan.

Starting with the monolith Updater gets you a working, safe implementation faster. Tackling the htlcManager refactor as a separate, subsequent project is a sound strategy for managing complexity and risk in a system like lnd.

Looking for a concept ACK here - maybe Gemini will think otherwise if the htlcManager has a cleaner flow. Personally I'm leaning towards Modulized approach, since we want to have a generalized control flow in the link to allow not just quiescence, dynamic commitments, but also splicing, etc.

[gemini-code-assist]

Summary of Changes

Hello @yyforyongyu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request lays the groundwork for dynamic channel commitments in LND, enabling the modification of channel parameters without closing and reopening channels. It introduces a new RPC for user interaction and details the internal design choices for managing the complex negotiation and commitment update process, inviting feedback on the proposed architecture.

Highlights

• Design Proposal for Dynamic Commitments: This pull request serves as a design document for implementing dynamic commitments in LND, outlining the intent, proposed RPCs, and architectural considerations for updating channel parameters on the fly.
• New RPC: UpdateChannelParams: A new streaming RPC endpoint, UpdateChannelParams, is introduced. This RPC will allow users to initiate a negotiation to update various channel parameters (e.g., dust limit, max pending amount, channel reserve, min HTLC, CSV delay, max accepted HTLCs) in a user-friendly manner, abstracting away the underlying quiescence and commitment dance.
• Architectural Discussion: Updater Interface: The PR proposes a new Updater interface within htlcswitch/dyn to manage the dynamic commitment flow. It presents two main design approaches: a 'Monolith Approach' where the Updater handles all aspects (quiescence, negotiation, execution), and a 'Modulized Approach' where the Updater focuses only on negotiation, leveraging existing commitment dance logic. The author is seeking concept approval for the overall design direction.
• New Logging Subsystem: A dedicated logging subsystem, DYNC, has been added under htlcswitch/dyn to provide specific logging for dynamic commitment operations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a design document and the initial scaffolding for dynamic channel commitments. This includes a new UpdateChannelParams RPC and a new htlcswitch/dyn package. The changes are well-structured and lay a good foundation for the new feature.

My review focuses on improving API design for long-term maintainability and ensuring strict adherence to the project's coding style guide. I've provided a few suggestions to make the new REST endpoint more conventional and to improve naming in the new protobuf messages. I've also pointed out a minor style guide violation in one of the new files.

[gemini-code-assist]

According to the style guide, function comments should begin with the function name and be a complete sentence explaining the function's purpose.¹

The init function is a special function in Go, but it's good practice to document its purpose, especially in a library package.

Style Guide References

Suggested change

	// The default amount of logging is none.
	// init initializes a logger for this package.

Footnotes

1. The style guide requires every function to be commented with its purpose, and the comment must begin with the function name and be a complete sentence. (link) ↩

[gemini-code-assist]

The field name error is a keyword in Go and many other languages. While protoc will generate a field named Error, it's generally better to use a more descriptive name like error_message or error_string to avoid any potential confusion or conflicts.

    string error_message = 2;

[gemini-code-assist]

The REST path /v1/channels/params/{...} is a bit unconventional. It suggests that params is a resource collection under /v1/channels. A more RESTful approach would be to treat params as a sub-resource of a specific channel.

Consider a path like /v1/channels/{channel_point.funding_txid_str}/{channel_point.output_index}/params. This clearly indicates that you are updating the parameters of a specific channel.

While there are other inconsistencies in the existing API, this is a good opportunity to improve the design for new endpoints.

      post: "/v1/channels/{channel_point.funding_txid_str}/{channel_point.output_index}/params"

[Roasbeef]

Cons,
A median to large size refactor may be needed to interface paymentDescriptor - we could choose to use it still as-is, but it will be ugly and prone to bugs.

This is something I consider to be pretty over due. I have a suuuper old version that I started way back here: Roasbeef@16d036e#diff-8896f76f365f8d3c7b6aec4e89140c923aeaf5a5ec1487b9c6cb9fd37cf5cab7R1.

We'd ofc need to weigh the risks of such a refactor.

The dynamic commitment flow is not as clear as the alternative (see below). We will perform negotiation in one place, and execution in htlcManager, which can be hard to follow and debug

Can't we extract this into a distinct state machine? Perhaps using protofsm? That lends well to having LLMs do stuff like generate mermaid markdown diagrams.

So then all the link does it forward to this state machine. The state machine can then just directly send a response to the peer if needed. Or invoke some other signal to do a commitment update (tho I think that'll auto trigger as the logs won't be 100% in sync after).

So the terminal output in the success case is it returns something to insert into the log.

It also means DynUpdater needs access to several subsystems,

It doesn't need to access these directly. They can be accessed via interfaces ofc. If the option of decoupled message passing is attractive here (eg: to the TowerClient), then the actor package may interest you. Similarly the actions to the Quiescer can also be a send on it's ref to add something to its mailbox. Requests can be blocking, or non-blocking.

*lnwallet.LightningChannel to update the channel config (ChannelConfig).

In my mental model, this happens within the state machine solely. This is part of the internal processing of the new log entry we insert. That gets updated in memory in the commitment linked list, we make a new commitment txn from that, then when we write it to disk, we also update the channel config.

*lnwallet.LightningChannel to process the commit_sig and revoke_and_ack,

Same as above, the job of the DynUpdater ends once we agree on the log entry to insert.

[Roasbeef]

There's a bit of nuance here, as some of these fields are actually asymmetric when applied to a channel.

As an example, the csv_delay we send in open_channel is actually what we want them to use for their commitment transaction.

I think we can hide this fact from the user with this RPC, but it's something to keep in mind during the "negotiation" phase. As if we want to reduce our csv delay, we need to ask the remote peer if their willing to. This is security sensitive, as if the remote party just always approvals this subset of params, then I can get them to reduce it to 1 block, and try to breach (they have less time to react).

That's all to say that certain params can be auto accept, while others may need som edefalut setting or a sort of acceptor subscription.

[yyforyongyu]

There's a bit of nuance here, as some of these fields are actually asymmetric when applied to a channel.

yeah good point, I'll update the docs to make them clearer.

That's all to say that certain params can be auto accept, while others may need som edefalut setting or a sort of acceptor subscription.

Def - I think we can reuse the validation logic used in channel opening.

[yyforyongyu]

Can't we extract this into a distinct state machine? Perhaps using protofsm? That lends well to having LLMs do stuff like generate mermaid markdown diagrams.

Yeah that's what I've tried in the PoC. Will go with the Modulized Approach, and the job of the DynUpdater ends once we agree on the log entry to insert, then it's up to the link to perform the commitment dance.