Skip to content
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

update to newer cln hold plugin #816

Merged
merged 5 commits into from
Sep 6, 2023
Merged

Conversation

daywalker90
Copy link
Contributor

Hi,
until we get hold invoices into the official repo, this will update robosats to my newer hold plugin version. It has significant performance improvements and this special version i linked in this pr offers both rpc and grpc methods related to holdinvoices. I had to split the grpc part and consistently named everything "hold" instead of "hodl" (as i did for the official repo), so some changes were necessary.

CLN now needs to run both cln-grpc (by simply setting the grpc-port= in the config) and the plugin here cln-grpc-hold. That means we now need node and in addition hold grpc files.

The new plugin works with the new CLN 23.08 version that has an improved pay command.

If anyone migrates from the old plugin to the new i highly recommend to wait for all current holdinvoices to settle/cancel and then switch.

TODO:

  • Fix the ports: in the cln config add grpc-port=CLN_GRPC_HOST and switch grpc-hodl-port to grpc-hold-port=CLN_GRPC_HOLD_HOST and double check with what i did in cln.py with CLN_GRPC_HOST and CLN_GRPC_HOLD_HOST
  • Build the new plugin from cln-grpc-hold and change the CLN config line important-plugin=/path/to/cln-grpc-hold
  • when compiling CLN make will generate the node python grpc files into contrib/pyln-grpc-proto/pyln/grpc/. The files
    node_pb2.py, node_pb2_grpc.py and primitives_pb2.py need to be copied to where the other files are. I added some sed commands to then fix the imports on those files.

@Reckless-Satoshi
Copy link
Collaborator

Amazing! Let me know if I should get going running some tests on this branch.

Hoping to see hodlvoice merge soon into the official repo. That way we can skip altogether having to mantain our own CLN Dockerfile: having to run a custom CLN image would definitely put-off some wannabe RoboSats coordinators.

@daywalker90
Copy link
Contributor Author

Don't know much about docker but cln has an official docker image for 23.08 and then you just need to pull my github branch and build the plugin. The branch only contains files for the plugin itself, it is entirely compileable without the cln repo.

@daywalker90
Copy link
Contributor Author

Yes, but you have to do the "TODO's", im too stupid. Then you can run some tests.

@daywalker90
Copy link
Contributor Author

daywalker90 commented Aug 28, 2023

Is docker/cln the only place where docker stuff would need changes? I found those just now and added a commit to fix the changes to the new branch. The cln Dockerfile can maybe make use of FROM elementsproject/lightningd:v23.08 to slim it down?

EDIT: This should solve the first 2 TODO's

@Reckless-Satoshi
Copy link
Collaborator

All clear. I will be digging into this as soon as I get some time for it. It might take ~2 weeks unfortunately!

@daywalker90
Copy link
Contributor Author

I think it might be more inline with the rest and easier on the docker side if i add the node.proto files from the cln repo in generate_grpc.sh, so i did that in the latest commit.

I wanted to make a point again for docker, cln and plugins. I don't know enough about docker, but cln has an official docker repo and plugins are usually just a binary or script you drop in and load dynamically or on cln's startup. It should also be somewhat easy to later add different plugins, since they are a big part of cln's ecosystem.

@Reckless-Satoshi Reckless-Satoshi merged commit ae9fdd7 into RoboSats:main Sep 6, 2023
1 check failed
@Reckless-Satoshi
Copy link
Collaborator

Hey @daywalker90 tested and seems to work as expected. I will patch the Dockerfile for the new plugin and v23.08 on the next commit.

Please drop an invoice for a small tip for this contribution of 200K Sats. Thank you!!

@daywalker90
Copy link
Contributor Author

oh thats too kind thank you,
lnbc2m1pj03x2dpp5jt2d6ug23hyh2a2r3fprxvqz5mt2s220j3ntm2t80g7vhwlrnnhqdqqcqzzsxqyz5vqsp5kdxsaqe4cdagwcnzzr2k4g9yr53y3nm2lz7ttydj76qp4e6tw8jq9qyyssqwyxwf0xu7zph3kjslgg57e8r2786ypnfspzn780gaylz27cu8g3x4wuxdpnjzeec643t8ldd042ksgv582lyjfk7724evrzmxe5f6mqqpgv4td

@Reckless-Satoshi
Copy link
Collaborator

lnbc2m1pj03x2dpp5jt2d6ug23hyh2

Wops, it expired before I came back to it. Could you share a new one?

@daywalker90
Copy link
Contributor Author

lnbc2m1pj03x2dpp5jt2d6ug23hyh2

Wops, it expired before I came back to it. Could you share a new one?

yes:
lnbc2m1pj0ksedpp5rn9xtauz94qxkef6dt68nadaw0lu9jgk6p297k5ne9xrdfhu5ngqdqqcqzzsxqyz5vqsp59sqm5xt8nwgjmgyq4tz45snlhgu3ty23ddteut3r5f8q7jc0tsyq9qyyssqyevfkaj789ahkfw700ak2ajqeyde8aj7sh60j436vegt8qgzfrnx60l4qwx8qy7wvm5fjgsve6z339su5wlra5zsyeuwncvtf7azgkcpxr5l3r

@daywalker90
Copy link
Contributor Author

This one expires in a week:
lnbc2m1pj0mw27sp535xacnwmu03gcsum97n03h24pj3v4tnl2v5xlmaec68g39hfqeespp53aulu6cmlnvra6ae0tvzh9f5nwr6sal5mf24ah9yw8fsvcjsjjxsdqqxqyjw5qcqpjrzjqvjt5gujufdl7a4t6zcl0e948d0c92jnlhwrgxds2xmvsqwmnc3ywrpfevqq8wcqq5qqqqlgqqqqqqgqvs9qxpqysgqq6s0hzx6w4j44n0rs9g2k032f7jrh8ssm6wus9j9v30fpgw8hff88v7666l57u9u2gnj865l4a7x43akl7ksvnlqnl8aeqe22dhtccqqqwgt2n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants