-
Notifications
You must be signed in to change notification settings - Fork 409
Rename from_raw
to from_blinded_path_and_payinfo
in BlindedPaymentPath
#3807
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
Rename from_raw
to from_blinded_path_and_payinfo
in BlindedPaymentPath
#3807
Conversation
I've assigned @joostjager as a reviewer! |
/// * `blinding_point`: The public key used for blinding the path. | ||
/// * `blinded_hops`: The encrypted routing information for each hop in the path. | ||
/// * `payinfo`: The [`BlindedPayInfo`] for the blinded path. | ||
pub fn from_blinded_path( |
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.
IMO -- can we keep the previous name or call this from_parts
?
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.
hmm I think from_parts
is too broad. Maybe from_blinded_path_and_payinfo
but could be too lengthly. I don't mind going back to from_raw
but I wanted to keep it similar to BlindedMessagePath::from_blinded_path
edit: so, I changed to from_blinded_path_and_payinfo
. Wasn't that bad
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.
Ah okay, I didn't notice BlindedMessagePath
already exposes a similar method, thanks! I'm good with the original name or this new one then.
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.
Great! I think I like the new one better.
986ac20
to
4845404
Compare
from_raw
to from_blinded_path
in BlindedPaymentPath
from_raw
to from_blinded_path_and_payinfo
in BlindedPaymentPath
4845404
to
51444c9
Compare
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.
Just sharing two thoughts. Not sure if relevant at all. This PR is just making a fn public.
/// * `blinding_point`: The public key used for blinding the path. | ||
/// * `blinded_hops`: The encrypted routing information for each hop in the path. | ||
/// * `payinfo`: The [`BlindedPayInfo`] for the blinded path. | ||
pub fn from_blinded_path_and_payinfo( |
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.
This one could call into from_parts
.
Or BlindedPath
and from_parts
could be made public instead and this method can be dropped?
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.
sure, the first one makes totally sense.
Similar to 8ce438c, we rename `from_raw` function used in test/fuzzy scenarios to build `BlindedPaymentPath` to a public function `from_blinded_path_and_payinfo`. This function is useful when reconstructing a blinded path based on already serialized components.
51444c9
to
9284298
Compare
@valentinewallace @joostjager thanks for the review! I staled your approvals with latest force push on updating on Joost comment. |
Similar to #3677, makes a way of building
BlindedPaymentPath
public.On previous experiments on doing this issue on lndk-org/lndk repo, I could build BlindedPath from LND Api response. After upgrading to 0.1.3, methods were made private.
This PR aims to make a way of building
BlindedPaymentPath
to be used inInvoiceRequest
responses from already serialized blinded paths.