ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.85k stars 902 forks source link

keysend: Add `maxfee` to keysend for consistency with pay #7653

Closed s373nZ closed 3 weeks ago

s373nZ commented 2 months ago

Add maxfee to keysend for consistency with pay

Description

Adds maxfee parameter to keysend with similar functionality of that of pay. Needed to achieve more consistent behavior across API endpoints.

Related Issues

Changes Made

Checklist

Ensure the following tasks are completed before submitting the PR:

Additional Notes

s373nZ commented 2 months ago

TODO:

s373nZ commented 2 months ago

Fixed the cln-grpc Rust test. I didn't think a Draft PR would trigger all the CI. Apologies for the build spam.

s373nZ commented 2 months ago

Added tests and fixed some of the logic for default values of the other parameters.

The maxfee handling logic is more or less a duplication of that found in the pay plugin. Perhaps this could be abstracted and shared? Will look into a little further, but I wonder where the appropriate place for that would be...

s373nZ commented 2 months ago

Sacrificed two semicolons to the flake8 gods.

s373nZ commented 1 month ago

Reran mssgen and regenerated the documentation examples as per the docs for good measure. Used the following commands:

PYTHONPATH=contrib/msggen python contrib/msggen/msggen/__main__.py bundle doc/schemas/
PYTHONPATH=contrib/msggen python contrib/msggen/msggen/__main__.py generate

make check-gen-updated

rm -rf /tmp/ltests* && REGENERATE='keysend' VALGRIND=0 TIMEOUT=10 TEST_DEBUG=1 pytest -vvv -s -p no:logging -n 6 tests/autogenerate-rpc-examples.pyrm -rf /tmp/ltests* && REGENERATE='keysend' VALGRIND=0 TIMEOUT=10 TEST_DEBUG=1 pytest -vvv -s -p no:logging -n 6 tests/autogenerate-rpc-examples.py

make

~The CI build is currently failing, but observation suggests this is happening with all PR branches due to something in master. Will keep an eye on the builds and continue to rebase.~ It looks like this change resulted in a successful build. AFAICT this is ready for an initial review.

s373nZ commented 1 month ago

Rebased, regenerated proto code and removed modifications to CHANGELOG.md for Changelog-Added: hint in commit message.

cdecker commented 3 weeks ago

ACK https://github.com/ElementsProject/lightning/pull/7653/commits/fdd60db630329a95d5ff3c90185913efc6687db2