Commit Graph

587 Commits (88b3321ec5434ba3b65ada6162b966d1a584f151)

Author SHA1 Message Date
Thomas Eizinger 7387884e6d
Move log messages to the appropriate abstraction layer
Log messages are ideally as close to the functionality they are talking about, otherwise we might end up repeating ourselves on several callsites or the log messages gets outdated if the behaviour changes.
4 years ago
Thomas Eizinger b8df4a3145
Inline tracing configuration for swap_cli
This allows us to configure the presentation separately from the ASB.
4 years ago
Thomas Eizinger a0e7c6ecf7
Don't Arc the AtomicU32
We never clone this type, there is no need to wrap it in an `Arc`.
4 years ago
Thomas Eizinger 40dcf0355a
Simplify `Transfer::transfer` return type
We never use the fee returned from this function, remove it.
4 years ago
bors[bot] 1de3fa486e
Merge #247
247: Calculate max_giveable based on spending script size r=da-kami a=thomaseizinger



Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
4 years ago
Thomas Eizinger 9f0b1c5cbe
Calculate max_giveable based on spending script size 4 years ago
Daniel Karzel 8c40ee1da4 Change anyhow! to bail! in error scenarios 4 years ago
bors[bot] 7251588e79
Merge #233
233: ASB max sell amount r=thomaseizinger a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
bors[bot] 5ddf41721e
Merge #238
238: Bob error handling r=thomaseizinger a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
rishflab 975d604405 Test to ensure default alice peer id and multi addr is valid 4 years ago
rishflab 9a82b572ec Default to buy xmr using default trait 4 years ago
rishflab d6d67f62f1 Swap cli executes BuyXmr path if subcommand is not given 4 years ago
rishflab 60de6a9219 Remove intermediate structs in cli arguments
These intermediate structs were creating unnecessary noise. The peer id
and multiaddr fields are going to be removed in the future further
reducing the need to have seperate structs for cancel, resume and
refund.
4 years ago
bors[bot] 9a32f7d405
Merge #236
236: Some wallet cleanup + watch for deposit r=thomaseizinger a=thomaseizinger



Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
4 years ago
Daniel Karzel bb1537d6f2 Error feedback for the user upon communication errors
If communication with the other party fails the program should stop and the user should see the respective error.
Communication errors are handled in the event-loop. Upon a communication error the event loop is stopped.
Since the event loop is only stopped upon error the Result returned from the event loop is Infallible.

If one of the two futures, event loop and swap,  finishes (success/failure) the other future should be stopped as well.
We use tokio::selec! to stop either future if the other stops.
4 years ago
Daniel Karzel 019d6c725a Maximum sell amount for ASB that defaults to 0.5 XMR 4 years ago
bors[bot] bb0377c6c7
Merge #232
232: ASB only sends quote response if sufficient XMR balance r=da-kami a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
Daniel Karzel e66e84085b Rename Bob's Behavior Failure to CommunicationError
Failure does not express what the error represents. It is only used for communication
errors for quote requests, receiving the XMR transfer proof and sending the encryption signature.
4 years ago
Thomas Eizinger f472070546
Remove `--send-btc` in favor of swapping the available balance
If the current balance is 0, we wait until the user deposits money
to the given address. After that, we simply swap the full balance.

Not only does this simplify the interface by removing a parameter,
but it also integrates the `deposit` command into the `buy-xmr`
command.

Syncing a wallet that is backed by electrum includes transactions
that are part of the mempool when computing the balance.
As such, waiting for a deposit is a very quick action because it
allows us to build our lock transaction on top of the yet to be
confirmed deposit transactions.

This patch introduces another function to the `bitcoin::Wallet` that
relies on the currently statically encoded fee rate. To make sure
future developers don't forget to adjust both, we extract a function
that "selects" a fee rate and return the constant from there.

