Skip to content

smite: add closing_complete codec#105

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

smite: add closing_complete codec#105
ekzyis wants to merge 1 commit into
morehouse:masterfrom
ekzyis:closing-complete

Conversation

@ekzyis
Copy link
Copy Markdown
Contributor

@ekzyis ekzyis commented May 28, 2026

part of #98

@ekzyis ekzyis marked this pull request as draft May 28, 2026 16:21
@ekzyis ekzyis force-pushed the closing-complete branch 4 times, most recently from 249aa32 to 06abdf2 Compare May 28, 2026 19:51
//
// When closing a channel and shutdown is complete, each peer sends
// `closing_complete` with the transaction details.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
Copy link
Copy Markdown
Contributor Author

@ekzyis ekzyis May 28, 2026

Choose a reason for hiding this comment

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

I added the Default trait here, instead of a sample_closing_complete() function in the tests (bolt.rs + bolt/<message>.rs).

This is inconsistent with the other messages though.

I wonder if there was a reason to not use the Default trait, or if we should also use the Default trait for them. We could get rid of a lot of code in the tests. It does not look to me like the specific values picked for the sample messages matter.

Adding a Default trait just for less code in the tests might be a bad argument though.

@ekzyis ekzyis force-pushed the closing-complete branch from 06abdf2 to 0b61a30 Compare May 28, 2026 20:30
@ekzyis ekzyis marked this pull request as ready for review May 28, 2026 20:51
}

#[test]
fn decode_truncated_closer_scriptpubkey_length_prefix() {
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.

Testing truncated length prefixes is new. Not sure if needed.

@ekzyis ekzyis force-pushed the closing-complete branch 3 times, most recently from ae096c9 to c0a4be7 Compare May 29, 2026 08:07
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.

1 participant