Skip to content

Add BOLT channel_reestablish (type 136) codec support#50

Open
theAnuragMishra wants to merge 1 commit into
morehouse:masterfrom
theAnuragMishra:bolt/channel_reestablish
Open

Add BOLT channel_reestablish (type 136) codec support#50
theAnuragMishra wants to merge 1 commit into
morehouse:masterfrom
theAnuragMishra:bolt/channel_reestablish

Conversation

@theAnuragMishra
Copy link
Copy Markdown

This PR adds support for the BOLT 2 channel_reestablish message, which is needed for reconnect/state resynchronization flows.

I have run the tests and it's all green.

part of #5

Copy link
Copy Markdown
Contributor

@ekzyis ekzyis left a comment

Choose a reason for hiding this comment

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

Hello and welcome! Thank you for the PR.

Noticed two things:

  1. Trailing bytes should be decoded as a TLV stream

  2. CI didn't run yet, but it would fail. You can run the commands locally before requesting the next review, see .github/workflows/rust.yml.

Related to this, I think we should add #[allow(clippy::too_many_lines)] to fn message_type_values():

$ cargo clippy --all-targets --all-features -- -D warnings
...
error: this function has too many lines (112/100)
   --> smite/src/bolt.rs:671:5
    |
671 |     fn message_type_values() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
    = note: `-D clippy::too-many-lines` implied by `-D clippy::pedantic`
    = help: to override `-D clippy::pedantic` add `#[allow(clippy::too_many_lines)]`

error: could not compile `smite` (lib test) due to 1 previous error
diff --git a/smite/src/bolt.rs b/smite/src/bolt.rs
index c261a1d..b7e2aa9 100644
--- a/smite/src/bolt.rs
+++ b/smite/src/bolt.rs
@@ -668,6 +668,7 @@ mod tests {
     }

     #[test]
+    #[allow(clippy::too_many_lines)]
     fn message_type_values() {
         assert_eq!(
             Message::Warning(Warning::all_channels("")).msg_type(),

Comment on lines +16 to +27
pub struct ChannelReestablish {
/// The channel ID.
pub channel_id: ChannelId,
/// The next commitment number expected from the peer.
pub next_commitment_number: u64,
/// The next revocation number expected from the peer.
pub next_revocation_number: u64,
/// The sender's view of the receiver's last per-commitment secret.
pub your_last_per_commitment_secret: [u8; PER_COMMITMENT_SECRET_SIZE],
/// The sender's current per-commitment point.
pub my_current_per_commitment_point: PublicKey,
}
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.

TLV stream missing

@Chand-ra
Copy link
Copy Markdown

Related to this, I think we should add #[allow(clippy::too_many_lines)] to fn message_type_values():

$ cargo clippy --all-targets --all-features -- -D warnings
...
error: this function has too many lines (112/100)
   --> smite/src/bolt.rs:671:5
    |
671 |     fn message_type_values() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
    = note: `-D clippy::too-many-lines` implied by `-D clippy::pedantic`
    = help: to override `-D clippy::pedantic` add `#[allow(clippy::too_many_lines)]`

error: could not compile `smite` (lib test) due to 1 previous error
diff --git a/smite/src/bolt.rs b/smite/src/bolt.rs
index c261a1d..b7e2aa9 100644
--- a/smite/src/bolt.rs
+++ b/smite/src/bolt.rs
@@ -668,6 +668,7 @@ mod tests {
     }

     #[test]
+    #[allow(clippy::too_many_lines)]
     fn message_type_values() {
         assert_eq!(
             Message::Warning(Warning::all_channels("")).msg_type(),

I do that here in PR #38. I thought of creating a separate PR for it but it seemed like a related change.

Comment thread smite/src/bolt/channel_reestablish.rs Outdated
Comment thread smite/src/bolt/channel_reestablish.rs Outdated
Comment thread smite/src/bolt.rs Outdated
Comment thread smite/src/bolt.rs Outdated
Comment thread smite/src/bolt.rs Outdated
Comment thread smite/src/bolt.rs
@theAnuragMishra
Copy link
Copy Markdown
Author

Thank you so much for the review guys. Addressing everything in some time

@theAnuragMishra theAnuragMishra force-pushed the bolt/channel_reestablish branch from 049bf9f to 9503029 Compare April 14, 2026 21:10
@theAnuragMishra
Copy link
Copy Markdown
Author

I have addressed all of your points @ekzyis and @Chand-ra . Thank you so much for your reviews.
in short:
added TLVStream support, moved the constant to types.rs, and picked a per_commitment_point value from lightning/bolt bolt 3 Appendix E.

Also ran all the checks in the ci locally, everything is clean.

Copy link
Copy Markdown
Contributor

@ekzyis ekzyis left a comment

Choose a reason for hiding this comment

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

Please look at how tlvs is implemented and tested in other messages, for example open_channel2.

Sign up for free to 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.

3 participants