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

BDK wallet integration #110

Merged
merged 48 commits into from
Feb 1, 2024
Merged

BDK wallet integration #110

merged 48 commits into from
Feb 1, 2024

Conversation

evanlinjin
Copy link
Contributor

@evanlinjin evanlinjin commented Jan 9, 2024

This is still WIP.

Architectural Decisions:

I decided to not use https://github.com/bitcoindevkit/bdk-ffi/tree/master as I wanted more flexibility with how wallets are persisted (right now). In the future we want to store utreexo proofs in the wallet, use chain.BlockChain as BDK's ChainOracle, etc. To do this, we should use bdk_chain structures directly and move away from bdk::Wallet (which bdk-ffi only supports).

How to build:

Run buildbdkgo.sh from the root dir:

$ ./buildbdkgo.sh

TODO:

- [ ] Figure out where to put libbdkgo static library and have it available for multiple targets.

  • Have a better script for compiling static library.
  • Expose method(s) for creating and signing transactions.
  • Implement RPC interface for wallet stuff.

wallet/bdkgo/bdkgo.go Outdated Show resolved Hide resolved
wallet/bdkgo/bdkgo.go Outdated Show resolved Hide resolved
danielabrozzoni added a commit to bitcoindevkit/bdk that referenced this pull request Jan 15, 2024
bf67519 feat(chain): add `LocalChain::disconnect_from` method (志宇)

Pull request description:

  Closes #1271

  ### Description

  Add a method for disconnecting a chain of blocks starting from the given `BlockId`.

  ### Notes to the reviewers

  I want to have this for utreexo/utreexod#110

  ### Changelog notice

  Added
  * `LocalChain::disconnect_from` method to evict a chain of blocks starting from a given `BlockId`.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature

ACKs for top commit:
  danielabrozzoni:
    ACK bf67519

Tree-SHA512: e6bd213b49b553355370994567722ad2c3460d11fcd81adc65a85e5d03822d3c38e4a4d7f967044991cf0187845467b67d035bf8904871f9fcc4ea61be761ef7
@evanlinjin evanlinjin force-pushed the bdk_wallet branch 7 times, most recently from 7abfd31 to 9e5e499 Compare January 26, 2024 07:22
@evanlinjin evanlinjin changed the title Intial work on BDK wallet integration BDK wallet integration Jan 26, 2024
@evanlinjin evanlinjin force-pushed the bdk_wallet branch 2 times, most recently from 8440267 to cf99f29 Compare January 29, 2024 05:58
@evanlinjin
Copy link
Contributor Author

The last remaining task is to expose BDK wallet commands via RPC. @kcalvinalvin will handle this!

@kcalvinalvin
Copy link
Contributor

For the linter failing on generated files, you can apply the below to goclean.sh:

diff --git a/goclean.sh b/goclean.sh
index e77d580b..cadfaa7a 100755
--- a/goclean.sh
+++ b/goclean.sh
@@ -14,4 +14,5 @@ golangci-lint run --deadline=10m --disable-all \
 --enable=gofmt \
 --enable=vet \
 --enable=gosimple \
---enable=unconvert
+--enable=unconvert \
+--skip-dirs=bdkwallet/bdkgo # these are generated files

@kcalvinalvin
Copy link
Contributor

Two initial requests for splitting up the PR for easier review/merging:

1: Could we make c9e4326 into a separate PR? It can be an immediate merge.
2: Could we make the code for generating the bindings a separate commit as well?

@evanlinjin
Copy link
Contributor Author

@kcalvinalvin also mentioned to add doc comments to all exported Golang code.

@kcalvinalvin
Copy link
Contributor

bdk wallet should be enabled by default. We should make users disable it if they don't want it.

