Some network and application specific code does not belong in the protocol module and was moved.
Eventloop, recovery and the outside behaviour were moved to the respective application module because they are application specific.
The `swap_setup` was moved into the network module because upon change both sides will have to be changed and should thus stay close together.
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).
It seems the current chosen channel timeouts are still not optimal.
I ran into issues with swapping over Tor and traced them down to the CLI timeout of the bmrng channel.
It appears that the ASB was not running as quick as the CLI, which caused a timeout on the CLI side (in addition to the delay when sending messages over Tor).
Only `execution_setup` caused the problem so far, but I would recommend changing all the channel timeouts to one minute to avoid this problem.
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.
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.
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.
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!
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
In order for the re-construction of TxLock to be meaningful, we limit
`Message2` to the PSBT instead of the full struct. This is a breaking
change in the network layer.
The PSBT is valid if:
- It has at most two outputs (we allow a change output)
- One of the outputs pays the agreed upon amount to a shared output script
Resolves#260.
This allows us to remove all visibility modifiers from the message
fields because child modules (in this case {alice,bob}::state) can
always access private fields of structs.
It also moves the messages into a more natural place. Previously,
they were defined within the network layer even though they are
independent of the libp2p implementation.
Thomas Eizinger
Daniel Karzel
Philipp Hoenisch
dependabot[bot]
bors[bot]