Route::route_params: Option -> required#4693
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
I've re-verified the Writeable/Readable implementation. This confirms my prior finding at router.rs:893 — TLVs 1 and 3 are read as No new issues found. Prior findings (already posted, not re-posted):
Re-verified benign:
|
cb7a128 to
912bed1
Compare
send_payment_with_routeRoute::route_params: Option -> required
|
I ran all the CI checks that seemed relevant, including fuzz. |
912bed1 to
4acf958
Compare
TheBlueMatt
left a comment
There was a problem hiding this comment.
This is An API change so def can't be backported, but can we backport a separate PR that just comments on route_params in 0.3 that this will soon move to being required and must always be set?
4acf958 to
e3b30de
Compare
Ok, I opened #4704 |
|
🔔 1st Reminder Hey @tankyleo! This PR has been waiting for your review. |
About to modify this method slightly, so opportunistically format it now.
e3b30de to
111dc63
Compare
This field has always been set since 0.0.117. Since we're making it required here, routes created/serialized prior to 0.0.117 will fail to deserialize on 0.4.
In the last commit, we made Route::route_params required instead of an Option. After this change, we can modify a few methods that took a Route in addition to a separate PaymentParameters argument, since the pay params can always be retrieved from the Route now.
111dc63 to
d5d502a
Compare
Route::route_paramshas always been set since 0.0.117. Any routes created/serializedprior to 0.0.117 will fail to deserialize on 0.4.
Should land after #4704