Commit Graph

107 Commits

Author SHA1 Message Date
Thomas Eizinger
73f30320a6
Seed should neither be Clone nor Copy
It is better to not copy around secret data within our process to
make heartbleed-like attacks harder.
2021-03-24 11:39:39 +11:00
Daniel Karzel
396c4177a6 Alice sweeps refunded funds into default wallet
Since Alice's refund scenario starts with generating the temporary wallet
from keys to claim the XMR which results in Alice' unloading the wallet.
Alice then loads her original wallet to be able to handle more swaps.
Since Alice is in the role of the long running daemon handling concurrent
swaps, the operation to close, claim and re-open her default wallet must
be atomic.
This PR adds an additional step, that sweeps all the refunded XMR back into
the default wallet. In order to ensure that this is possible, Alice has to
ensure that the locked XMR got enough confirmations.
These changes allow us to assert Alice's balance after refunding.
2021-03-18 17:59:48 +11:00
Thomas Eizinger
09c41f89c4
Rename ExecutionParams to EnvironmentConfig 2021-03-17 16:31:16 +11:00
Thomas Eizinger
bc43ed6ebd
Pass execution params directly into wallet for initialization
This reduces the amount of parameters that we need to pass in.
2021-03-17 16:30:58 +11:00
bors[bot]
95acbc6277
Merge #307
307: Reduce load on electrum r=thomaseizinger a=rishflab

.

Co-authored-by: rishflab <rishflab@hotmail.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
2021-03-17 05:10:50 +00:00
Thomas Eizinger
a0830f099f
Pass relevant execution params into wallet instead of via functions
The execution params don't change throughout the lifetime of the
program. They can be set in the wallet at the very beginning.
This simplifies the interface of the wallet functions.
2021-03-16 19:24:31 +11:00
rishflab
e5c0158597
Greatly reduce load onto the Electrum backend
We achieve our optimizations in three ways:

1. Batching calls instead of making them individually.

To get access to the batch calls, we replace all our
calls to the HTTP interface with RPC calls.

2. Never directly make network calls based on function
calls on the wallet.

Instead, inquiring about the status of a script always
just returns information based on local data. With every
call, we check when we last refreshed the local data and
do so if the data is considered to be too old. This
interval is configurable.

3. Use electrum's notification feature to get updated
with the latest blockheight.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Rishab Sharma <rishflab@hotmail.com>
2021-03-16 19:24:31 +11:00
Daniel Karzel
ea05c306e0 Alice spawns swaps outside the event loop
Instead of spawning the swap inside the event loop we send the swap back
to the caller to be spawned. This means we no longer need the remote handle
that was only used in the tests.
This now properly logs the swap results in production.
It also gives us more control over Alice's swap in the tests.
2021-03-16 18:31:46 +11:00
rishflab
752e5be8f3
Cleanup test logging 2021-03-16 17:11:29 +11:00
rishflab
9f534996ee Remove unused capability to configure bitcoind docker version tag
We only use one version of this container
2021-03-12 12:50:42 +11:00
Daniel Karzel
be52892e65
Monero wallet should not know about all execution params
Instead of passing all execution params in we only make the monero_avg_block_time known to the monero wallet.
2021-03-11 17:43:01 +11:00
Thomas Eizinger
82738b111e
Refactor monero::Wallet::watch_for_transfer to not use backoff
Instead, we use a regular loop and extract everything into a function
that can be independently tested.
`backoff` would be useful to retry the actual call to the node.
2021-03-11 17:42:54 +11:00
Thomas Eizinger
37f97ac471
Shorten function name
The variable will always be at least called `wallet`, hence we can
omit the `_wallet` postfix from the function name.
2021-03-05 15:49:14 +11:00
Thomas Eizinger
4f66269887
Move error message on sync _into_ the function
The bitcoin::Wallet::sync_wallet function doesn't do anything else
other than delegating. As such, we have just as much information
about what went wrong inside this function as we have outside.

By moving the .context call into the function, we can avoid repeating
us on every call-site.
2021-03-05 15:49:14 +11:00
Thomas Eizinger
1822886cd0
Provide stronger isolation of kraken module
Instead of leaking the tokio::sync::Receiver type in our
return value, we create a newtype that implements the desired
interface. This allows us to get rid of the `RateService` structs
and instead implement `LatestRate` directly on top of this struct.

Given that `LatestRate` is only used within the event_loop module,
we move the definition of this type into there.
2021-03-05 13:56:25 +11:00
Thomas Eizinger
6d9b21cb47
Change imports_granularity to module
This reduces the overall amount of LoC that imports take up in our
codebase by almost 100.
It also makes merge-conflicts less likely because there is less
grouping together of imports that may lead to layout changes which
in turn can cause merge conflicts.
2021-03-04 14:48:13 +11:00
bors[bot]
cba9f119b6
Merge #261
261: Sweep xmr funds from generated temp wallet r=da-kami a=da-kami

Fixes #252

Please review by commit :)
Did a few cleanups before actually doing the feature.

Please note the comment that influenced this solution: https://github.com/comit-network/xmr-btc-swap/issues/252#issuecomment-789387074



