531: Bump thiserror from 1.0.24 to 1.0.25 r=thomaseizinger a=dependabot[bot]
Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.24 to 1.0.25.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/dtolnay/thiserror/releases">thiserror's releases</a>.</em></p>
<blockquote>
<h2>1.0.25</h2>
<ul>
<li>Support <code>error(transparent)</code> on errors containing a non-<code>'static</code> inner error (<a href="https://github-redirect.dependabot.com/dtolnay/thiserror/issues/113">#113</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="19cb5cee4b"><code>19cb5ce</code></a> Release 1.0.25</li>
<li><a href="e49c10f2ba"><code>e49c10f</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/dtolnay/thiserror/issues/134">#134</a> from dtolnay/nonstatic</li>
<li><a href="1ed8751081"><code>1ed8751</code></a> Support non-static AsDynError lifetimes</li>
<li><a href="51a1ff6593"><code>51a1ff6</code></a> Add regression test for issue 113</li>
<li><a href="ee2a47d3af"><code>ee2a47d</code></a> Adjust macro hygiene test formatting</li>
<li><a href="c610d97267"><code>c610d97</code></a> Update ui test suite to nightly-2021-05-14</li>
<li><a href="c10adbc25e"><code>c10adbc</code></a> Ignore manual_map clippy lint</li>
<li>See full diff in <a href="https://github.com/dtolnay/thiserror/compare/1.0.24...1.0.25">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
</details>
535: Bitcoin network check when building PSBT r=da-kami a=da-kami
This ensures that funds are not sent to an address on the wrong network.
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Daniel Karzel <daniel@comit.network>
Forcing the user to create an implementation of `EstimateFeeRate`
every time they want to create a wallet for testing is tedious and
leads to duplicated code.
The implementation for tests is rarely dynamic and thus can be
simplified to static arguments.
This also allows us to provide convenience constructors to make tests
that don't care about fees less distracting by reducing the number of
constants that are floating around.
Adds the ping behaviour to both ASB and CLI behaviour that periodically pings a connected party to ensure that the underlying network connection is still alive.
This fixes problems with long-running connections that become dead without a connection closure being reported back to the swarm.
It appears to be more stable.
Encountered issues with the previous setup, `monero-wallet-rpc` logs:
```
2021-05-24 04:23:54.852 E !r. THROW EXCEPTION: tools::error::no_connection_to_daemon
2021-05-24 04:23:54.857 E Exception at while refreshing, what=no connection to daemon
```
525: Bitcoin transaction published state r=da-kami a=da-kami
This improves the error handling on the ASB.
Once the Bitcoin redeem transaction is seen in mempool, the state machine cannot transition to a cancel scenario anymore because at that point the CLI will have redeemed the Monero.
The additional state then waits for transaction finality and prevents re-publishing the transaction.
Co-authored-by: Daniel Karzel <daniel@comit.network>
This improves the error handling on the ASB.
Once the Bitcoin redeem transaction is seen in mempool, the state machine cannot transition to a cancel scenario anymore because at that point the CLI will have redeemed the Monero.
The additional state then waits for transaction finality.
Since we introduced our own parsing function for command line arguments, we have to make sure that clap's behaviour is handled correctly.
Clap's `get_matches_from_safe` returns an error of a certain kind, of which `ErrorKind::HelpDisplayed` and `ErrorKind::VersionDisplayed ` have to be handled to properly print the help/version and exit the program.
The clap error includes the message, so we print help/version in main now and ensure the program exits with `0` afterwards.
It is currently not expected that ASB and CLI are used for swaps > 10_000$ equivalent to XMR/BTC, thus the finality confirmations were reduced to an equivalent of 20 mins of work (2 blocks for Bitcoin, 10 for Monero).
Monero enforces 10 unlocking blocks until the balance is spendable, so the finality confirmations cannot be set lower than 10.
We subscribe to transactions upon broadcast, where we use output index `0` for the subscription.
In order to ensure that this subscription is guaranteed to be for the locking script (and not a change output) we now ensure that the locking script output is always at index `0` of the outputs of the transaction.
We chose this solution because otherwise we would have to add more information to broadcasting a transaction.
This solution is less intrusive, because the order of transaction outputs should not have any side effects and ensuring index `0` makes the whole behaviour more deterministic.
The Electrum block-header subscription did not provide us with block headers, because upon the connection being closed by a node the subscription would end.
Re-newing the the subscription upon re-connect is not easily achievable, that's why we opted for a polling mode for now, where we start a block header subscription on every update iteration, that is only used once (when the subscription is made).
There is no `--mainnet` flag.
Since we cannot just pass an empty string to `.arg()` we use the `.args()` method to pass nothing for mainnet and the respective flags for stagenet and testnet.
By default the finality confirmations of the network's `env::Config` will be applied and no finality confirmations will be persisted on disk in the config file.
It is however possible to set finality confirmations in the config file for bitcoin and monero for power users at their own risk.
If set the defaults will be overwritten with the parameter from the config file upon startup.
To run the ASB on testnet, one actively has to provide the `--testnet` flag.
Mainnet and testnet data and config are separated into sub-folders, i.e. `{data/config-dir}/asb/testnet` and `{data-dir}/asb/mainnet`.
The initial setup is also per network. If (default) config for the network cannot be found the initial setup is triggered.
Startup includes network check to ensure the bitcoin/monero network in config file is the same as the one in the `env::Config`.
Note: Wallet initialization is done with the network set in the `env::Config`, the network saved in the config file is just to indicate what network the config file is for.
This includes testing CLI commandline args
Clap's `default_value_with` actually did not work on `Subcommand`s because the parent's flags were not picked up.
This was fixed by changing parameters dependent on testnet/mainnet to options.
This problem should have been detected by tests, that's why the command line parameter tests were finally (re-)added.
Thanks to @rishflab for some pre-work for this.
In order to allow people to plug into public nodes / be more flexible with their own setup we now enforce specifying the monero daemon port to be used by the `monero-wallet-rpc`.
In the past we had problems with flags/parameter changes several times, where on instance was changed, buy another one was missed. This should mitigate this problem.
This patch introduces structs for all duplicated parameters and uses flatten to only have one point for changes.
Additionally removes all mentions of `alice` from the commands / variables. This code is on an application level and should not be concerned with swap protocol roles.
We need to check two things:
- balance to be higher than dust amount (546).
- balance to be higher than min-relay fee.
Additionally, the tx_builder might fail if not enough funds are in the wallet to pay for the overall transaction fees.
Introduces a minimum buy Bitcoin amount similar to the maximum amount already present.
For the CLI the minimum amount is enforced by waiting until at least the minimum is available as max-giveable amount.
Max-buy and spread is not something that one would configure on every run.
More convenient to keep this in the config.
The max-buy Bitcoin value was adapted to `0.02` which is more reasonable for mainnet.
Activated feature `serde-float` to serialize the spread (Decimal) as float instead of string.
```
...
[maker]
max_buy_btc = 0.02
ask_spread = 0.02
```
Weights fluctuate because of the length of the signatures. Valid ecdsa signatures can have 68, 69, 70, 71, or 72 bytes. Since most of our transactions have 2 signatures the weight can be up to 8 bytes less than the static weight (4 bytes per signature).
Adds `cancel`, `refund`, `punish`, `redeem` and `safely-abort` commands to the ASB that can be used to trigger the specific scenario for the swap by ID.
`asb --help` :
(...)
SUBCOMMANDS:
balance Prints the Bitcoin and Monero balance. Requires the monero-wallet-rpc to be running.
help Prints this message or the help of the given subcommand(s)
history Prints swap-id and the state of each swap ever made.
start Main command to run the ASB.
withdraw-btc Allows withdrawing BTC from the internal Bitcoin wallet.
In the production code it is a weird indirection that we load the state and then pass in the state and the database.
In the tests we have one additional load by doing it inside the command, but loading from the db is not expensive.
Each test spawns swarm for Alice and Bob that only contains the spot_price behaviours and uses a memory transport.
Tests cover happy path (i.e. expected price is returned) and error scenarios.
Implementation of `TestRate` on `LatestRate` allows testing rate fetch error and quote calculation error behaviour.
Thanks to @thomaseizinger for ramping up the test framework for comit-rs in the past!
Instead of handling all errors on the inside spot_price errors are bubbled up (as `SwapRequestDeclined`).
This allows us to test both Alice's and Bob's behaviour for all scenarios.
What goes over the wire should not be coupled to the errors being printed.
For the CLI and ASB we introduce a separate error enum that is used for logging.
When sending over the wire the errors are mapped to and from the `network::spot_price::Error`.
As part of Bob-specific spot_price code was moved from the network into bob.
Clearly separation of the network API from bob/alice.
Move Alice's spot price logic into a dedicated network behaviour that handles all the logic.
The new behaviour encapsulates the complete state necessary for spot price request decision making.
The network behaviour cannot handle asynchronous calls, thus the balance is managed inside the spot price and has to updated regularly from the outside to ensure the spot price balance check has up to date data.
At the moment the balance is updated upon an incoming quote requests.
Code that is relevant for both ASB and CLI remains in the `network::spot_price` module (e.g. `network::spot_price::Error`).
When a CLI requests a spot price have some errors that are expected, where we can provide a proper error message for the CLI:
- Balance of ASB too low
- Buy amount sent by CLI exceeds maximum buy amount accepted by ASB
- ASB is running in maintenance mode and does not accept incoming swap requests
All of these errors returns a proper error to the CLI and prints a warning in the ASB logs.
Any other unexpected error will result in closing the channel with the CLI and printing an error in the ASB logs.
Resume-only is a maintenance mode where no swaps are accepted but unfinished swaps are resumed.
This is achieve by ignoring incoming spot-price requests (that would lead to execution setup) in the event-loop.
Fees are hard to compute and it is too easy to get wrong and lose a lot of money. Hence, a hardcoded maximum of 100,000 satoshi for a single transaction is in place.
Electrum has an estimate-fee feature which takes as input the block you want a tx to be included.
The result is a recommendation of BTC/vbyte.
Using this recommendation and the knowledge about the size of our transactions we compute an appropriate fee.
The size of the transactions were taken from real transactions as published on bitcoin testnet.
Note: in reality these sizes might fluctuate a bit but not for much.
Alice chooses the fee for TxPunish because she is the one that cares.
Bob chooses the fee for TxRefund because he is the one that cares.
Note must be taken here because if the fee is too low (e.g. < min tx fee) then she might not be able to publish TxRedeem at all.
Alice chooses the fee for TxRedeem because she is the one that cares. Note must be taken here because if the fee is too low (e.g. < min tx fee) then she might not be able to publish TxRedeem at all.
460: Different default directories for CLI and ASB r=da-kami a=da-kami
Fixes#437
Using the same default directory as data-/config-dir has caused unwanted side effects when running both applications on the same machine.
Use these directory names:
- ASB: `xmr-btc-swap-asb`
- CLI: `xmr-btc-swap-cli`
Since the functionality is now application specific the respective functions were moved into the appropriate module of the application.
Co-authored-by: Daniel Karzel <daniel@comit.network>
Using the same default directory as data-/config-dir has caused unwanted side effects when running both applications on the same machine.
Use these directory names:
- ASB: xmr-btc-swap/asb
- CLI: xmr-btc-swap/cli
Since the functionality is now application specific the respective functions were moved into the appropriate module of the application.
Using the same default directory as data-/config-dir has caused unwanted side effects when running both applications on the same machine.
Use these directory names:
- ASB: xmr-btc-swap-asb
- CLI: xmr-btc-swap-cli
Since the functionality is now application specific the respective functions were moved into the appropriate module of the application.
Bob validates that incoming transfer proof messages are coming from the peer-id of Alice.
Currently Bob will ignore any transfer proof message that is not coming from the counterparty peer-id associated to the current swap in execution.
Once we add support for trying to save received transfer proofs for swaps that are currently not in execution we can also adapy allowing this for different counterparty peer-ids. This requires access to the database in Bob's event loop.
Alice validates that incoming encsig messages are coming from the peer-id that is associated with the swap.
Encsig message from a peer-id different to the one associated with the swap are ignored.
This PR does a few things.
* It adds a TorTransport which either dials through Tor's socks5 proxy or via clearnet.
* It enables ASB to register hidden services for each network it is listening on. We assume that we only care about different ports and re-use the same onion-address for all of them. The ASB requires to have access to Tor's control port.
* It adds support to dial through a local Tor socks5 proxy. We assume that Tor is always available on localhost. Swap cli only requires Tor to be running so that it can send messages via Tor's socks5 proxy.
* It adds a new e2e test which swaps through Tor. For this we assume that Tor is currently running on localhost. All other tests are running via clear net.
442: Minor cleanups towards implementing a Monero wallet for local signing r=thomaseizinger a=thomaseizinger
Extracted out of #434.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
A `RequestResponseCodec` for pull-based protocols where the response is encoded using JSON.
This was added to more properly express the behavior of the quote protocol, where the dialer
doesn't send any message and expects the listener to directly send the response.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
- Listen on both tcp and websockets as default
- Listening addresses in config as array
- Configure fallback transport using `or_transport` - if listening on a given address fails on WS, we fall back to TCP.
Instead of forwarding every error, we deliberately ignore certain
variants that are not worth being printed to the log. In particular,
this concerns "UnsupportedProtocols" and "ResponseOmission".
To make this less verbose we introduce a macro for mapping a
`RequestResponseEvent` to `{alice,bob}::OutEvent`. We use a macro
because those `OutEvent`s are different types and the only other
way of abstracting over them would be to introduce traits that we
implement on both of them.
To make the macro easier to use, we move all the `From` implementations
that convert between the protocol and the more high-level behaviour
into the actual protocol module.
405: Concurrent swaps with same peer r=da-kami a=da-kami
Fixes#367
- [x] Concurrent swaps with same peer
Not sure how much more time I should invest into this. We could just merge the current state and then do improvements on top...?
Improvements:
- [x] Think `// TODO: Remove unnecessary swap-id check` through and remove it
- [x] Add concurrent swap test, multiple swaps with same Bob
- [ ] Save swap messages without matching swap in execution in the database
- [ ] Assert the balances in the new concurrent swap tests
- [ ] ~~Add concurrent swap test, multiple swaps with different Bobs~~
- [ ] ~~Send swap-id in separate message, not on top of `Message0`~~
Co-authored-by: Daniel Karzel <daniel@comit.network>
- Swap-id is exchanged during execution setup. CLI (Bob) sends the swap-id to be used in his first message.
- Transfer poof and encryption signature messages include the swap-id so it can be properly associated with the correct swap.
- ASB: Encryption signatures are associated with swaps by swap-id, not peer-id.
- ASB: Transfer proofs are still associated to peer-ids (because they have to be sent to the respective peer), but the ASB can buffer multiple
- CLI: Incoming transfer proofs are checked for matching swap-id. If a transfer proof with a different swap-id than the current executing swap is received it will be ignored. We can change this to saving into the database.
Includes concurrent swap tests with the same Bob.
- One test that pauses and starts an additional swap after the transfer proof was received. Results in both swaps being redeemed after resuming the first swap.
- One test that pauses and starts an additional swap before the transfer proof is sent (just after BTC locked). Results in the second swap redeeming and the first swap being refunded (because the transfer proof on Bob's side is lost). Once we store transfer proofs that we receive during executing a different swap into the database both swaps should redeem.
Note that the monero harness was adapted to allow creating wallets with multiple outputs, which is needed for Alice.
397: Always log at debug level to file r=rishflab a=rishflab
WILL SQUASH DOWN TO 3 COMMITS WHEN APPROVED!
Log at debug level to file
EnvFilter is applied globally. This means you cannot log at INFO level
to the terminal and at DEBUG level to log files. To get a around this
limitation I had to implement the layer trait on a new type and filter
in the on_event() trait method. Each swap has its own log file denoted
by its swap_id. The logger appends to the existing file when resuming a
swap.
Closes#278
I think the `DebugTerminalPritner` and `InfoTerminalPrinter` could be consolidated with some effort with some generics wizardry. It works for now and I think it can be done later. I wish in general there was a cleaner way to do this.
Co-authored-by: rishflab <rishflab@hotmail.com>
EnvFilter is applied globally. This means you cannot log at INFO level
to the terminal and at DEBUG level to log files. To get a around this
limitation I had to implement the layer trait on a new type and filter
in the on_event() trait method. Each swap has its own log file denoted
by its swap_id. The logger appends to the existing file when resuming a
swap.
Closes#278
396: Remove default connection details from CLI r=thomaseizinger a=rishflab
Connecting buyers to us by default is not consistent with our vision of
a decentralised network of sellers.
Closes#395
Co-authored-by: rishflab <rishflab@hotmail.com>
387: Improve the resilience of the network layer r=thomaseizinger a=thomaseizinger
We improve the resilience in two ways:
1. Use a timeout on Bob's side for the execution-setup.
2. Use the `bmrng` library to model the communication between Alice and Bob.
See commit messages for details.
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Edge cases of UTXOs where value < fee cause the BDK's `coin_select` calculation to panic.
This issue was fixed upstream thus we point the BDK dependency against the commit of the merged fix.
It might very well be that the cancel transaction is already published.
If that is the case, there is no point in failing the command. We simply
transition to cancel and exit normally.
The reason this comes up now is because Alice now properly waits for
the cancel timelock as well and publishes the cancel transaction first.
Ultimately, she should not do that because there is no benefit to her
unless she can also publish the punish transaction.
Sending the transfer proof might never resolve because Bob doesn't
come back online. In that case, we need to make sure we bail out
as soon as the timelock expires.
We use the "precondition" feature of the `tokio::select!` macro to
avoid polling certain futures. In particular, we skip polling all
futures that - when resolved - require us to send a message to Alice.
This allows us to delay the ACKing of the encrypted signature up until
the swap has actually requested it.
Similarly, it allows us to wait for the ACK of the transfer proof within
the swap before continuing.
bmrng is a library providing a request-response channel that allows
the receiving end of the channel to send a response back to the sender.
This allows us to more accurately implement the functions on the
`EventLoopHandle`. In particular, we now _wait_ for the ACK of specific
messages from the other party before resolving the future.
For example, when sending the encrypted signature, the async function
on the `EventLoopHandle` does not resolve until we received the ACK
from the other party.
We also delete the `Channels` abstraction in favor of directly creating
bmrng channels. This allows us to directly control the channel buffer
which we set to 1 because we don't need more than that on Bob's side.
There is no point in first checking for the expired timelocks and
then constructing a `select!` that also watches for the timelock to
expiry.
We can simply only have the select! invocation to achieve the same
effect. In case the timelock is already expired, this future will
resolve immediately.
Normally, the polling order of `select!` is pseudo-random. We
configure it to be _biased_ here to make sure the futures are polled
in order.
The execution setup is our only libp2p protocol that doesn't have
a timeout built-in. Hence, if anything fails on Alice's side, we
would wait here forever.
Wrapping the future in a timeout ensures that we fail eventually
if this protocol doesn't succeed.
