Commit Graph

216 Commits

Author SHA1 Message Date
bors[bot]
113f2fa385
Merge #322
322: Refactor `ExecutionParams` and harmonize sync intervals of wallets r=thomaseizinger a=thomaseizinger



Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
2021-03-17 06:25:59 +00:00
bors[bot]
fc28609a96
Merge #323 #324
323: Minor fixes r=thomaseizinger a=rishflab



324: Enable dependabot r=thomaseizinger a=thomaseizinger



Co-authored-by: rishflab <rishflab@hotmail.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
2021-03-17 05:59:01 +00: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
rishflab
8675d88727 Don't wait for tx lock confirmed after broadcast
Bob does not care whether tx lock is confirmed. That is alice's problem.
This wait was introduced to remedy a bug in status_of_script() which was
 failing when called on a transaction with no confirmations.
2021-03-17 15:34:29 +11:00
rishflab
f5e6ba18e0 Use different address for redeem and punish
Having the same address could potentially cause issues when subscribing
to transactions by script
2021-03-17 15:26:44 +11:00
Thomas Eizinger
273cf15631
Introduce Watchable abstraction for Bitcoin wallet
We have a repeated pattern where we construct one of our
Tx{Cancel,Redeem,Punish,Refund,Lock} transactions and wait until
the status of this transaction changes. We can make this more
ergonomic by creating and implementing a `Watchable` trait that
gives access to the TxId and relevant script for this transaction.
This allows us to remove a parameter from the `watch_until_status`
function.

Additionally, there is a 2nd pattern: "Completing" one of these
transaction and waiting until they are confirmed with the configured
number of blocks for finality. We can make this more ergonomic by
returning a future from `broadcast` that callers can await in case
they want to wait for the broadcasted transaction to reach finality.
2021-03-16 19:24:32 +11: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
Thomas Eizinger
84ea092a1b
Remove unnecessary state variables by constructing TXs on demand 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
Thomas Eizinger
e17cbadccb
Don't add signatures to transaction unless necessary
In order to compute the cancel TxID, we don't need to add the
signatures.
2021-03-16 19:22:47 +11:00
Thomas Eizinger
6beb732e35
Eliminate build_bitcoin_punish_transaction
We reduce indirection by constructing TxPunish directly based off
`State3` and make the type itself more powerful by moving the logic
of completing it with a signature onto it.
2021-03-16 19:21:15 +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
Thomas Eizinger
dd6c66a594
Move completing of Bitcoin redeem tx onto RedeemTx
This allows us to have access to RedeemTx from within the scope
of the state transition which we are going to need for more
efficient watching of what happens to this TX on the blockchain.
2021-03-16 17:11:31 +11:00
rishflab
21429f24b2
Inline wait_for_locked_bitcoin() that is only called once
Reduce indirection.
2021-03-16 17:11:31 +11:00
rishflab
6a3e4802f1
Remove redundant reference 2021-03-16 17:11:30 +11:00
Thomas Eizinger
c2329b19a2
Tell the user more about the monero lock transaction
First, we tell the user that we are now waiting for Alice to lock
the monero. Additionally, we tell them once we received the
transfer proof which will lead directly into the
"waiting for confirmations" function.
2021-03-05 16:56:47 +11:00
Thomas Eizinger
4138039ea0
Make sure all error messages start with an uppercase letter
These might potentially be shown to a user, let's make them all
consistent.
2021-03-05 15:49:15 +11:00
Thomas Eizinger
5953037b81
Don't repeat the module name within the type 2021-03-05 15:49:13 +11:00
Thomas Eizinger
b178e95f95
Redo layout of eventloop module
1. Move internal types to the bottom and make them private
2. Sort public types by their importance
2021-03-05 14:02:24 +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
bors[bot]
20f2e56e2d
Merge #271
271: Bob can verify that the XMR lock tx was published r=da-kami a=da-kami

