feat: Support receiving LN for other users

[benthecarman]

The goal here is to be able to create a lightning invoice on behalf of another user and when it is paid the funds are locked to their key rather than our own.

Any feedback would be much appreciated!

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 53.48837% with 40 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (master@646e42c). Click here to learn what that means. Report is 10 commits behind head on master.

:exclamation: Current head 6ef6583 differs from pull request most recent head 85ff68e. Consider uploading reports for the commit 85ff68e to get more accurate results

Files	Patch %	Lines
modules/fedimint-ln-client/src/lib.rs	44.44%	35 Missing :warning:
modules/fedimint-ln-client/src/receive.rs	78.26%	5 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #3820 +/- ## ========================================= Coverage ? 57.00% ========================================= Files ? 193 Lines ? 43393 Branches ? 0 ========================================= Hits ? 24738 Misses ? 18655 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[elsirion]

I discussed a similar scheme with @Kodylow recently for a trusted but federated-custody LNURL server:

• User registers a public key P with LNURL server, receives LNURL U
• Each time an invoice is requested for U:
  • A public key P_i=tweak(P, i) is derived by tweaking P with a unique, continuous index i
  • The LNURL server posts an IncomingContractOffer where the preimage pre=preimage(P_i) is derived from P_i and the amount is the one from the LNURL request (this is where we need to trust the server to be honest)
  • An invoice is created with payment hash hash(pre) and the respective amount and returned to the requester
• When a user comes back online and wants to check for received money:
  • They ask the LNURL server for the last used index last
  • They know the index prev up to which they scanned last time
  • They scan for incoming contracts belonging to keys P_prev,...,P_last and start claim state machines for all non-empty, non-expired offers.

This way the LNURL server, while being trusted, never holds any money.

[benthecarman]

A public key P_i=tweak(P, i) is derived by tweaking P with a unique, continuous index i

I wonder if just doing an xpub would be simpler

[elsirion]

I wonder if just doing an xpub would be simpler

Conceptually not really, you basically do the same tweaking, just with more complexity involved. Maybe it's preferable because it's a bit more standardized, but not by much imo.

[benthecarman]

My thinking is that might help with privacy. If you see the keys just incrementing by 1 the gateway could correlate payments with a user whereas with a xpub they couldn't, only the lnurl server could

[elsirion]

Oh sure, you'd hash pub key+idx to get the real tweak, then correlation isn't possible. That's how Tweakable::tweak is implemented in Fedimint already.

https://github.com/fedimint/fedimint/blob/f6f368dd9ce664118404d80e08d93f2ac338f514/modules/fedimint-wallet-common/src/tweakable.rs#L29-L31

[benthecarman]

Added the tweaking, just copy-pasted some of the code that you linked since it was in the wallet module

[Kodylow]

@elsirion tagging for visibility on support for receive on behalf of other user by pubkey

[elsirion]

See 684551aa36920ff183d0d3976869e733916eee2b for a very hacky version of external invoice generation that needs a lot of refactoring to make it:

• reuse as much logic as possible
• make it backwards compatible (I actually think it already is, but at the cost of a lot of code duplication)

[elsirion]

I think #4282 unblocked this PR.

[benthecarman]

Rebased the current changes, not really sure where to take it from here tbh

[m1sterc001guy]

Rebased the current changes, not really sure where to take it from here tbh

To make this backwards compatible, you essentially need to map the old LightningReceiveSubmittedOffer to the new one.

1. Make a copy of LightningReceiveSubmittedOffer with no changes (can be called v0 or something)
2. Make a new struct that is identical, but contains ReceivingKey instead of KeyPair
3. Inside get_database_migrations of fedimint-ln-client, add a new closure into the BTreeMap where the closure reads the active/inactive states and checks if any of them are type LightningReceiveSubmittedOfferV0 (i.e with KeyPair). From these "old" states, create the new LightningReceiveSubmittedOffer and return it from the closure. There's an example migration in dummy-client that does something similar.
4. Bump the database version in fedimint-ln-client.

This will essentially map the old state struct to the new one that you're adding. Hopefully that makes sense. We're doing a deep dive on this next week but feel free to ping me in the meantime.

[benthecarman]

I think I did this correctly, not sure how to test. Added as a separate commit for now for easier review

[m1sterc001guy]

not sure how to test

Best way to test is probably to follow the dummy client example again.

1. Add function create_client_states in fedimint-ln-tests and call it from snapshot_client-db_migrations. Create the old state that you expect to be migrated.
2. Add some code in test_client_db_migrations that verifies that all of the old states are now the new states.
3. Run just snapshot-client-db-migrations fedimint-ln-tests. This will re-generate the database backup with your new state written to the db and run the migration test, which will verify that the migration was successful.
4. If all of that passes, check in everything to your branch, including the database files.

[benthecarman]

Okay got migration tests passing, actually caught a bug so that feels good

[benthecarman]

It changed some committed log files, not sure if those are supposed to be included or not

[benthecarman]

Responded to all review besides the fancier version of the db migrations, still figuring out how to properly do that

[benthecarman]

Alright think I got the fancy migrations working :rocket:

[douglaz]

Failing tests?

[benthecarman]

Failing tests?

Needed to re generate, fixed

[m1sterc001guy]

I don't want to block on this too much more since I think it's looking good and would like to get it in, but we should be able to add a test for this right? Maybe that can be done in a follow up.

[benthecarman]

I don't see where it reissues that received ecash

Trying to work on adding that, getting the tests to work for me has been a struggle, still figuring out what I'm doing wrong

[benthecarman]

got the beginnings of a test working, need to add functionality to claim the output now

[benthecarman]

need to add functionality to claim the output now

maybe this is better for a follow up, I am pretty lost on how to do this. Might be easier for someone else to do it so I am not blocking anything

I've squashed my commits now and have a test with a todo for claiming the final ecash

[benthecarman]

rebased

[elsirion]

Looks good, just general question maybe to @elsirion are there any other changes that need to be made on the ecash claiming side for the client? would this affect recovery if it needs the tweaks for all the ecash received via external LN ? I don't see where it reissues that received ecash

@Kodylow modules don't have to know of each other, so anything we do in the LN module can't affect the e-cash module. All the LN module sees is an API that lets it dump money into a primary module that can keep it somehow. This primary module could be the e-cash module or some other hypothetical, maybe account-based, module.