search

agora-org / agora

File server that accepts Lightning Network payments for downloads
Creative Commons Zero v1.0 Universal
184 stars 26 forks source link

Add support for core-lightning #311

Open yzernik opened 2 years ago

yzernik commented 2 years ago

This adds support for core-lightning as a lightning node backend in addition to LND.

The high level changes:

Note: core-lightning does not allow multiple invoices with the same memo/label. I updated the files.rs module to append a random UUID to the end of the memo/label when adding invoice to avoid conflicts.

yzernik commented 2 years ago

@casey Slow tests should be passing now

I tested with the following:

cargo test
cargo test --features slow-tests slow_tests
cargo test --all-features --all
casey commented 2 years ago

It looks like there are a couple more test failures: https://github.com/agora-org/agora/runs/6696289984?check_suite_focus=true https://github.com/agora-org/agora/runs/6696289797?check_suite_focus=true

casey commented 2 years ago

Check out the github actions workflow for the tests that run on CI.

yzernik commented 2 years ago

A few updates:

yzernik commented 2 years ago

I figured out why the ubuntu-latest build was getting canceled in the middle. It's because it was configured to "fail-fast" as soon as the Windows build failed. I fixed this by adding the fail-fast: false config.

Now the ubuntu-latest build is passing all of the tests but failing on Clippy lint of the generated LND protobuf code. I need to find out how to tell Clippy to ignore the auto-generated code.

yzernik commented 2 years ago

@casey Another update:

casey commented 2 years ago

I think it's safe to disable the tests on windows. It's not the end of the world if Core Lightning isn't supported there.

yzernik commented 2 years ago

All github actions are now passing!

casey commented 2 years ago

Very nice! I'm prepping for a BTC++ workshop, so I probably won't be able to look at this until I get back on the 13th of June, but I'll review it then.

casey commented 2 years ago

I'm slammed with other, much more frivolous projects. (My Bitcoin NFT scheme and a generative art engine.) So no real time to give this a proper review. One thing I was thinking was that, in liu of review, we could merge this into a non-master branch, e.g., a core-lightning branch, and point to it from the readme. You could point people to the branch and they could build it from source, you could also make changes to the core-lightning branch without much review (I would just rubber-stamp PRs as long as the tests passed). What do you think?

yzernik commented 2 years ago

@casey I have already merged these changes into the master branch of my fork. So maybe just use that instead of the core-lightning branch in this repo?

I am going to open a new PR to point to my fork of Agora in umbrel-apps, now that they support core-lightning. EmbassyOS also uses core-lightning, so I will also point to my fork in the EmbassyOS wrapper.

I guess it doesn't hurt to have a core-lightning branch in this repo, unless it confuse people.

casey commented 2 years ago

It's up to you! It might have better visibility in the main repo, but either way is fine. Definitely open a PR adding a link to your branch from the readme.

yzernik commented 2 years ago

Sure. I'll do that!

For the umbrel app, the way they organize apps is that each has a list of other apps as dependencies. So for any lightning app, they package it twice (once for LND and once for core-lightning). For example, RideTheLightning:

So I can leave the existing package the same, pointing to this repo. And for the core-lightning version, point to my fork.

casey commented 2 years ago

So I can leave the existing package the same, pointing to this repo. And for the core-lightning version, point to my fork.

Perfect, sounds good.