The Monero `txhash` log was removed. I feel the user should have the possibility to verify that the transaction was actually published so I added the tx-hash to the confirmation output. 

We could potentially print the tx-hash when receiving the transfer proof already, but that might not add much value compared to printing it with the confirmations. 

Additionally we should allow the user to at least know when the XMR can be expected in the user's wallet, otherwise the swap ends like this:

```
2021-03-04 13:49:19   INFO Monero lock tx received 5 out of 5 confirmations
```

This is just not very informative - yes, the final transaction is an implementation detail, but I don't think we should hide the transactions from the user. By printing the tx-hash for spending from the lock-tx into the user wallet we ensure the user knows that the XMR can now be expected in the user wallet. 

--- 

To add context, here the complete log (with debug enabled) **before** this change: 

```
2021-03-04 13:30:46  DEBUG Database and seed will be stored in /Users/dakami/Library/Application Support/xmr-btc-swap
2021-03-04 13:30:46  DEBUG Starting monero-wallet-rpc on port 56145
2021-03-04 13:30:51  DEBUG Requesting quote
2021-03-04 13:30:51   INFO Received quote: 1 XMR = 0.00433500 BTC
2021-03-04 13:30:51   INFO Still got 0.01018746 BTC left in wallet, swapping ...
2021-03-04 13:30:51   INFO Spot price for 0.00500000 BTC is 1.153402537485 XMR
2021-03-04 13:30:52  DEBUG Starting execution setup with 12D3KooWCdMKjesXMJz1SiZ7HgotrxuqhQJbP5sgBm2BwP1cqThi
2021-03-04 13:30:55   INFO Published Bitcoin 3a6690a962191529892318819fb20e7f1ac4625400e64ee734056a9b2a17ad8f transaction as lock
2021-03-04 13:41:13  DEBUG Received Transfer Proof from 12D3KooWCdMKjesXMJz1SiZ7HgotrxuqhQJbP5sgBm2BwP1cqThi
2021-03-04 13:42:11   INFO Monero lock tx received 1 out of 5 confirmations
2021-03-04 13:45:33   INFO Monero lock tx received 2 out of 5 confirmations
2021-03-04 13:47:49   INFO Monero lock tx received 3 out of 5 confirmations
2021-03-04 13:48:56   INFO Monero lock tx received 4 out of 5 confirmations
2021-03-04 13:49:19   INFO Monero lock tx received 5 out of 5 confirmations
2021-03-04 13:49:19  DEBUG Encrypted signature sent
2021-03-04 13:49:19  DEBUG Alice acknowledged encrypted signature
2021-03-04 13:49:19  DEBUG watching for tx: e5569d3f0bcccac95252dffaebe74ead0360c09b76bc762de890aaa0e51afbcf
2021-03-04 13:49:20  DEBUG Received protocol error "missing transaction" from Electrum, retrying...
2021-03-04 13:49:22  DEBUG Received protocol error "missing transaction" from Electrum, retrying...
```



Co-authored-by: Daniel Karzel <daniel@comit.network>
2021-03-04 06:24:59 +00:00
Daniel Karzel
47a31760c0 Bob can verify the Monero txs by tx-hash
Print tx-hashes for monero transactions to allow Bob to look the transaction up in block explorer.

The story of Bab:
Our famous actor Bob has a brother named Bab.
In school they were often mixed up, because their names were so similar.
Eventually Bab renamed himself into Barbara, but that was even more confusing for now he
carried a female name even though he was not female. Bob wanted to help his brother and told him he
could just go for Bub. But that did not solve anything. Fun fact: Bub is actually married to Alice.
2021-03-04 16:51:55 +11:00
Thomas Eizinger
601bf07255
Introduce quote protocol and display it to the user before they fund
Previously, the user neither knew the price nor the maximum quantity
they could trade. We now request a quote from the user and display
it to them.

