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.
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>
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 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.
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.
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.
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.
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.
The only reason we need this argument is because we need to access
the output descriptor. We can save that one ahead of time at when
we construct the type.
To achieve this we also:
- upgrade rust-bitcoin to 0.26
- upgrade bitcoin-harness to latest version (which also depends bitcoin 0.26)
- upgrade to latest edcsa-fun
- replace cross_curve_dleq proof with sigma_fun (to avoid an upgrade dance over there)
We are aware of issues of timeouts when waiting for acknowledgements.
Also, to properly supports acks in a multiple swap context, we need to
revert to doing event processing on the behaviour so that we can link
leverage the `RequestResponse` libp2p behaviour and link the messages
requests ids to swap ids when receiving an ack or response.
Acks are usefully for specific scenarios where we queue a message on the
behaviour to be sent, save as sent in the DB but crash before the
message is actually sent. With acks we are able to resume the swap,
without ack, the swap will abort (refund).
`alice::swap::run_until` will be called once the execution setup is
done. The steps before are directly handled by the event loop,
hence no channels are needed for said steps: connection established,
swap request/response & execution setup.
This was introduced due to a CI run, where Bob included tx_refund, but Alice had waited until T2 had expired,
and then went for punishing Bob instead of refunding.
Weirdly, Alice's punich transaction did not fail in that scenario.
If dialing Bob fails Alice waits for the acknowledgement of the transfer proof indefinitely.
The timout prevents her execution from hanging.
Added a ToDo to re-visit the ack receivers. They don't add value at the moment and should be removed.
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.
149: Fix Alice redeem scenario r=da-kami a=da-kami
Follow up of #144, partial fix of https://github.com/comit-network/xmr-btc-swap/issues/137
Fix Alice redeem scenario
- Properly check the timelocks before trying to redeem
- Distinguish different failure scenarios and reactions to it.
- if we fail to construct the redeem transaction: wait for cancel.
- if we fail to publish the redeem transaction: wait for cancel but let the user know that restarting the application will result in retrying to publish the tx.
- if we succeed to publish the tx but then fail when waiting for finality, print error to the user (secreat already leaked, the user has to check manually if the tx was included)
Co-authored-by: Daniel Karzel <daniel@comit.network>
Thomas Eizinger
bors[bot]
rishflab
Daniel Karzel
Franck Royer