Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

channel: build_closing_transaction i64 -> u64 underflow #3410

Closed
phlip9 opened this issue Nov 14, 2024 · 2 comments
Closed

channel: build_closing_transaction i64 -> u64 underflow #3410

phlip9 opened this issue Nov 14, 2024 · 2 comments
Assignees
Milestone

Comments

@phlip9
Copy link

phlip9 commented Nov 14, 2024

Hey, was taking a look at channel close fees today and noticed something funky in Channel::build_closing_transaction.

When value_to_holder: i64 or value_to_counterparty: i64 are negative (funder doesn't have enough channel balance to pay the total_fee_satoshis) they'll underflow at the _ as u64 conversion.

Adding some logs, you can see:

// context
[lightning/src/ln/channel.rs:4004:3] holder_balance = 2500
[lightning/src/ln/channel.rs:4004:3] counterparty_balance = 17500

// first block
[lightning/src/ln/channel.rs:4004:3] total_fee_satoshis = 3053
[lightning/src/ln/channel.rs:4004:3] value_to_holder = -553
[lightning/src/ln/channel.rs:4004:3] value_to_counterparty = 17500

// after if blocks
[lightning/src/ln/channel.rs:4022:3] total_fee_satoshis = 3606
[lightning/src/ln/channel.rs:4022:3] value_to_holder as u64 = 18446744073709551063
[lightning/src/ln/channel.rs:4022:3] value_to_counterparty as u64 = 17500

Fortunately the remote just force closes, so it doesn't seem too problematic.

@TheBlueMatt
Copy link
Collaborator

Good point, yea we should debug_assert on that not underflowing and if it does in prod just force-close the channel. I don't believe that underflow is actually reachable.

@TheBlueMatt TheBlueMatt added this to the 0.1 milestone Nov 25, 2024
@arik-so arik-so self-assigned this Dec 5, 2024
@TheBlueMatt
Copy link
Collaborator

Fixed in #3452.

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

No branches or pull requests

3 participants