Fixes #255.
2021-03-04 16:26:27 +11:00
Thomas Eizinger
3da01ea44a
Rename local variables according to new protocol name 2021-03-04 16:20:54 +11:00
Thomas Eizinger
ead5db3f45
Re-order fields to group them semantically 2021-03-04 16:20:51 +11:00
Thomas Eizinger
bc176bc4fb
Minor import optimizations 2021-03-04 14:48:28 +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
Thomas Eizinger
2c8200621d
Move pub use into pub use block 2021-03-04 14:47:47 +11:00
bors[bot]
d1363d130c
Merge #265
265: Replace quote with spot-price protocol r=thomaseizinger a=thomaseizinger

This is essentially functionally equivalent but includes some
cleanups by removing a layer of abstraction: `spot_price::Behaviour`
is now just a type-alias for a request-response behaviour.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
2021-03-04 02:52:06 +00: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
2e3c2d8edf Remove Arcs in favour of references 2021-03-03 17:23:07 +11:00
Daniel Karzel
d63790c2a6 Remove unnecessary monero wallet trait abstractions 2021-03-03 17:15:37 +11:00
Daniel Karzel
1041212a60 Work in review comments 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
Thomas Eizinger
7042ed9441
Replace quote with spot-price protocol
This is essentially functionally equivalent but includes some
cleanups by removing a layer of abstraction: `spot_price::Behaviour`
is now just a type-alias for a request-response behaviour.
2021-03-03 17:09:38 +11:00
Daniel Karzel
9f53dab3c6 Harmonize names to make more sense
The wallet is an instance of a wallet that has a name.
When we use `CreateWalletForOutputThenReloadWallet` we actually unload the wallet.
It would be cleaner to create a new instance that does that swap, but I did not go that far.
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
Thomas Eizinger
089ac0806e
Simplify constructor of Bob's EventLoop
We never customize the behaviour or transport. Might as well hide
those details in the implementation.
2021-03-03 14:08:56 +11:00
Daniel Karzel
1b167f3eb6 Cleanup swap initialization for Alice and Bob 2021-03-03 14:07:00 +11:00
Thomas Eizinger
3ad9516188
Reduce logging when signing transactions
1. We can generalize the signing interface by passing a PSBT in
instead of the `TxLock` transaction.
2. Knowing the transaction ID of a transaction that we are about
to sign is not very useful. Instead, it is much more useful to know
what failed. Hence we add a `.context` to the call of `sign_and_finalize`.
3. In case the signing succeeds, we will immediately broadcast it
afterwards. The new broadcasting interface will tell us that we broadcasted
the "lock" transaction.
2021-03-02 12:53:40 +11:00
Thomas Eizinger
8c9b087e39
Unify logging of broadcasted transactions
We eliminate unnecessary layers of indirection for broadcasting logic
and force our callers to provide us with the `kind` of transaction
that we are publishing.

Eventually, we can replace this string with some type-system magic
we can derive the name from the actual transaction. For now, we just
require the caller to duplicate this information because it is faster
and good enough TM.
2021-03-02 12:51:22 +11:00
Thomas Eizinger
3a503bf95f
Shorten function name
This struct is a wallet. The only thing it can meaningfully broadcast
are transactions. The fact that they have to be signed for that is
implied. You cannot broadcast unsigned transactions.
2021-03-02 12:25:47 +11:00
Thomas Eizinger
45cff81ea5
Remove traits in favor of using the wallet struct directly
Abstracting over the individual bits of functionality of the wallet
does have its place, especially if one wants to keep a separation
of an abstract protocol library that other people can use with their
own wallets.

However, at the moment, the traits only cause unnecessary friction.
We can always add such abstraction layers again once we need them.
2021-03-02 12:22:23 +11:00
Thomas Eizinger
06e3bccaa6
Don't print PeerId when requesting quote
Bob always just talks to one party, the PeerId is just noise.
2021-03-02 09:50:16 +11:00
Thomas Eizinger
b7c3524b4f
Abort the eventloop if the dialling fails 2021-03-02 09:50:14 +11:00