542: More resilient `MoneroWalletRpc` startup in the harness r=da-kami a=da-kami
Recently we se this problem on CI quite often:
```
May 27 01:26:55.898 INFO testcontainers::core::wait_for_message: Found message after comparing 80 lines
May 27 01:27:00.858 INFO testcontainers::core::wait_for_message: Found message after comparing 2 lines
May 27 01:27:00.859 INFO monero_harness: Starting monerod: DQma_monerod
May 27 01:27:08.143 INFO testcontainers::core::wait_for_message: Found message after comparing 183 lines
May 27 01:27:08.204 INFO monero_harness: Starting miner wallet: miner
May 27 01:27:09.832 INFO testcontainers::core::wait_for_message: Found message after comparing 16 lines
May 27 01:27:12.261 INFO monero_harness: Starting wallet: alice
May 27 01:27:14.482 INFO testcontainers::core::wait_for_message: Found message after comparing 16 lines
thread 'alice_punishes_after_restart_if_bob_dead' panicked at 'called `Result::unwrap()` on an `Err` value: error sending request for url (http://127.0.0.1:49177/json_rpc): operation was canceled: connection closed before message completed
```
Given the message `connection closed before message completed` it is likely that the `monero-wallet-rpc` is not fully started yet.
Unfortunately we cannot wait to see a different message in the logs upon container startup, because there are just no further deterministic messages after the one we are currently listening on.
To overcome this problem without extending testcontainers we introduce a retry mechanism when creating the wallet.
Co-authored-by: Daniel Karzel <daniel@comit.network>
538: Add ping protocol to ensure connection is alive r=da-kami a=da-kami
Fixes#532
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.
Co-authored-by: Daniel Karzel <daniel@comit.network>
Recently we se this problem on CI quite often:
```
May 27 01:26:55.898 INFO testcontainers::core::wait_for_message: Found message after comparing 80 lines
May 27 01:27:00.858 INFO testcontainers::core::wait_for_message: Found message after comparing 2 lines
May 27 01:27:00.859 INFO monero_harness: Starting monerod: DQma_monerod
May 27 01:27:08.143 INFO testcontainers::core::wait_for_message: Found message after comparing 183 lines
May 27 01:27:08.204 INFO monero_harness: Starting miner wallet: miner
May 27 01:27:09.832 INFO testcontainers::core::wait_for_message: Found message after comparing 16 lines
May 27 01:27:12.261 INFO monero_harness: Starting wallet: alice
May 27 01:27:14.482 INFO testcontainers::core::wait_for_message: Found message after comparing 16 lines
thread 'alice_punishes_after_restart_if_bob_dead' panicked at 'called `Result::unwrap()` on an `Err` value: error sending request for url (http://127.0.0.1:49177/json_rpc): operation was canceled: connection closed before message completed
```
Given the message `connection closed before message completed` it is likely that the `monero-wallet-rpc` is not fully started yet.
Unfortunately we cannot wait to see a different message in the logs, because there are just no further deterministic messages after the one we are currently listening on.
to overcome this problem without extending testcontainers we introduce a retry mechanism when creating the wallet.
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.
This will allow us to compile on stable Rust.
The latest version of `secp256kfun` uses `curve25519-dalek-ng` instead
of the original curve25519-dalek crate. Instead of converting back and
forth, we simply switch to this crate as well. Judging from the README
it is just a fork because there was trouble between the maintainers of
the original crate.
528: Add the arm CLI build to the release binaries r=da-kami a=da-kami
Currently it is not shipped because we forgot to include it.
I don't see a reason not to ship that as well. @bonomat and me tested it on a raspi last Friday :)
Co-authored-by: Daniel Karzel <daniel@comit.network>
527: Release version 0.6.0 r=da-kami a=comit-botty-mc-botface
Hi @da-kami!
This PR was created in response to a manual trigger of the release workflow here: https://github.com/comit-network/xmr-btc-swap/actions/runs/870083908.
I've updated the changelog and bumped the versions in the manifest files in this commit: 30b2cb467a8dde873da16cedf69f5b4ccbdb7508.
Merging this PR will create a GitHub release and upload any assets that are created as part of the release build.
Co-authored-by: Daniel Karzel <daniel@comit.network>
Co-authored-by: COMIT Botty McBotface <botty@coblox.tech>
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.
520: Cli json logging r=da-kami a=da-kami
Combining `--json` with the debug file logger was a pita, so I stopped and went for a simpler approach:
If `--json` is given we just log to terminal - **no** logfiles will be created in `{data-dir}/logs`.
The `--debug` flag applies to `--json` (i.e. if not given it will just print json on info level). We could change that to automatically fallback to debug - could add a `required_if` dependency via strucopt/clap but I did not want to invest more time into thinking about this.
Note on extending binary functionality:
As discussed with @thomaseizinger recently, we will have to think about multiple binaries soon, i.e. a binary that focuses to be used to building on top of it (that always logs json) and potientially keeping a simple CLI that is more user friendly. This also goes towards more clearly separating the application code from re-usable protocol / network code.
Co-authored-by: Daniel Karzel <daniel@comit.network>
519: Avoid application error upon `--help` r=da-kami a=da-kami
Our `preview` release is currently broken because of this issue.
The way we use clap's `get_matches_from_safe` caused parsing errors upon `--help` and `--version` to be bubbled up to the application - which causes the application to exit with an error when running `--help` and `--version`.
This is solved by using `get_matches_from` instead of `get_matches_from_safe` which handles these known clap commands internally and exits early.
Added smoke tests to CI so we catch such kind of problems in the future. Smoke testing by calling `--help` is cheap and should be OK in CI.
Co-authored-by: Daniel Karzel <daniel@comit.network>
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.
518: Fix bug that breaks swap ID for logging r=da-kami a=da-kami
Somehow I introduced this really stupid bug. Might have happened during a rebase.
We create the swap id at the top of main.
Creating another id here breaks the name of the swap's logfile for the CLI, because the actual swap will have another id.
Should definitely go in before releasing :)
Co-authored-by: Daniel Karzel <daniel@comit.network>
490: Mainnet switch r=da-kami a=da-kami
Fixes #446Fixes#360Fixes#506Fixes#478
To be precise: It is actually a testnet switch, because I think mainnet should be default.
I took several assumptions on the way (e.g. network support, ...).
At this stage any feedback welcome :)
TODO:
- [ ] successful mainnet swap with this code base before merging :)
Co-authored-by: Daniel Karzel <daniel@comit.network>
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.