Fixes #196.
4 years ago
Thomas Eizinger 32cb0eb896
Rename `build_tx_lock_psbt` to `send_to_address`
Being defined on the wallet itself, a more generic name fits better
on what this function actually does.
4 years ago
Thomas Eizinger 67fe01a2ef
Remove `BuildTxLockPsbt` and `GetNetwork` traits
These traits were only used once within the `TxLock` constructor.
Looking at the rest of the codebase, we don't really seem to follow
any abstractions here where the protocol shouldn't know about the
exact types that is being passed in.

As such, these types are just noise and might as well be removed in
favor of simplicity.
4 years ago
Thomas Eizinger 6c38d66864
Remove `Tx` arguments from `add_signatures` functions
The only reason we need this argument is because we need to access
the output descriptor. We can save that one ahead of time at when
we construct the type.
4 years ago
Thomas Eizinger 0f8fbd087f
Make all fields of `bitcoin::Wallet` private
This reveals that the `network` field is actually unused.
4 years ago
Thomas Eizinger 1876d17ba4
Remove `map_err` in favor of `?`
`?` maps the error automatically.
4 years ago
Thomas Eizinger 7d324d966a
Remove `syncing` wallet log
BDK already has a log line for the sync that we could enable if we
wanted such a log.
Additionally, _we_ are not actually syncing the wallet, bdk is so our
log line was lying. It should have said "calling bdk to sync wallet".
4 years ago
bors[bot] 93d59398af
Merge #231
231: Error only on close message when fetching the rate r=thomaseizinger a=da-kami

Ping/Pong messages disturb the rate requests quite frequently resulting in failed swap setup because there is no rate available.

As a result messages Ping, Pong and Binary are now ignored and not reported as error.


Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
Daniel Karzel 1f1b3a95bc Logging for different scenarios when reading from rate stream 4 years ago
rishflab abc9aaa327 Use default alice peer id and multiaddr if not specified in cli args 4 years ago
Daniel Karzel cad6a1c3a7 ABS only sends quote response if sufficient XMR balance 4 years ago
rishflab f52567155a Use default testnet config if config file path not specified 4 years ago
Daniel Karzel fc2c08c7c9 Error only on close message when fetching the rate
Messages Ping, Pong and Binary are ignored and not reported as error.
4 years ago
rishflab 51c16f23d8
Download and run monero wallet rpc on swap cli startup
If the monero wallet rpc has not already been downloaded we download the monero cli package and extract the wallet rpc. The unneeded files are cleaned up. The monero wallet rpc is started on a random port which is provided to the swap cli.

We added a fork of tokio-tar via a git subtree because we needed a tokio-tar version that was compatible with tokio 1.0. Remove this subtree in favor of a regular cargo dependency when this PR merges: https://github.com/vorot93/tokio-tar/pull/3.
4 years ago
Daniel Karzel 0945cee459 Remove traits in favour of public functions 4 years ago
Daniel Karzel 578d23d7fc Proper encapsulation of wallet boundaries through private fields 4 years ago
Daniel Karzel 947bcb6192 ASB reloads the default wallet after generate_from_keys atomically 4 years ago
Daniel Karzel 684cbe4d0b Remember monero wallet-height for Alice's refund scenario 4 years ago
Daniel Karzel fa04775188 Rename function explicit to cancellation to cancel
For transitioning to state4 we either go into a redeem or a cancellation scenario.
The function name state4 is misleading, because it is only used for cancellation scenarios.
4 years ago
Daniel Karzel 1404057dbe Remove misleading TODO
This TDOO is misleading, because - to our current knowledge - it is impossible for
Bob to retrieve the exact inclusion block-height of the lock transaction (send by Alice).
The wallet RPC is only capable of retrieving the inclusion block height of a transaction
through `get_payments` and `get_bulk_payments` which requires the `payment_id`.
The `payment_id` can be retrieved through `get_transfer_by_txid` which states
"Show information about a transfer to/from this address." - however the address that the
transfer goes to is not part of Bob's wallet yet! Thus, it is impossible for Bob to use
`get_transfer_by_txid` which in turn means Bob is unable to use `get_payments`.

