321: Properly handle concurrent messages to and from peers r=thomaseizinger a=thomaseizinger
Previously, we were forwarding incoming messages from peers to all
swaps that were currently running. That is obviously wrong. The new
design scopes an `EventLoopHandle` to a specific PeerId to avoid
this problem.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
322: Refactor `ExecutionParams` and harmonize sync intervals of wallets r=thomaseizinger a=thomaseizinger
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Previously, we were forwarding incoming messages from peers to all
swaps that were currently running. That is obviously wrong. The new
design scopes an `EventLoopHandle` to a specific PeerId to avoid
this problem.
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.
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.
320: Fix env filter for asb r=thomaseizinger a=thomaseizinger
1. The asb didn't log any if the statements within main.rs
2. We were initializing unnecessary filters that don't make any sense
for the asb. warp and http are not used and the harness-es are for
test only.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
317: Fix monero refresh interval r=thomaseizinger a=thomaseizinger
The comparison should be the MAXIMUM of the two values, not the
minimum, otherwise we always refresh at an interval of 1 second.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
A non-interactive terminal is likely something along the lines of
journalctl which captures a timestamp by itself. In theory, it could
also be just a logfile but we rather accept this limitation and keep
the configuration surface simple rather than exposing another config
switch.
1. The asb didn't log any if the statements within main.rs
2. We were initializing unnecessary filters that don't make any sense
for the asb. warp and http are not used and the harness-es are for
test only.
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.
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.
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>
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.
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.
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.
The CLI has sensible default values for all parameters,
thus a config file is not really an advantage but just
keeps getting in our way, so re remove it.
306: Fix logging and retrying of Monero transaction watching r=thomaseizinger a=thomaseizinger
Hopefully, this should also reduce the load because I am not asking the node every second.
Related: https://github.com/comit-network/xmr-btc-swap/issues/202
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Daniel Karzel <daniel@comit.network>
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.
This config setting makes backoff stop retrying if we didn't get an
error within this timeframe.
For us, this results in backoff not actually doing anything.
The connection to kraken is very long-running. It might be active
for hours without failing. However, the default value for
`max_elapsed_time` is set to 15 minutes. As such, once the connection
fails any time after that, backoff doesn't actually retry the operation
but just gives up.
Fixes#303.
In order to be able to re-connect on certain errors, we model
connection errors separately from parsing errors. We also change
the API of the whole module to no longer forward all errors to
the subscribers but instead, only update the subscribers with
either a latest rate or a permanent failure in case we exhausted
all our options to re-connect the websocket.
To model all of this properly, we introduce to sub-modules so that
each submodule can have their own `Error` type.
Resolves#297.
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.
The type hints are generated from the field names. This has the
unfortunate consequence of the config field becoming file_path which
does not really make sense people working on the codebase.
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.
288: Switch to public stagenet node that works r=rishflab a=rishflab
The xmr.to node has been unreliable lately. The exan.tech node seems to
working.
@da-kami is following up with making this configurable. Lets get this in so we can get a release on Friday.
Co-authored-by: rishflab <rishflab@hotmail.com>
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.
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.
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.
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.
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>
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>
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.
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.
