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

make lwk available #288

Merged
merged 29 commits into from
Jul 5, 2024
Merged

Conversation

YusukeShimizu
Copy link
Contributor

@YusukeShimizu YusukeShimizu commented Mar 11, 2024

lbtc swap with lwk

Description

Previously, lbtc swap required running elementsd, but now it can be done with lwk.
Because elementsd requires a lot of resources, making it available in lwk will allow more users to perform swaps.

setup

see docs/setup_lwk.md.

Specification

lwk and esplora-electrs connection is required.
New json rpc api client is implemented for connection to lwk.

Connection to esplora-electrs using go-electrum.
esplora-electrs is also used by lwk as a blockchain.

Configuration

The user can choose to use lwk instead of elements for lbtc swap.
It has the following configuration items.

type LWKConf struct {
	SignerName       string
	WalletName       string
	LWKEndpoint      string
	ElementsEndpoint string
	Network          string
	LiquidSwaps      *bool
}

The user can choose to set up a wallet that holds the signer.
Default uses the same default endpoint (blockstream.info:995) as lwk, but the user can choose his own electrum for ElementsEndpoint.

wallet

Peerswap's onchain wallet provide the following features.

type Wallet interface {
	GetAddress() (string, error)
	SendToAddress(string, uint64) (string, error)
	GetBalance() (uint64, error)
	CreateAndBroadcastTransaction(swapParams *swap.OpeningParams, asset []byte) (txid, rawTx string, fee uint64, err error)
	SendRawTx(rawTx string) (txid string, err error)
	GetFee(txSize int64) (uint64, error)
}

GetAddress() (string, error)

This method is used to get the address of the wallet.
Use lwk address.

SendToAddress(string, uint64) (string, error)

This method is used to send money to a specific address.
lwk send to create a psbt, sign to sign it, and broadcast to broadcast it.

CreateAndBroadcastTransaction(swapParams *swap.OpeningParams, asset []byte) (txid, rawTx string, fee uint64, err error)

This method is used to create and broadcast opening tx.
The interface has been split into the following two interfaces,
but they will be merged into a single interface for lwk support.
Existing bitcoin wallets will also be changed, but functionality will remain the same.

CreateFundedTransaction(preparedTx *transaction.Transaction) (rawTx string, fee uint64, err error)
FinalizeFundedTransaction(unpreparedTx string) (preparedTxHex string, err error)

SendRawTx(rawTx string) (txid string, err error)

This method is used to broadcast the closing tx.
This is done using electrum cient's blockchain.transaction.broadcast method.

GetFee(txSize int64) (uint64, error)

This method is used to get the fee blockchain.estimatefee.

tx watcher

tx watcher is a function that monitors onchain status and notifies opening tx and closing tx.
The following interface must be implemented.

type TxWatcher interface {
	AddWaitForConfirmationTx(swapId, txId string, vout, startingHeight uint32, scriptpubkey []byte)
	AddWaitForCsvTx(swapId, txId string, vout uint32, startingHeight uint32, scriptpubkey []byte)
	AddConfirmationCallback(func(swapId string, txHex string, err error) error)
	AddCsvCallback(func(swapId string) error)
	GetBlockHeight() (uint32, error)
	StartWatchingTxs() error
}

So far, peerswap has used bitcoind/elementsd, but lwk uses the electrum client on which lwk depends.

AddWaitForConfirmationTx(swapId, txId string, vout, startingHeight uint32, scriptpubkey []byte)

Wait for a specific tx to be confirmed with a specific block height.
This function uses the electrum client blockchain.scriptthash.get_history.

AddWaitForCsvTx(swapId, txId string, vout uint32, startingHeight uint32, scriptpubkey []byte)

Wait for a specific tx to reach the csv limit at a specific block height.
This function uses the electrum client blockchain.scriptthash.get_history.

GetBlockHeight() (uint32, error)

