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

Alternative Bitcoin backend(s) #56

Open
darosior opened this issue Oct 7, 2022 · 23 comments
Open

Alternative Bitcoin backend(s) #56

darosior opened this issue Oct 7, 2022 · 23 comments
Assignees
Labels
Feature New feature or functionality. RFC Needs conceptual discussion and/or feedback from users

Comments

@darosior
Copy link
Member

darosior commented Oct 7, 2022

The only Bitcoin backend the first release will have, and that we'll support in the near future, is Bitcoin Core using a watchonly wallet there. Other software tend to (ab)use the Electrum protocol and other kinds of backends (explorer APIs for instance). We might want to support other Bitcoin backends than Bitcoin Core.

For instance i can think of those as being desirable:

  • Electrum (but we need to introduce Tor, and/or TLS... Meh)
  • Maybe a Bitcoin Core backend that does not require a watchonly wallet (scantxoutset?)

Thoughts? Is there interest from users for prioritising this feature?

@darosior darosior added the RFC Needs conceptual discussion and/or feedback from users label Oct 7, 2022
@darosior
Copy link
Member Author

Looks like https://github.com/bitcoindevkit/rust-electrum-client could be a reasonable choice for an Electrum backend. Depends on their supported MSRV of course.

Or maybe even BDK? It looks like a lot of their features have been made optional, so we could just use it as the Electrum backend without pulling more dependencies than their own code. That sounds reasonable to both not reinvent the wheel and concentrate efforts there should we need improvements or have bug fixes. Provided there is a decent MSRV too.
See https://github.com/bitcoindevkit/bdk.

@darosior
Copy link
Member Author

Another alternative. Sexier but that requires more work, is supporting BIP158 using Nakamoto. https://github.com/cloudhead/nakamoto

@darosior
Copy link
Member Author

Looks like BDK's MSRV is 1.46. Which works for us. But i think they are going for 1.48 now, which is still fine.

@vincenzopalazzo
Copy link

Another alternative. Sexier but that requires more work, is supporting BIP158 using Nakamoto. https://github.com/cloudhead/nakamoto

I like to have this alternative, and I will put some more love on it in the near future, so if you think to reconsidered it we may want to talk some day

@darosior
Copy link
Member Author

darosior commented May 14, 2023 via email

@vincenzopalazzo
Copy link

I am in touch with @cloudhead, so I plan to work upstream all my changes, for now, are all inside different PR https://github.com/cloudhead/nakamoto/pulls and I arrived at a proof of concept with a core lightning plugin.

Would it be cool to have a list of things that you need, one of my problems currently is estimating the fee like bitcoind, but I need some free time to think about it harder

@darosior
Copy link
Member Author

To estimate fees you'd need to relay transactions. I though Nakamoto was only about BIP158.

@vincenzopalazzo
Copy link

Nakamoto has a way to keep track of the fee from the block processing message, I also exposed an API with this commit cloudhead/nakamoto@acc70aa, but it is a monkey way.

I need to think more hard about how to do this.

To estimate fees you'd need to relay transactions.

I had no real-world experience with this, so I think I need to make a more deep reading of the estimation fee of Bitcoin core, but I guess we could use in nakamoto the BIP133 as Neutrino does

I though Nakamoto was only about BIP158.

What do you mean? The BIP158 is the definition of the Golomb-Coded Sets, but nakamoto is offering an light way way to access the bitcoin network like neutrino does

@siwatanejo
Copy link

but nakamoto is offering an light way way to access the bitcoin network like neutrino does

But this way of accessing the blockchain still requires more data downloading than another kind of light client, for example: electrum client. Right?

I need to think more hard about how to do this.

Did you come up with something in the past months? If not, I'm thinking that querying some wellknown online services (e.g. mempool blockexplorer) for this might not be ideal, but a temporary solution until a better one can be thought of?

@vincenzopalazzo
Copy link

Did you come up with something in the past months?

