-
Notifications
You must be signed in to change notification settings - Fork 906
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
renepay: bugfix reading pending sendpays #7337
Conversation
plugin_err(pay_plugin->plugin, | ||
"(%s:%d) routetracker_get_amount failed " | ||
"probably due to an amount_msat overflow.", | ||
__PRETTY_FUNCTION__, __LINE__); |
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 did not make my reseach but we should make sure that this macros are present in all kind of compilers
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.
Never thought of this before. I have them all over the code.
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.
Never thought of this before. I have them all over the code.
well, I think we are safe, till now there are no compiled bug reports, so I think we are safe. I like the line number idea in the log a, so all good
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.
Well, __func__
and __LINE__
are standards (https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html).
The __PRETTY_FUNCTION__
macro seems to be introduced by gcc, but it is also supported by clang; this one is a substitute for __func__
for long function names in C++ (my bad habits from C++ creeping into clightning).
I'll make a follow-up PR to substitute all __PRETTY_FUNCTION__
with __func__
, cause there are being used in many places in the code.
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.
utACK 01c5fa6
53e6e87
to
29cc53d
Compare
Rebased onto master. |
When renepay starts, one of the first operations it does is to check for pending sendpays for the same invoice. Those are added up to an internal database to keep track of. Also the amount they deliver to the destination is computed so that the current payment rpc call would try to complete the payment for whatever amount remains to pay. For an incomplete payment after calling the RPC renepay I found this in the logs: 2024-05-22T19:44:19.853Z DEBUG plugin-cln-renepay: There are pending sendpays to this invoice. groupid = 6 delivering = 0msat, last_partid = 10 Where delivering should be the sum of the amounts delivered by pending routes.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
29cc53d
to
1ab1e20
Compare
Rebased on top of master to fix PR #7403 |
When renepay starts, one of the first operations it does is to check for pending sendpays for the same invoice. Those are added up to an internal database to keep track of. Also the amount they deliver to the destination is computed so that the current payment rpc call would try to complete the payment for whatever amount remains to pay. For an incomplete payment after calling the RPC renepay I found this in the logs:
2024-05-22T19:44:19.853Z DEBUG plugin-cln-renepay: There are pending sendpays to this invoice. groupid = 6 delivering = 0msat, last_partid = 10
Where
delivering
should correspond to the sum of the amounts delivered by pending routes.