Rename Event::PaymentClaimable::via_channel_ids to receiving_..
#3971
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Simple enough, one question though
| (4, amount_msat, required), | ||
| // Marked as legacy in version 0.2.0 for the same reason as `via_channel_id_legacy`; | ||
| // superseded by `via_user_channel_ids`. | ||
| (5, via_user_channel_id_legacy, option), |
There was a problem hiding this comment.
Hmm, any reason why we rename it here, but still reference via_user_channel_id_legacy in impl MaybeReadable for Event?
There was a problem hiding this comment.
Perhaps this legacy field can just remain as-is considering it would maybe need a changelog entry if it were changed anyway?
There was a problem hiding this comment.
Cause my search-and-replace sucks :). Having the legacy field called something different seems confusing to me - its just an internal variable and the point is that its the same thing as the current/new field just a different encoding, so IMO should be named the same.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
In 60540f7 we renamed `Event::PaymentClaimable::inbound_channel_ids` back to `via_channel_ids` which was changed in c62aa23. However, it was pointed out that while `inbound_channel_ids` was confusing, so was `via_channel_ids` (which can be easily interpreted as the hops that a payment took, rather than the last hops it took to get to us in an MPP payment). Thus, here, we rename it yet again to `receiving_channel_ids`, which is hopefully less ambiguous.
TheBlueMatt
force-pushed
the
2025-07-rename-again
branch
from
98ecbfc to
52613a7
Compare
|
Sorry, missed one $ git diff-tree -U1 98ecbfc96 52613a798
diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 37cb34c600..2b71207356 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -2159,3 +2159,3 @@ impl MaybeReadable for Event {
let mut claim_deadline = None;
- let mut via_user_channel_id_legacy = None;
+ let mut receiving_user_channel_id_legacy = None;
let mut onion_fields = None;
@@ -2170,3 +2170,3 @@ impl MaybeReadable for Event {
(4, amount_msat, required),
- (5, via_user_channel_id_legacy, option),
+ (5, receiving_user_channel_id_legacy, option),
(6, _user_payment_id, option),
@@ -2194,3 +2194,3 @@ impl MaybeReadable for Event {
receiving_channel_id_legacy
- .map(|chan_id| vec![(chan_id, via_user_channel_id_legacy)])
+ .map(|chan_id| vec![(chan_id, receiving_user_channel_id_legacy)])
}) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3971 +/- ##
==========================================
- Coverage 88.93% 88.92% -0.01%
==========================================
Files 174 174
Lines 123875 123875
Branches 123875 123875
==========================================
- Hits 110169 110162 -7
- Misses 11253 11261 +8
+ Partials 2453 2452 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 participants
In 60540f7 we renamed
Event::PaymentClaimable::inbound_channel_idsback tovia_channel_idswhich was changed inc62aa23. However, it was pointed out that while
inbound_channel_idswas confusing, so wasvia_channel_ids(which can be easily interpreted as the hops that a payment took, rather than the last hops it took to get to us in an MPP payment).Thus, here, we rename it yet again to
receiving_channel_ids, which is hopefully less ambiguous.