Skip to content

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Dec 30, 2025

In some cases, the peer storage we receive could be empty or truncated. We want to be safe and correctly catch every exception possible when deserializing, because we don't want the app to crash and the user to be stuck.

@t-bast t-bast requested a review from sstone December 30, 2025 14:13
Copy link
Member

@sstone sstone left a comment

Choose a reason for hiding this comment

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

There should be a simple test that highlights what was missed.

In some cases, the peer storage we receive could be empty or truncated.
We want to be safe and correctly catch every exception possible when
deserializing, because we don't want the app to crash and the user to
be stuck.
@t-bast
Copy link
Member Author

t-bast commented Dec 30, 2025

It's actually really hard to properly cover in unit tests (fuzzing would be great here) and there were very few existing tests, but I added two cases (empty backup and corrupted backup) in the last push, which fail without the fix.

@t-bast t-bast requested a review from sstone December 30, 2025 14:53
@t-bast t-bast merged commit 1ddb67b into master Dec 30, 2025
2 of 4 checks passed
@t-bast t-bast deleted the empty-backup-fix branch December 30, 2025 18:24
sstone added a commit to ACINQ/phoenix that referenced this pull request Dec 31, 2025
Includes:
- a fix for uncaught deserialization errors (see ACINQ/lightning-kmp#843)
- a update to bitcoin-kmp with a change to taproot tweaks (see ACINQ/bitcoin-kmp#170)
dpad85 pushed a commit to ACINQ/phoenix that referenced this pull request Jan 5, 2026
Includes:
- a fix for uncaught deserialization errors (see ACINQ/lightning-kmp#843)
- a update to bitcoin-kmp with a change to taproot tweaks (see ACINQ/bitcoin-kmp#170)
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