Function to retrieve block height.
Use blockchain.headers.subscribe of electrum client.

StartWatchingTxs() error

Function to start tx watcher.
Periodically calls blockchain.scripthash.get_history of electrum client and executes callback when the condition is met.

@YusukeShimizu YusukeShimizu force-pushed the lwk-swap branch 8 times, most recently from 8603558 to 37f5bcd Compare March 29, 2024 07:51
remove blockwatcher.go and associated block height functions
The blockwatcher.go file and its associated functions for
tracking block height changes have been removed.

block height tracking is no longer necessary and has been replaced
by a different mechanism.
@YusukeShimizu YusukeShimizu force-pushed the lwk-swap branch 3 times, most recently from 02832df to e7513c6 Compare May 1, 2024 00:47
`CreateFundedTransaction` and `FinalizeFundedTransaction`
have merged interfaces, CreateAndBroadcastTransaction.
This is because there was originally no reason to separate them,
and because lwk uses the psbt format,
transacrion conversion needs to be performed.

The liquid wallet interface should not have a
unique liquid wallet-dependent definition.
However, there were some that depended on gelements,
 so they were separated out.
lwkclient is a library for interacting directly with
lwk server via json rpc.
LWKRpcWallet acts as an onchain wallet for peerswap
It uses lwk and electrs.

- Implement API structure with request handling,
including retry logic and response body draining.
- Provide configuration options for HTTP client with retry logic
- Utilize JSON-RPC 2.0 for request and response handling.
- Ensure proper error handling and logging.

- Create lwkclient with methods for address generation,
sending transactions, signing, broadcasting,
balance querying, and wallet details retrieval.

- Implement LWKRpcWallet with necessary wallet operations
- Ensure wallet setup and transaction creation/broadcasting
- Add utility functions for balance retrieval and fee estimation.
tx watcher is a function that monitors onchain status
and notifies opening tx and closing tx.

Introduce ElectrumRPC interface and electrumTxWatcher
for Electrum-based transaction watching.

Also, add generated mock for electrumRPC interface.
Mock generation is performed by go.uber.org/mock/mockgen.
Input can be in toml or ini format,
but each is managed as the same config.

