Skip to content

splice: Clean up warning message - #9359

Open
ddustin wants to merge 1 commit into
ElementsProject:masterfrom
ddustin:ddustin/cleaner_splice_error
Open

splice: Clean up warning message#9359
ddustin wants to merge 1 commit into
ElementsProject:masterfrom
ddustin:ddustin/cleaner_splice_error

Conversation

@ddustin

@ddustin ddustin commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Add a clean warning message if peer sends junk ACK data.

@ddustin
ddustin force-pushed the ddustin/cleaner_splice_error branch from 3144726 to 53d9cbb Compare July 28, 2026 15:27

@Andezion Andezion 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.

The idea is right and matches the existing error-handling pattern in this function. Should splice_accepter() get a symmetric guard?

Comment thread channeld/channeld.c
@@ -4417,6 +4417,12 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg)

type = fromwire_peektype(inmsg);

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.

Didn't the check placed too late to do anything? Look at the top of the function -

static void splice_initiator(struct peer *peer, const u8 *inmsg)
{
	/ some code /
	struct wally_psbt *psbt = peer->splicing->current_psbt;   - on line 4412
	/ some code /
	type = fromwire_peektype(inmsg);  - on line 4418

	if (!peer->splicing) {  - and here we have new  check
		peer_failed_warn(...);
	}
}

So psbt is initialized from peer->splicing->current_psbt in the declaration block, which executes before type = fromwire_peektype(inmsg) and before the new if (!peer->splicing) check. If peer->splicing is NULL, the function already dereferences it at line 4412 and crashes - so the new check is unreachable code?

Maybe we should move the NULL check to the very first line of the function, before psbt is declared/initialized?

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