-
Notifications
You must be signed in to change notification settings - Fork 402
Inbound user_channel_id
randomization follow-up
#1855
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
Inbound user_channel_id
randomization follow-up
#1855
Conversation
Codecov ReportBase: 90.68% // Head: 91.51% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1855 +/- ##
==========================================
+ Coverage 90.68% 91.51% +0.82%
==========================================
Files 89 90 +1
Lines 47947 53434 +5487
Branches 47947 53434 +5487
==========================================
+ Hits 43481 48900 +5419
- Misses 4466 4534 +68
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked that we always set inbound channel user_id
s to a random value by asserting that it's non-0 in channel.funding_created
and channel.funding_signed
and ensuring tests pass
Yeah, I'm a little unsure whether to include a regression test checking for non-0 as it technically is indeterministic. Then again, it's veeery unlikely we'd ever hit the 0 in a |
No strong preference. I'm fine rolling the dice on that, though, if we think a regression test would be good. |
60d0a10
to
bd281dd
Compare
Yeah, after having it written out it seems kinda obvious that it's fine to take the chance 😅 |
Grrr, good catch. Feel free to squash. |
As it was previously omitted, we clarify here starting from which version users can expect the `user_channel_id` to be randomized for inbound channels.
bd281dd
to
7f6713c
Compare
Squashed without further changes. |
This is a follow-up to #1790, which addresses two issues:
user_channel_id
was indeed set inChannel::new_from_req
, it was actually overridden with a 0 value just a few lines below. In the first commit, we address this oversight.user_channel_id
to be randomized, addressing Randomizeuser_channel_id
for inbound channels #1790 (comment).