Co-authored-by: Daniel Karzel <daniel@comit.network>
2021-03-03 06:29:37 +00:00
Daniel Karzel
d63790c2a6 Remove unnecessary monero wallet trait abstractions 2021-03-03 17:15:37 +11:00
Daniel Karzel
66c8401c95 Sweep all from generated wallet to user wallet
The default implementation for the command was removed because it does not
add additional value if we have a mandatory parameter anyway.
2021-03-03 17:15:37 +11:00
Daniel Karzel
5111a12706 Wallet name constants for the e2e test setup
Container initialization and wallet initialization have to ensure to use the same wallet name.
In order to avoid problems constants are introduced to ensure we use the same wallet name.
2021-03-03 17:03:34 +11:00
Daniel Karzel
2bb1c1e177 No prefix for wallets in monero harness
Prefixing docker-containers and -networks is a necessity to be able to spin up multiple containers and networks.
However, there is no reason to prefix the wallet names that live inside a container. One cannot add a wallet with
the same name twice, so the prefixing of wallets does not bring any advantage. When re-opening a wallet by name
the wallet name prefix is cumbersome and was thus removed.
2021-03-03 17:03:34 +11:00
Thomas Eizinger
2440964385
Allow ASB to be configured with max BTC buy amount
This will make it easier to also configure the CLI to display an appropriate max amount the user has to deal with.
2021-03-03 16:56:34 +11:00
Thomas Eizinger
ce077a3ff5
Decouple Bob's EventLoop from the builder
Instead of instantiating the `EventLoop` within the builder, we only
pass in the necessary arguments (which is the `EventLoopHandle`) to
the Builder upon `new`.

This is work towards #255 which will require us to perform network
communication (which implies having the `EventLoop`) before starting
a swap.
2021-03-03 14:53:05 +11:00
Thomas Eizinger
54bc91581f
Don't unnecessarily create async blocks
If our expression directly evaluates to a future, we don't need to
create an async block.

This requires us to have `EventLoopRun::run` consume the instance
instead of just taking a mutable reference (otherwise we run into
lifetime issues). However, that is better anyway because `run` is
an endless loop so you never get to use the handle afterwards
anyway.
2021-03-03 14:53:05 +11:00
Thomas Eizinger
a4c25080b6
Merge network::Seed into crate::Seed
This allows us to unify the way we derive new secret key material
and simplify the usage of seed by only having a single one.
2021-03-03 14:53:01 +11:00
rishflab
a41b255dab Upgrade bitcoin wallet to use BIP84 derivation scheme
Explicitly specify the change descriptor because the behaviour when it
is not specified is unclear.
2021-03-03 12:12:10 +11:00
bors[bot]
7251588e79
Merge #233
233: ASB max sell amount r=thomaseizinger a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>
2021-03-01 01:47:34 +00:00
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.
2021-02-26 17:18:12 +11:00
Daniel Karzel
019d6c725a Maximum sell amount for ASB that defaults to 0.5 XMR 2021-02-26 16:48:27 +11:00
Daniel Karzel
0945cee459 Remove traits in favour of public functions 2021-02-25 10:34:22 +11:00
Daniel Karzel
578d23d7fc Proper encapsulation of wallet boundaries through private fields 2021-02-25 10:30:24 +11:00
Daniel Karzel
947bcb6192 ASB reloads the default wallet after generate_from_keys atomically 2021-02-25 00:34:05 +11:00
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`.
2021-02-25 00:33:58 +11:00
Thomas Eizinger
729f4f09a8
Remove unnecessary tracing_core dependency 2021-02-23 14:30:19 +11:00
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.
2021-02-22 13:26:56 +11:00
Franck Royer
b20c16df78 Improving logging on failure 2021-02-22 13:26:27 +11:00
Franck Royer
92b3df4158 Introduce dynamic rates 2021-02-22 13:24:59 +11:00
Daniel Karzel
bdb35c310d To avoid CI failure wait for the balance instead of sleep 2021-02-22 11:36:14 +11:00
rishflab
4768c79070 Derive bitcoin private key from seed 2021-02-16 14:57:12 +11:00
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.
2021-02-16 14:57:10 +11:00
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.
2021-02-16 14:04:52 +11:00
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.
2021-02-15 16:20:34 +11:00
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.
2021-02-15 16:20:31 +11:00
Franck Royer
65e0e5b731
Use Remote handle to access ongoing swaps on Alice 2021-02-15 15:00:47 +11:00
Franck Royer
8fada42074
Make config argument global
The `config` argument apply to all commands. It is now optional and
needs to be passed before a command.
E.g. `cli --config ./config.toml history`
2021-02-15 11:01:50 +11:00
Franck Royer
6e6dc320b4
Alice event loop now handles the creation of new swaps 2021-02-10 16:42:55 +11:00
Daniel Karzel
c9adbde5d5 Add test for Bob's manual cancel and refund 2021-02-08 21:17:56 +11:00
Franck Royer
c316ea5244
Upgrade toolchain
Needed to use libp2p-async-await
2021-02-04 15:18:33 +11:00
Daniel Karzel
89b3775e05 Rename config to execution_params 2021-01-29 17:27:50 +11:00
Daniel Karzel
802dc61e7e Configuration for RPC urls and Bitcoin wallet name 2021-01-29 17:21:19 +11:00