Skip to content

smite: add closing_sig codec#109

Open
ekzyis wants to merge 1 commit into
morehouse:masterfrom
ekzyis:closing-sig
Open

smite: add closing_sig codec#109
ekzyis wants to merge 1 commit into
morehouse:masterfrom
ekzyis:closing-sig

Conversation

@ekzyis

@ekzyis ekzyis commented May 29, 2026

Copy link
Copy Markdown
Contributor

part of #98 | based on #105

This is basically the same as #105.

Comment thread smite/src/bolt/closing_sig.rs
@ekzyis ekzyis force-pushed the closing-sig branch 2 times, most recently from d458859 to b53492f Compare June 9, 2026 07:32
@ekzyis ekzyis mentioned this pull request Jun 10, 2026
29 tasks

@morehouse morehouse left a comment

Copy link
Copy Markdown
Owner

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 thread smite/src/bolt.rs Outdated
Comment thread smite/src/bolt/closing_complete.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
@ekzyis ekzyis force-pushed the closing-sig branch 3 times, most recently from 94ddba2 to efd1288 Compare June 25, 2026 20:07
@ekzyis

ekzyis commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

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 morehouse June 26, 2026 05:27

@morehouse morehouse left a comment

Copy link
Copy Markdown
Owner

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 thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/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]);

Copy link
Copy Markdown
Owner

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]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done in 17571c5

Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs Outdated
Comment thread smite/src/bolt/closing_sig.rs
@ekzyis

ekzyis commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

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), and has 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];

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.

2 participants