Yes, I had some private problem to solve, but I have a couple of branch that implements the nakamoto support, so I will make PoC by the end of the month

@darosior
Copy link
Member Author

But this way of accessing the blockchain still requires more data downloading than another kind of light client, for example: electrum client. Right?

Yes.

If not, I'm thinking that querying some wellknown online services (e.g. mempool blockexplorer) for this might not be ideal, but a temporary solution until a better one can be thought of?

Sure, a PR to implement Electrum support would be welcome. Note it's not as trivial a task as you might think though.

On a related note, we (Wizardsardine) have been working on a new service. There is discussions about integrating this service to Liana here, which would allow one to not need bitcoind and access other services provided by this backend.

@siwatanejo
Copy link

Note it's not as trivial a task as you might think though.

Let me guess, the thing that inspired you to say this is: server selection (algo or UI), right?

@darosior
Copy link
Member Author

More like adapting all our Bitcoin logic to support both bitcoind and Electrum: https://github.com/wizardsardine/liana/tree/master/src/bitcoin.

@vincenzopalazzo
Copy link

In here there is a logic to use nakamoto https://github.com/vincenzopalazzo/liana/tree/macros/nakamoto

I need just to spend some time to understand how track the transaction and avoid liana crashing

@nondiremanuel
Copy link
Collaborator

We will try to address the Electrum part (inserting it into v7 for this reason). I paste here the possible approach proposed by @darosior, to be discussed. We should divide the activity in 3/4 parts:

  1. Track transactions in our own database, not in bitcoind's watchonly wallet (remove the last dependency on bitcoind)
  2. [optional] Make the API with the "Bitcoin Backend" more Electrum-friendly as right now it's very bitcoind-focused
  3. Implement a "Bitcoin Backend" which uses BDK to sync against Electrum with minimal dependencies
  4. Introduce it in the GUI as a configuration

In order to foster the discussion, I tag @jp1ac4 (in case you wanted to share some thoughts on the approach). Thanks!

@nondiremanuel nondiremanuel moved this from Todo to To be discussed / Defined in Liana General Jun 21, 2024
@nondiremanuel nondiremanuel added this to the Liana v7 milestone Jun 25, 2024
@nondiremanuel nondiremanuel moved this from To be discussed / defined to In Progress in Liana General Jul 10, 2024
@jp1ac4
Copy link
Collaborator

jp1ac4 commented Jul 10, 2024

We should divide the activity in 3/4 parts:

Quick update: I've created #1180 to address Step 1 and am currently working on Step 3 (as part of which I will probably identify and make changes towards Step 2).

darosior added a commit that referenced this issue Aug 2, 2024
bf1e90e sqlite: merge two migration tests (jp1ac4)
e883675 sqlite: add a unit test for migration between v4 and v5 (Antoine Poinsot)
af5fddf commands: use database for TxGetter (jp1ac4)
a86d12d commands: get wallet transactions from db (jp1ac4)
afa6a51 poller: save transactions in database (jp1ac4)
ba4c819 sqlite: separate DB migration from constructor (Antoine Poinsot)
50e7ffa lib: setup the connection to bitcoind before the connection to SQLite. (Antoine Poinsot)

Pull request description:

  This is the first step of #56 (comment).

  The poller will now save transactions in our own database. These transactions are selected based on the deposit and spend transactions of coins. Only the txid and transaction itself are saved, with the corresponding block height and time taken from the coins table.

  In a couple of follow-up commits, I've replaced some RPC calls to bitcoind with DB queries.

ACKs for top commit:
  darosior:
    re-ACK bf1e90e

