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).
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.
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.
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.
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.
- 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.
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.
We don't need to hide the fields of this Behaviour as the only reason
for why this struct exists is because libp2p forces us to compose our
NetworkBehaviours into a new struct.
The request-response behaviour that is used for sending the transfer
proof actually has a functionality for buffering a message if we
are currently not connected. However, the request-response behaviour
also emits a dial attempt and **drops** all buffered messages if this
dial attempt fails. For us, the dial attempt will very likely always
fail because Bob is very likely behind NAT and we have to wait for
him to reconnect to us.
To mitigate this, we build our own buffer within the EventLoop and
send transfer proofs as soon as we are connected again.
Resolves#348.
The swap should not be concerned with connection handling. This is
the responsibility of the overall application.
All but the execution-setup NetworkBehaviour are `request-response`
behaviours. These have built-in functionality to automatically emit
a dial attempt in case we are not connected at the time we want to
send a message. We remove all of the manual dialling code from the
swap in favor of this behaviour.
Additionally, we make sure to establish a connection as soon as the
EventLoop gets started. In case we ever loose the connection to Alice,
we try to re-establish it.
Decomposing a RequestResponseEvent is quite verbose. We can introduce
a helper function that does the matching for us and delegates to
specific `From` implementations for the protocol specific bits.
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.
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.
Daniel Karzel
Thomas Eizinger
Philipp Hoenisch
dependabot[bot]
bors[bot]