To make it loosely coupled, used the builder pattern to set item.
I also add validation of the config.
The config file cannot be modified from the outside.
@YusukeShimizu YusukeShimizu force-pushed the lwk-swap branch 2 times, most recently from 48f67fd to e7769ed Compare May 2, 2024 06:08
packages.nix Outdated
Comment on lines 19 to 31
# lwk: init at 0.3.0 #292522
# https://github.com/NixOS/nixpkgs/pull/292522/commits/2b3750792b2e4b52f472b6e6d88a6b02b6536c43
rev2 = "2b3750792b2e4b52f472b6e6d88a6b02b6536c43";
nixpkgs2 = fetchNixpkgs rev2;
pkgs2 = import nixpkgs2 {};
# blockstream-electrs: init at 0.4.1 #299761
# https://github.com/NixOS/nixpkgs/pull/299761/commits/680d27ad847801af781e0a99e4b87ed73965c69a
rev3 = "680d27ad847801af781e0a99e4b87ed73965c69a";
nixpkgs3 = fetchNixpkgs rev3;
pkgs3 = import nixpkgs3 {};
blockstream-electrs = pkgs3.blockstream-electrs.overrideAttrs (oldAttrs: {
cargoBuildFlags = [ "--features liquid" "--bin electrs" ];
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also nix flakes for lwk and blockstream-electrs, I am curious to know if it was unknown or if it's preferred to avoid flakes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for confirming this.
I know that flake exists and I'm not trying to avoid installing it.

I don't know enough about nix but would it be better to add binLiquid to the package and use it for peerswap devShell?
I created draft PR of this.

I also seem to get build errors.
I think it should be fixed.

nix run .#blockstream-electrs-liquid -- --network liquid
error: builder for '/nix/store/nkkv9v3r310p5gp09zv3hmn29y63mia3-electrs-deps-0.4.1.drv' failed with exit code 101;
       last 25 log lines:
       >     Checking futures-channel v0.3.30
       >     Checking which v4.4.2
       >    Compiling bitcoind v0.34.1
       >     Checking rand v0.4.6
       >     Checking rustc-demangle v0.1.23
       >     Checking iana-time-zone v0.1.60
       >     Checking unicode-width v0.1.11
       >     Checking tower-service v0.3.2
       >     Checking winapi v0.2.8
       >     Checking option-ext v0.2.0
       >    Compiling bitcoin_hashes v0.10.0
       >     Checking base64-compat v1.0.0
       >     Checking dirs-sys v0.4.1
       >     Checking hyper v0.14.28
       >     Checking bitcoincore-rpc-json v0.18.0
       >    Compiling elementsd v0.9.1
       > error[E0425]: cannot find function `download_filename` in this scope
       >   --> /nix/store/n1a9vx4rppg99v909c7hw2jc5d1yj7f2-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/elementsd-0.9.1/build.rs:59:33
       >    |
       > 59 |         let download_filename = download_filename();
       >    |                                 ^^^^^^^^^^^^^^^^^ not found in this scope
       >
       > For more information about this error, try `rustc --explain E0425`.
       > error: could not compile `elementsd` (build script) due to previous error
       > warning: build failed, waiting for other jobs to finish...
       For full logs, run 'nix-store -l /nix/store/nkkv9v3r310p5gp09zv3hmn29y63mia3-electrs-deps-0.4.1.drv'.
error: 1 dependencies of derivation '/nix/store/qpdjg9rmj3b9w33l50gmg4jdl8671738-electrs-0.4.1.drv' failed to build

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which commit are you using?

current new-index branch seems fine to me

$ git log | head -n 1
commit efc1fec8b0f96b5663d7257a0c2cffd8ef143219
$ nix run .#blockstream-electrs-liquid -- --network liquid
Config { log: StdErrLog { verbosity: Error, quiet: false, show_level: true, timestamp: Off, modules: [], writer: "stderr", color_choice: Auto, show_module_names: false }, network_type: Liquid, db_path: "./db/liquid", daemon_dir: "/home/casatta/.bitcoin/liquidv1", blocks_dir: "/home/casatta/.bitcoin/liquidv1/blocks", daemon_rpc_addr: 127.0.0.1:7041, cookie: None, electrum_rpc_addr: 127.0.0.1:51000, http_addr: 127.0.0.1:3000, http_socket_file: None, monitoring_addr: 127.0.0.1:34224, jsonrpc_import: false, light_mode: false, address_search: false, index_unspendables: false, cors: None, precache_scripts: None, utxos_limit: 500, electrum_txs_limit: 500, electrum_banner: "Welcome to electrs-esplora 0.4.1", electrum_rpc_logging: None, parent_network: Bitcoin, asset_db_path: None }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the same commit.
There seems to be an error in my environment.

$ git log | head -n 1
commit efc1fec8b0f96b5663d7257a0c2cffd8ef143219

$ nix run .#blockstream-electrs-liquid -- --network liquid
error: builder for '/nix/store/nkkv9v3r310p5gp09zv3hmn29y63mia3-electrs-deps-0.4.1.drv' failed with exit code 101;
       last 25 log lines:
       >     Checking rand v0.4.6
       >     Checking tower-service v0.3.2
       >     Checking option-ext v0.2.0
       >     Checking winapi v0.2.8
       >     Checking rustc-demangle v0.1.23
       >     Checking unicode-width v0.1.11
       >    Compiling bitcoin_hashes v0.10.0
       >     Checking iana-time-zone v0.1.60
       >     Checking bitcoincore-rpc-json v0.18.0
       >     Checking base64-compat v1.0.0
       >     Checking chrono v0.4.34
       >     Checking textwrap v0.11.0
       >     Checking rand v0.3.23
       >     Checking hyper v0.14.28
       >     Checking dirs-sys v0.4.1
       >    Compiling elementsd v0.9.1
       > error[E0425]: cannot find function `download_filename` in this scope
       >   --> /nix/store/n1a9vx4rppg99v909c7hw2jc5d1yj7f2-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/elementsd-0.9.1/build.rs:59:33
       >    |
       > 59 |         let download_filename = download_filename();
       >    |                                 ^^^^^^^^^^^^^^^^^ not found in this scope
       >
       > For more information about this error, try `rustc --explain E0425`.
       > error: could not compile `elementsd` (build script) due to previous error
       > warning: build failed, waiting for other jobs to finish...
       For full logs, run 'nix log /nix/store/nkkv9v3r310p5gp09zv3hmn29y63mia3-electrs-deps-0.4.1.drv'.
error: 1 dependencies of derivation '/nix/store/qpdjg9rmj3b9w33l50gmg4jdl8671738-electrs-0.4.1.drv' failed to build

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you try a nix flake update before nix run I am thinking to update the flake.lock anyway...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, doesn't seem to solve the problem.

$ nix flake update
warning: updating lock file '/workspaces/electrs/flake.lock':
• Updated input 'crane':
    'github:ipetkov/crane/a329cd00398379c62e76fc3b8d4ec2934260d636?narHash=sha256-iZDHWTqQj6z6ccqTSEOPOxQ8KMFAemInUObN2R9vHSs%3D' (2024-03-28)
  → 'github:ipetkov/crane/442a7a6152f49b907e73206dc8e1f46a61e8e873?narHash=sha256-uXNW6bapWFfkYIkK1EagydSrFMqycOYEDSq75GmUpjk%3D' (2024-05-04)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/2726f127c15a4cc9810843b96cad73c7eb39e443?narHash=sha256-UKcYiHWHQynzj6CN/vTcix4yd1eCu1uFdsuarupdCQQ%3D' (2024-03-27)
  → 'github:NixOS/nixpkgs/b211b392b8486ee79df6cdfb1157ad2133427a29?narHash=sha256-CLU5Tsg24Ke4%2B7sH8azHWXKd0CFd4mhLWfhYgUiDBpQ%3D' (2024-05-07)
• Updated input 'rust-overlay':
    'github:oxalica/rust-overlay/aa858717377db2ed8ffd2d44147d907baee656e5?narHash=sha256-oD4OJ3TRmVrbAuKZWxElRCyCagNCDuhfw2exBmNOy48%3D' (2024-03-28)
  → 'github:oxalica/rust-overlay/a8bfc2569a1965c0da8711d289d973f0011b441a?narHash=sha256-oujsCgNiQnZoQntNkkNkA7BhCmUvf9FLWj%2B2oGT2Jvc%3D' (2024-05-08)
warning: Git tree '/workspaces/electrs' is dirty
$ nix run .#blockstream-electrs-liquid -- --network liquid
warning: Git tree '/workspaces/electrs' is dirty
error: builder for '/nix/store/rcd2hcxdnb6v23c7mjjm33wqjbasdrvr-electrs-deps-0.4.1.drv' failed with exit code 101;
       last 25 log lines:
       >    Compiling bitcoin_hashes v0.12.0
       >     Checking object v0.32.2
       >     Checking futures-channel v0.3.30
       >     Checking which v4.4.2
       >    Compiling bitcoind v0.34.1
       >     Checking rand v0.4.6
       >     Checking iana-time-zone v0.1.60
       >     Checking tower-service v0.3.2
       >     Checking rustc-demangle v0.1.23
       >     Checking unicode-width v0.1.11
       >    Compiling bitcoin_hashes v0.10.0
       >     Checking bitcoincore-rpc-json v0.18.0
       >     Checking option-ext v0.2.0
       >     Checking winapi v0.2.8
       >    Compiling elementsd v0.9.1
       > error[E0425]: cannot find function `download_filename` in this scope
       >   --> /nix/store/cjjfa06qzk9wahmbqjh7kn86bsmaq3dv-vendor-cargo-deps/c19b7c6f923b580ac259164a89f2577984ad5ab09ee9d583b888f934adbbe8d0/elementsd-0.9.1/build.rs:59:33
       >    |
       > 59 |         let download_filename = download_filename();
       >    |                                 ^^^^^^^^^^^^^^^^^ not found in this scope
       >
       > For more information about this error, try `rustc --explain E0425`.
       >     Checking dirs-sys v0.4.1
       > error: could not compile `elementsd` (build script) due to previous error
       > warning: build failed, waiting for other jobs to finish...
       For full logs, run 'nix log /nix/store/rcd2hcxdnb6v23c7mjjm33wqjbasdrvr-electrs-deps-0.4.1.drv'.
error: 1 dependencies of derivation '/nix/store/z7wa0ixizb5xq32dfs8s0fv23yds4lcl-electrs-0.4.1.drv' failed to build

The integration test for LWK is added to ensure
that the swap functionality works as expected
with the LWK and electrs support.

The setup functions are implemented to create the necessary test
environment, including lwk, Bitcoin and Liquid nodes,
 as well as two lightning nodes with the PeerSwap.
This setup is crucial for testing the swap-in process
in an environment that closely mimics production.

also, added Electrs and LWK structs for testing framework.
- Implement `Electrs` struct to manage electrs daemon processes
- Implement `LWK` struct to manage LWK daemon processes
- Provide constructors for both structs to
initialize with dynamic ports and configurations
- Include methods to run the processes and connect
@YusukeShimizu YusukeShimizu changed the title wip: make lwk available make lwk available May 6, 2024
@YusukeShimizu YusukeShimizu marked this pull request as ready for review May 6, 2024 01:05
@grubles
Copy link
Collaborator

grubles commented May 8, 2024

CLN as swap-out sender seems to be unable to get the starting block height from LWK?

Using CLN 24.02.2 and LWK 0.3.0

2eb4834f573c060d9200ce77544a29b48a0aa5923 got msgtype: a45d for swap: efa2c9c38ea24acff600af12d61677f50679b0e2d137fcd037c760411eaa4cfc
2024-05-08T19:09:05.555Z DEBUG   plugin-peerswap: [FSM] event:id: efa2c9c38ea24acff600af12d61677f50679b0e2d137fcd037c760411eaa4cfc, Event_OnTxOpenedMessage on State_SwapOutSender_AwaitTxBroadcastedMessage
2024-05-08T19:09:05.584Z INFO    plugin-peerswap: [FSM] Action failure could not get starting block height of the swap.
2024-05-08T19:09:05.585Z DEBUG   plugin-peerswap: [FSM] event:id: efa2c9c38ea24acff600af12d61677f50679b0e2d137fcd037c760411eaa4cfc, Event_ActionFailed on State_SwapOutSender_AwaitTxConfirmation
2024-05-08T19:09:05.591Z DEBUG   plugin-peerswap: [FSM] event:id: efa2c9c38ea24acff600af12d61677f50679b0e2d137fcd037c760411eaa4cfc, Event_ActionSucceeded on State_SwapOutSender_SendPrivkey
2024-05-08T19:09:05.592Z DEBUG   035ca2fe4793a5e789ce846062eb4834f573c060d9200ce77544a29b48a0aa5923-connectd: peer_out INVALID 42081
2024-05-08T19:09:05.599Z DEBUG   plugin-peerswap: [FSM] event:id: efa2c9c38ea24acff600af12d61677f50679b0e2d137fcd037c760411eaa4cfc, Event_ActionSucceeded on State_SwapOutSender_SendCoopClose
2024-05-08T19:09:05.600Z INFO    plugin-peerswap: [Swap:efa2c9c38ea24acff600af12d61677f50679b0e2d137fcd037c760411eaa4cfc] Swap claimed cooperatively

@YusukeShimizu YusukeShimizu force-pushed the lwk-swap branch 4 times, most recently from 44c7d05 to a448d46 Compare May 9, 2024 07:39
ensure client is properly reinitialized before subscribing to headers
 to prevent potential stale connections.
The Event_OnRetry is designed to execute 20 retries and stop
the process if it still fails.

This patch adds the missing Event_OnTimeout transition to
transit to coop close if the lwk is not restarted.
Copy link
Contributor

@nepet nepet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @YusukeShimizu! That was a heavy lift. I really like your interface refinements and the way you seamlessly integrate lwk into the rest of the repo. I wish the rest of the repo would be as clean as the lwk package ^^.

I may have an explanation as to why the liveleyness test using the rpc_command hook resulted in unexpected behaviour, crashing other plugins: The hook will be called on EVERY rpc call globally, regardless the method or the namespace. So if peerswap as a plugin is still active but the connection to gbitcoin or elements is already lost, this results in returning an error on EVERY rpc call made (inter process as well) which may crash other plugins that come after peerswap in the shutdown sequence. A possible fix would have been to only act on methods that are of peerswaps namespace.

I have some minor nits about naming of units - kvB and vB mixups and would suggest to revisit the fee unit conversion unit-test as it seems to be off.

Besides that the PR seemes good to go for me to test it out in the wild.

lwk/lwkwallet.go Outdated Show resolved Hide resolved
electrum/block_subscriber.go Show resolved Hide resolved
electrum/block_subscriber.go Outdated Show resolved Hide resolved
lwk/lwkwallet.go Outdated Show resolved Hide resolved
lwk/lwkwallet.go Outdated Show resolved Hide resolved
lwk/lwkwallet.go Outdated Show resolved Hide resolved
lwk/lwkwallet_test.go Outdated Show resolved Hide resolved
clightning/clightning.go Outdated Show resolved Hide resolved
clightning/clightning.go Outdated Show resolved Hide resolved
lwk/electrumtxwatcher.go Show resolved Hide resolved
@YusukeShimizu YusukeShimizu force-pushed the lwk-swap branch 15 times, most recently from f1671ff to f9463c1 Compare July 1, 2024 01:12
The version of lwk has been raised to cli_0.5.1.
Change to use flake to use cli_0.5.1 in nix as well.
Using flake allows you to run nix develop instead of nix-shell.

Just uses the flake on nix shell.
For the nix-env addon users.
Variable and function names have been corrected
to eliminate confusion with fee calculations.
Test cases have also been refactored.
implement SetLabel method to call walletSetTxMemo
for setting transaction memos.
@YusukeShimizu
Copy link
Contributor Author

Thank you for your review.
Can you please confirm that I have made the following corrections?

  • Migrated nix to nix flake and update lwk version to use wallet_set_tx_memo. 45fb414
    • flake was needed to update the lwk version, but as it is common to use flake, I think this is a good opportunity to switch.
    • In addition, CI is now also faster, around 6 minutes.
  • Refactor of tx fee calculation 7045055
    • Mainly name fixes and test case fixes.
  • set label 66fead0

@YusukeShimizu YusukeShimizu requested a review from nepet July 1, 2024 08:10
@nepet
Copy link
Contributor

nepet commented Jul 1, 2024

Looks good to me!
Ack 66fead0

@grubles
Copy link
Collaborator

grubles commented Jul 4, 2024

Tested ACK 66fead0

@YusukeShimizu YusukeShimizu merged commit 95b9983 into ElementsProject:master Jul 5, 2024
8 checks passed
@YusukeShimizu YusukeShimizu deleted the lwk-swap branch July 5, 2024 10:47
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.

4 participants