Implement start_batch message batching
#3793
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
jkczyz
force-pushed
the
2025-05-start-batch
branch
from
97118cf to
b2d073f
Compare
jkczyz
force-pushed
the
2025-05-start-batch
branch
from
b2d073f to
5110534
Compare
|
Fixed some places where we disconnected but didn't send a warning (or disconnected when we should ignore and send a warning) according too the spec. Feels a little verbose, so let me know if there is some utility for this that I'm missing. |
|
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @wpaulino! This PR has been waiting for your review. |
Instead of batching commitment_signed messages using a batch TLV, the splicing spec has been updated to introduce a start_batch messages. It used to indicate that the next batch_size messages for the channel_id should be treated as one logical message. This commit simply adds the message while the following commits will implement the handling logic.
Update commitment_signed message to contain the funding_txid instead of both that and a batch_size. The spec was updated to batch messages using start_batch, which contains the batch_size. This commit also updates PeerManager to batch commitment_signed messages in this manner instead of the previous custom approach.
jkczyz
force-pushed
the
2025-05-start-batch
branch
from
5110534 to
c6d62c9
Compare
Squashed. Fixed CI by running |
|
Up to you, but we can probably drop b1abb5d once the optional |
jkczyz
force-pushed
the
2025-05-start-batch
branch
from
c6d62c9 to
f373238
Compare
Added a fixup that uses the |
|
Good to squash |
While the spec only includes commitment_signed messages in batches, there may be other types of batches in the future. Generalize the message batching code to allow for other types in the future.
If a message in a commitment_signed batch does not contain a funding_txid or has duplicates, the channel should be failed. Move this check from PeerManager to FundedChannel such that this can be done.
jkczyz
force-pushed
the
2025-05-start-batch
branch
from
f373238 to
ebf0d4e
Compare
|
Squashed and fixed linter failures. |
|
🔔 3rd Reminder Hey @wpaulino! This PR has been waiting for your review. |
Instead of batching
commitment_signed messagesusing a batch TLV, the splicing spec has been updated to introduce astart_batchmessages. It used to indicate that the nextbatch_sizemessages for thechannel_idshould be treated as one logical message. Updatecommitment_signedmessage to contain thefunding_txidinstead of both that and abatch_size. Also, updatePeerManagerto batch accordingly instead of the previous custom approach.