Skip to content

Conversation

@dzdidi
Copy link
Contributor

@dzdidi dzdidi commented Feb 25, 2025

@sangbida Things done for now

Most of the logic is a copy/paste of core-lightning client removal of what was standing out as unnecessary
SQLite storage is replaced with LDK's KV Store (in the most naive way)
All original unit tests are passing
I guess we can just comment on diff with things that needs to be done and start picking them one by one.

Also if you have a better process in mind please let me know, this was the first thing I've came up with

dzdidi and others added 23 commits January 30, 2025 13:50
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Update toolchain version

Change the path to link cln
@dzdidi
Copy link
Contributor Author

dzdidi commented Feb 25, 2025

@sangbida I have not considered earlier to implement it into core-lightning plugin, maybe as a feature flag (at least for db implementation). What do you think?

Here is how it could look approx dzdidi#4

@TheBlueMatt
Copy link

Have you seen lightningdevkit/rust-lightning#2552 and the PR that it depends on (that was merged)? That was previous work on the LDK end to make this easier (I assume you're using those APIs?)

@sangbida
Copy link

(pasting this comment again on the right PR)

Hey,
I was thinking we could compile some requirements in a doc somewhere, share it in the ldk discord just to make sure we're on the right track. Happy to compile this and share it around for feedback. It might help us to find things we can work on parallely. What do you think?

I'll take some time and go through this PR today, awesome work on it so far :)

@TheBlueMatt
Copy link

Yes, absolutely. I hope that the current LDK code is sufficient to get what you need from LDK for TEOS, though the above-linked PR would make it way easier and more robust (would love it if someone wants to pick that up!). Happy to chat more, though I need to remind myself the status of everything.

@mariocynicys
Copy link
Collaborator

I have not considered earlier to implement it into core-lightning plugin, maybe as a feature flag

I think we better have separate modules for different clients since the end binary would be "either" for LDK "or" for CLN (and never "mix of features").
We can have a new module though (e.g. teos-client-common) that has the common concepts shared between different clients/plugins, like retrier etc...

@dzdidi
Copy link
Contributor Author

dzdidi commented Feb 26, 2025

Me and @tnull couple of weeks ago we chatted about it and here are his key points:

  • adding/enabling watchower should be simply have a set_watchtower(&self, url: String) method on Builder
  • we can add an interface extension, i.e., have Node::watchtower_handler return a WatchtowerHelper API object where methods could live, and then in turn have them re-exposed in LDK Server
  • KVStore should be used for persistence so that LDK-Node can do both local and VSS modes if needed

@TheBlueMatt
Copy link

I'm a little confused how you implemented this without ever calling counterparty_commitment_txs_from_update at all. The point of a watchtower isn't just to store data (i.e. implement LDK's KVStore), but rather to have enough information to punish a counterparty for misbehaving. It looks like that isn't implemented here (but intends to be implemented via CommitmentRevocation?). I don't think it makes sense to eventually implement that by hooking LDK's KVStore as you won't get nearly enough info to figure out what tx data you need, rather, a watchtower should likely be implemented by hooking LDK's Persist trait.

dzdidi added 25 commits April 1, 2025 17:59
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
Signed-off-by: dzdidi <[email protected]>
@dzdidi dzdidi force-pushed the feat/teos-ldk-client branch from 9abb943 to f88c9e0 Compare April 1, 2025 15:59
@mariocynicys
Copy link
Collaborator

any updates?

@dzdidi
Copy link
Contributor Author

dzdidi commented Aug 11, 2025

any updates?

@sangbida , if I remember it was bocked by your changes to LDK

@sangbida
Copy link

Unfortunately, I had to pause my work on LDK because I was struggling to work on a large change at the time and I am unable to pick up the watchtower change to LDK at this time. I am not sure if anyone else is working on that at this time.

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.

5 participants