diff --git a/config.go b/config.go
index ac7cb9e3..cebacb72 100644
--- a/config.go
+++ b/config.go
@@ -216,7 +216,7 @@ type config struct {
        RegisterAddressToWatchOnlyWallet                     []string `long:"registeraddresstowatchonlywallet" description:"Registers addresses to be watched to the watch only wallet. Must have --watchonlywallet enabled"`
        RegisterExtendedPubKeysToWatchOnlyWallet             []string `long:"registerextendedpubkeystowatchonlywallet" description:"Registers extended pubkeys to be watched to the watch only wallet. Must have --watchonlywallet enabled."`
        RegisterExtendedPubKeysWithAddrTypeToWatchOnlyWallet []string `long:"registerextendedpubkeyswithaddresstypetowatchonlywallet" description:"Registers extended pubkeys to be watched to the watch only wallet and let's the user override the hd type of the extended public key. Must have --watchonlywallet enabled. Format: '<extendedpubkey>:<address type>. Supported address types: '{p2pkh, p2wpkh, p2sh}'"`
-       BdkWallet                                            bool     `long:"bdkwallet" description:"Enable the BDK wallet."`
+       NoBdkWallet                                          bool     `long:"nobdkwallet" description:"Disable the BDK wallet."`
 
        // Electrum server options.
        ElectrumListeners    []string `long:"electrumlisteners" description:"Interface/port for the electrum server to listen to. (default 50001). Electrum server is only enabled when --watchonlywallet is enabled"`
diff --git a/server.go b/server.go
index 5b524a64..ee4ada32 100644
--- a/server.go
+++ b/server.go
@@ -3533,7 +3533,7 @@ func newServer(listenAddrs, agentBlacklist, agentWhitelist []string,
                }
        }
 
-       if cfg.BdkWallet {
+       if !cfg.NoBdkWallet {
                // Setup BDK wallet if it is enabled.
                s.bdkWallet, err = bdkwallet.NewManager(bdkwallet.ManagerConfig{
                        Chain:       s.chain,
diff --git a/utreexod.go b/utreexod.go
index 38981b22..479a53b0 100644
--- a/utreexod.go
+++ b/utreexod.go
@@ -185,7 +185,7 @@ func btcdMain(serverChan chan<- *server) error {
                        btcdLog.Error(err)
                        return err
                }
-               if walletDirExists && !cfg.BdkWallet {
+               if walletDirExists && cfg.NoBdkWallet {
                        err = fmt.Errorf("BDK wallet is disabled but this node has been previously "+
                                "started with BDK wallet enabled with walletdir of \"%v\". Please "+
                                "completely remove the walletdir to start the node without wallet.",

@kcalvinalvin
Copy link
Contributor

Could we also add make clean and make all?

diff --git a/Makefile b/Makefile
index 3ae8d068..4f238250 100644
--- a/Makefile
+++ b/Makefile
@@ -16,3 +16,10 @@ build-utreexod: build-bdk ## Build utreexod with all features
 
 build-utreexod-without-bdk: ## Build utreexod without BDK wallet
        go build
+
+all: build-bdk build-utreexod
+
+clean:
+       rm utreexod
+       go clean
+       cargo clean

bdkwallet/manager.go Outdated Show resolved Hide resolved
kcalvinalvin and others added 24 commits February 1, 2024 15:09
Instead of binary bytes of the expected spk.
rpcserver.NotifyNewTransactions() were not notifying the bdkwallet and
the watchonly wallet of new transactions that were submitted to the
node previously.
Just generally making the code idiomatic.
It rebroadcasts the unconfirmed txs that are stored on the bdk side to
peers connected to the node. A node could be shut down after it has
passed off the tx to bdk but before it has sent the txs off to other
peers. Having this rpc makes it so that the user can manually trigger
rebroadcasts.
Since everyone reverses the hashes, we also need to reverse them so it
looks right.
@kcalvinalvin kcalvinalvin marked this pull request as ready for review February 1, 2024 06:11
@kcalvinalvin kcalvinalvin merged commit 7e04252 into utreexo:main Feb 1, 2024
4 checks passed
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.

2 participants