Skip to content

smite: add closing_sig codec - #109

Merged
morehouse merged 1 commit into
lnfuzz:masterfrom
ekzyis:closing-sig
Jul 13, 2026
Merged

smite: add closing_sig codec#109
morehouse merged 1 commit into
lnfuzz:masterfrom
ekzyis:closing-sig

Conversation

@ekzyis

@ekzyisekzyis commented May 29, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

part of #98 | based on #105

This is basically the same as #105.

ekzyis
ekzyis commented Jun 1, 2026
View reviewed changes
Comment threadsmite/src/bolt/closing_sig.rs
@ekzyis
ekzyisforce-pushed the closing-sig branch 2 times, most recently from d458859 to b53492fCompareJune 9, 2026 07:32
@ekzyisekzyis mentioned this pull request Jun 10, 2026
Open
29 tasks
morehouse
morehouse requested changes Jun 16, 2026
View reviewed changes

@morehousemorehouse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

All comments from #105 apply here as well.

We can deduplicate the ClosingTlvs a little more I think.

Comment threadsmite/src/bolt.rs Outdated
Comment threadsmite/src/bolt/closing_complete.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
@ekzyisekzyis mentioned this pull request Jun 25, 2026
Merged
@ekzyis
ekzyisforce-pushed the closing-sig branch 3 times, most recently from 94ddba2 to efd1288CompareJune 25, 2026 20:07
@ekzyis

ekzyis commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Rebased on master (bf52f5f) after #105 was merged, and addressed all feedback (#105+here) in efd1288.

Decided to put everything into a single commit to avoid adding duplicate code in one commit only to remove it in the next. Lmk if you'd prefer it differently.

@ekzyis
ekzyis requested a review from morehouseJune 26, 2026 05:27
morehouse
morehouse reviewed Jul 10, 2026
View reviewed changes

@morehousemorehouse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's several divergences from the closing_complete codec that we can fix for consistency. Other than that it looks good.

Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
fn decode_truncated_closee_scriptpubkey() {
let mut data = vec![0x11; CHANNEL_ID_SIZE];
data.extend_from_slice(&[0x00, 0x0a]);
data.extend_from_slice(&[0x22; 0x0a]);

@morehousemorehouseJul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit:

Suggested change
data.extend_from_slice(&[0x22;0x0a]);
data.extend_from_slice(&[0x22;10]);

@ekzyisekzyisJul 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

done in 17571c5

Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs Outdated
Comment threadsmite/src/bolt/closing_sig.rs
@ekzyis

ekzyis commented Jul 11, 2026•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

There's several divergences from the closing_complete codec that we can fix for consistency.

Mhh, I'm not sure how I missed these, but 17571c5 should now have addressed all the feedback. I verified this by comparing the output of git range-diff master efd1288 17571c5 with your comments. Branch has also been rebased on master (f99ac3e).

If nits like the one regarding decimal vs. hexadecimal for the vec! lengths repeat syntax come up often enough, we could run grep -R "; 0x" smite smite-* | wc -l and check in CI that the output is 1. Currently, this is the only line with that syntax:

smite/src/bolt/wire.rs:707: let oversized = vec![0x00; 0xffff + 1];

morehouse
morehouse approved these changes Jul 13, 2026
View reviewed changes

@morehousemorehouse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@morehouse
morehouse merged commit 0ec0089 into lnfuzz:masterJul 13, 2026
5 checks passed
@ekzyis
ekzyis deleted the closing-sig branch July 15, 2026 16:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@morehousemorehousemorehouse approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ekzyis@morehouse