splice: Clean up warning message - #9359
Conversation
Changelog-None
3144726 to
53d9cbb
Compare
Andezion
left a comment
There was a problem hiding this comment.
The idea is right and matches the existing error-handling pattern in this function. Should splice_accepter() get a symmetric guard?
| @@ -4417,6 +4417,12 @@ static void splice_initiator(struct peer *peer, const u8 *inmsg) | |||
|
|
|||
| type = fromwire_peektype(inmsg); | |||
|
|
|||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Thanks for the review! You're absolutely right 🤦♂️, nice catch.
It's a good thought however the accepter side never receives an ACK -- it only sends them. |
Add a clean warning message if peer sends junk ACK data.