The only possible way for Bob to know the exact inclusion block/height of the lock transaction
would be if Alice sends it over to Bob. But for that Alice would have to extract it she would have
to wait for confirmation - which she currently does not and might never do. Even if she does await
the first confirmation before sending the transfer proof the solution for retrieving the inclusion
block-height is not fleshed out on her side yet.
4 years ago
Daniel Karzel 9f1deb9fdc Wrap the Monero wallet client in a Mutex
In order to ensure that we can atomically generate_from_keys and then reload a wallet,
we have to wrap the client of the monero wallet RPC inside a mutex.
When introducing the Mutex I noticed that several inner RPC calls were leaking to the
swap crate monero wallet. As this is a violation of boundaries I introduced the traits
`GetAddress`, `WalletBlockHeight` and `Refresh`.

Note that the monero wallet could potentially know its own public view key and
public spend key. If we refactor the wallet to include this information upon wallet
creation we can also generate addresses using `monero::Address::standard`.
4 years ago
Daniel Karzel aed8358fb7 Remove dead code 4 years ago
bors[bot] 2654879ff3
Merge #218
218: Cleanup dependencies r=thomaseizinger a=thomaseizinger

Fixes https://github.com/comit-network/xmr-btc-swap/issues/208.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
4 years ago
bors[bot] fb2057453a
Merge #219 #221
219: Rename variables to add to understanding the code r=da-kami a=da-kami



221: Fix/improve comment explaining TxRefund encsigning r=thomaseizinger a=rishflab



Co-authored-by: Daniel Karzel <daniel@comit.network>
Co-authored-by: rishflab <rishflab@hotmail.com>
4 years ago
Thomas Eizinger 66db8e1851
Remove unnecessary log dependency
By updating `tracing_log`, we can access the re-export. That we need
to initialize the `tracing_log` adaptor.

The usage of `log::LevelFilter` for the `init_tracing` function was
conceptually incorrect. We should be using a type from the `tracing`
library here.
4 years ago
Thomas Eizinger 03078f328c
Split monero-harness into harness and rpc
This allows us to move `monero-harness` and `bitcoin-harness` into
`[dev-dependencies]` of `swap`.
4 years ago
Thomas Eizinger 2a3db9bd80
Remove unnecessary derivative dependency 4 years ago
Thomas Eizinger f0ba80794c
Remove unnecessary serde_derive dependency declaration
We already express the same thing through the serde `derive` feature.
4 years ago
Thomas Eizinger d54fac6fd9
Remove unnecessary tempfile prod dependency 4 years ago
Thomas Eizinger 729f4f09a8
Remove unnecessary tracing_core dependency 4 years ago
Thomas Eizinger 418aa02191
Remove unnecessary ed25519-dalek dependency 4 years ago
rishflab 8280072400 Fix/improve comment explaining TxRefund encsigning 4 years ago
Daniel Karzel 151f33ba10 Rename variables to add to understanding the code 4 years ago
bors[bot] 61a8282be1
Merge #203
203: Introduce dynamic rates r=da-kami a=D4nte



Co-authored-by: Franck Royer <franck@coblox.tech>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
Daniel Karzel b4ceee49df Change monitoring to default wallet
The automated swap backend (asb) requires Monero funds, because Alice is selling Monero.
We use a hardcoded default wallet named asb-wallet. This wallet is opened upon startup.
If the default wallet does not exist it will be created.
4 years ago
Daniel Karzel 9496dce917 Skip heartbeat messages 4 years ago
Thomas Eizinger a8bfc1d686 Make LatestRate::Error require std::error::Error trait bound
This allows us to use .context instead of .map_err when calling
`latest_rate()`. For the static rate module, we simply fill in
`Infallible` which is actually better suited because it describes
that we are never using this error.
4 years ago
Thomas Eizinger b47b06aa23 Import anyhow::Result across the codebase
There is no need to fully qualify this type because it is a type
alias for std::Result. We can mix and match the two as we want.
4 years ago
Franck Royer 519d1a5701 Log rate and amounts for Alice when doing execution setup 4 years ago
Franck Royer b20c16df78 Improving logging on failure 4 years ago
Franck Royer 644f4c1732 Bubble up ws error to consumer
Note that because we are using `watch` channel, only a reference to the
channel value can be returned.
Hence, using custom Error that can be cloned to be able to
pass `Result` through the channel.
4 years ago
Franck Royer 92b3df4158 Introduce dynamic rates 4 years ago
bors[bot] 0359f8fbc0
Merge #216
216: To avoid CI failure wait for the balance instead of sleep r=da-kami a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
bors[bot] a6724f29af
Merge #214
214: Rename nectar to asb (automated swap backend) r=da-kami a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
Daniel Karzel bdb35c310d To avoid CI failure wait for the balance instead of sleep 4 years ago
bors[bot] 81228c9d5b
Merge #209
209: Upgrade to bdk 0.4 r=thomaseizinger a=thomaseizinger