Tree-SHA512: a1d0a6381efe307655b94a3ff257c58e4d921e98a7fa79e5c9f80016c19df761b10266d4122cb290b78424c5e2acefc163683fcfc948950e3c838e39ba31ba57
@jp1ac4 jp1ac4 mentioned this issue Aug 8, 2024
7 tasks
@nondiremanuel nondiremanuel removed this from the v7 - Liana milestone Sep 4, 2024
darosior added a commit that referenced this issue Sep 5, 2024
341f940 func test: prevent disconnects when using mocktime (Michael Mallan)
b630d46 func test: wait for block heights to match (Michael Mallan)
ada89c5 doc: update with electrum info (Michael Mallan)
177dfc4 lib: expose BDK's electrum client (Michael Mallan)
72c6314 ci: add electrs to func tests (jp1ac4)
371e31e func test: allow to run using electrs backend (jp1ac4)
a85d488 func test: allow for different bitcoin backends (jp1ac4)
1b04b29 func test: fix min rbf feerate (jp1ac4)
c7ee862 bitcoin: add electrum backend (jp1ac4)
c4c2424 bitcoin: expose MempoolEntryFees (jp1ac4)
5011ad9 bitcoin: return spent block height & time separately (jp1ac4)
689442c bitcoin: allow to store UTXO deriv index (jp1ac4)
4c02b0d bitcoin: use mut ref for start_rescan (Michael Mallan)
69259c1 poller: sync wallet before checking updates (jp1ac4)
89e004d bitcoin: add sync_wallet method to interface (jp1ac4)
34b9a49 config: add general bitcoin backend option (jp1ac4)
e267f66 descriptors: allow to get underlying public key (jp1ac4)

Pull request description:

  This is to add the Electrum backend as part of #56 (comment).

  This requires the database to be running version 5 following #1180. The migration from a previous DB version must be done using bitcoind. Thereafter, the daemon can be run using an Electrum backend by replacing the `[bitcoind_config]` section in the daemon.toml config file with:
  ```
  [electrum_config]
  addr = "127.0.0.1:50001" # adjust IP:port as required
  ```
  Remaining tasks:

  - [x] Include ancestors and descendants when getting a transaction's `MempoolEntry`
  - [ ] Additional sanity checks, e.g. check Electrum version
  - [x] Check if logic regarding ongoing rescan needs to be adjusted
  - [x] Add Electrum backend to functional tests
  - [x] Add Electrum backend to CI pipline functional tests
  - [ ] Add unit tests
  - [x] Update README & other docs

ACKs for top commit:
  darosior:
    re-ACK 341f940

Tree-SHA512: dbbb375123cc5c566f5c8535d002ac4491b4be5ab2766845b5b7ab51d19e7e85eafc0097235e1ebc5c5a049bd2222ab5582264314e46c77b5fff8027da31b803
@pythcoiner
Copy link
Collaborator

@nondiremanuel we now have 2 backend as alternative to bitcoind, not sure if we plan to have more? maybe we can close this issue?

@darosior
Copy link
Member Author

darosior commented Nov 4, 2024

Don't know what's planned or not, but block filters would be a nice backend to have once BDK integrates it.

@vincenzopalazzo
Copy link

vincenzopalazzo commented Nov 5, 2024 via email

@pythcoiner
Copy link
Collaborator

en a while back a PR to try integrate nakamoto (kyoto is better anyway because use pure async await) but there was some feature missing.

iirc #909 was blocking for your PR? @darosior how much work still to do to make it mergeable? maybe i can try to finalize it

@vincenzopalazzo
Copy link

vincenzopalazzo commented Nov 5, 2024 via email

@pythcoiner
Copy link
Collaborator

that lian was tracking the txs through bitcoin core, but when you use anothe

iirc, the tx are now track in the db, @jp1ac4?

@jp1ac4
Copy link
Collaborator

jp1ac4 commented Nov 5, 2024

iirc, the tx are now track in the db, @jp1ac4?

Yep, we store transactions in our own DB now and we may be able to reuse some of the BDK-related poller logic that we added for the Electrum backend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or functionality. RFC Needs conceptual discussion and/or feedback from users
Projects
Status: In Review
Development

No branches or pull requests

6 participants