Having `spot_price` and `execution_setup` as separate protocols did not bring any advantages, but was problematic because we had to ensure that `execution_setup` would be triggered after `spot_price`. Because of this dependency it is better to combine the protocols into one.
Combining the protocols also allows a refactoring to get rid of the `libp2p-async-await` dependency.
Alice always listens for the `swap_setup` protocol. When Bob opens a substream on that protocol the spot price is communicated, and then all execution setup messages (swap-id and signature exchange).
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.
To achieve this, we decompose `watch_for_locked_xmr` into two parts:
1. A non-self-consuming function to construct a `WatchRequest`
2. A state transition that can now consume `self` again because
it is only called once within the whole select! expression.
Ideally, we would move more logic onto this state transition (like
comparing the actual amounts and fail the transition if it is not
valid). Doing so would have an unfortunate side-effect: We would
always wait for the full confirmations before checking whether or
not we actually receive enough XMR.
This allows us to have state transitions that consume self.
This allows us to move critical crypto logic onto `State3` which
holds all the necessary data which consequently allows us to get
rid of `lock_xmr` altogether by inlining it into the swap function.
The reduced indirection improves readability.
We define the sync interval as 1/10th of the blocktime. For the
special case of our tests, we however check at max once per second.
The tests have a super fast blocktime. As such we shouldn't hammer
the nodes with a request every 100ms.
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.
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.
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>
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.
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.
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.
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`.
Upgrade bitcoin harness dependency to latest commit
Upgrade backoff to fix failing tests. The previous version of backoff had a broken version of the retry function. Upgraded to a newer comit which fixes this problem.
Upgrade hyper to 0.14 as the 0.13 was bringing in tokio 0.2.24
Upgraded bitcoin harness to version that uses tokio 1.0 and reqwest 0.11
Upgrade reqwest to 0.11. Reqwest 0.11 uses tokio 1.0
Upgrade libp2p to 0.34 in preparation for tokio 1.0 upgrade
Rust fmt automatically groups the imports (from top to bottom) as `pub use` `use crate` and `use`.
There is no need to introduce sections which cause annoyance when auto importing using the IDE.
Daniel Karzel
Philipp Hoenisch
Thomas Eizinger
bors[bot]
rishflab