Effectively, this also means:

- Upgrading to rust-bitcoin 0.26
- Upgrading to miniscript 5
- Upgrading monero to 0.10
- Upgrading curve25519-dalek to 3
- Upgrading bitcoin-harness to rust-bitcoin 0.26 (https://github.com/coblox/bitcoin-harness-rs/pull/21)
- Upgrade `ecdsa_fun` to latest version
- Replace `cross_curve_dleq` with `sigma_fun` (to avoid an upgrade dance on that library)

I refrained from specifying `rev`s in the Cargo.toml because we have a lock-file anyway. This should allow us to update those dependencies easier in the future by just running `cargo update -p <dependency>`.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
4 years ago
Daniel Karzel fe3d6f1fef Rename nectar to asb (automated swap backend) 4 years ago
Daniel Karzel 164de3c524 Properly calculate the confirmations for Bitcoin tx
Once the transaction was included into a block it has one confirmation - before inclusion it has zero.
current-block-height - transaction-block-height = zero; but that means one confirmation.
Hence, the confirmation calculation was adapted to: Current-block-height - (transaction-block-height - 1).
4 years ago
Thomas Eizinger 2d8ede80e1
Use released version of backoff 4 years ago
Thomas Eizinger cabf0efb8c
Only construct proof system once
The proof system is a static element and can be reused several times.
4 years ago
Thomas Eizinger 84bc2c82b7
Upgrade to bdk 4.0
To achieve this we also:

- upgrade rust-bitcoin to 0.26
- upgrade bitcoin-harness to latest version (which also depends bitcoin 0.26)
- upgrade to latest edcsa-fun
- replace cross_curve_dleq proof with sigma_fun (to avoid an upgrade dance over there)
4 years ago
bors[bot] b3f49cf83e
Merge #200
200: Wait for refund if insufficient Monero is locked up r=da-kami a=da-kami

In a scenario where Alice does not lock up sufficient funds Bob should properly transition to refunds. At the moment the CLI just panics. 
I noticed this when Alice accidentally had a different amount set than Bob. In the future this should not happen, because Alice provides the amount for Bob. However, in case Alice is malicious Bob should still transition correctly. 

Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
bors[bot] 8537b88a68
Merge #201
201: Fix ASB - Prevent the future from being stopped in production r=da-kami a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
bors[bot] 48635156ad
Merge #206
206: Remove misplaced wallet sync call r=rishflab a=rishflab

These bdk wallet sync calls must of gotten lost during a rebase. Removed the call in build TxLock and added one when nectar starts up

Co-authored-by: rishflab <rishflab@hotmail.com>
4 years ago
Daniel Karzel babd1d7b60
Wait for refund if insufficient Monero is locked up 4 years ago
Daniel Karzel ebb869e6f4 Distinguish transient and permanent Electrum errors 4 years ago
Daniel Karzel 9b93cabfdf Use context instead of map_error 4 years ago
rishflab fe362d765b Add sync wallet on nectar's startup 4 years ago
rishflab 00e2b257c4 Remove misplaced sync wallet call 4 years ago
Thomas Eizinger 8c83f7e2e1
Upgrade to testcontainers v0.12 4 years ago
Daniel Karzel 2ced9ddba4 Prevent future from being stopped by calling forget on handle 4 years ago
Daniel Karzel 77ec7e502f Distinguish permanent and transient error when watching tx 4 years ago
rishflab b66bb00c77 Remove stale code 4 years ago
rishflab bc1d2bda54 Test URL creation for default electrum HTTP API 4 years ago
rishflab d296c22ecf Log bitcoin deposit address on startup 4 years ago
rishflab 4768c79070 Derive bitcoin private key from seed 4 years ago
Thomas Eizinger a51194b9fa Instantiate electrum client with custom config with 2 retries
The default number of retries is 1. Unfortunately, the way this config
value is interpreted doesn't actually lead to a retry. We have to set
it to 2 to actually make it retry.

See https://github.com/bitcoindevkit/rust-electrum-client/issues/47.
4 years ago
rishflab 180e778df9 Allow blockchain calls to fail
Prior to this change, functions could not fail early on permanent errors eg. parsing a url. Merged error enums.
4 years ago
rishflab a0ef1f96ec Replace bitcoind wallet with bdk wallet
The bitcoind wallet required the user to run a bitcoind node. It was replaced with a bdk wallet which allows the user to connect to an electrum instance hosted remotely. An electrum and bitcoind testcontainer were created to the test the bdk wallet. The electrum container reads the blockdata from the bitcoind testcontainer through a shared volume. bitcoind-harness was removed as bitcoind initialisation code was moved into test_utils. The bdk wallet differs from the bitcoind wallet in that it needs to be manually synced with an electrum node. We synchronise the wallet once upon initialisation to prevent a potentially long running blocking task from interrupting protocol execution. The electrum HTTP API was used to get the latest block height and the transaction block height as this functionality was not present in the bdk wallet API or it required the bdk wallet to be re-synced to get an up to date value.
4 years ago
bors[bot] 4d8e801c1e
Merge #190
190: Do not pass Monero amount to the CLI r=D4nte a=D4nte

The CLI user only pass the Bitcoin amount they want to sell.

The CLI then do a quote request to nectar which provides the Monero amount the taker can get.

Co-authored-by: Franck Royer <franck@coblox.tech>
4 years ago
bors[bot] 5d215caf35
Merge #188
188: Tor cleanup r=da-kami a=da-kami

We never removed Tor install from CI. I don't think it should be necessary given that Tor was removed in code.

Co-authored-by: Daniel Karzel <daniel@comit.network>
4 years ago
Franck Royer f36f5acaf1
Remove unnecessary async block 4 years ago
Franck Royer 73c52ffb61
Remove unused structs 4 years ago
Franck Royer 2dbd43e2c0
Only pass btc amount to CLI
The CLI requests a quote to nectar to know how much xmr it can get.
Also align terminology with the sequence diagram.
4 years ago
Franck Royer 144da75270
Remove redundant data 4 years ago
Franck Royer fb3b2aa546
Remove unnecessary pub qualifier 4 years ago
Franck Royer b4e2e11dde
Remove unnecessary fields 4 years ago
Franck Royer d6effb7d39
Remove unnecessary pub qualifiers 4 years ago
Franck Royer b8a84aa34b
Avoid possible mix up between timelocks
Introduce new type to ensure no mix up happens when ordering the fields
in function calls.
4 years ago
Franck Royer 9a321a4f09
Move transaction types in dedicated modules
To allow the related timelock to be defined with the
transaction that uses it. This will allow the access to the
timelock's struct inner field with defining `From` impl.
4 years ago
Franck Royer 65e0e5b731
Use Remote handle to access ongoing swaps on Alice 4 years ago
bors[bot] a0753e24dc
Merge #189
189: The first message is more of a quote request than a swap request r=D4nte a=D4nte



Co-authored-by: Franck Royer <franck@coblox.tech>
4 years ago