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.
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.
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.
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.
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.
Awaiting the confirmations in an earlier state can cause trouble with resuming
swaps with short cancel expiries (test scenarios).
Since it is the responsibility of the refund state to ensure that the XMR can
be sweeped, we now ensure that the lock transaction has 10 confirmations before
refunding the XMR using generate_from_keys.
Sending the transfer transaction in a distinct state helps ensuring
that we do not send the Monero lock transaction twice in a restart
scenario.
Waiting for the first transaction confirmation in a separate state
helps ensuring that we send the transfer proof in a restart scenario.
Once we resume unfinished swaps upon startup we have to ensure that
it is safe for Alice to act.
If Bob has locked BTC it is only make sense for Alice to lock up the
XMR as long as no timelock has expired. Hence we abort if the BTC is
locked, but any timelock expired already.
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.
To achieve this, we need to add some pure helpers to the state structs.
This has the added benefit that we can reduce the amount of code within
the swap function.
If TxLock does not confirm in a reasonable amount of time, Alice should
give up on the swap rather than waiting forever. Watching for TxLock in
the mempool is not required and it causes unnecessary complexity. What
if Alice does not see the transaction in mempool but it is already
confirmed? She will abort the swap for no reason.
Instead of watching for status changes directly on bitcoin::Wallet,
we return a Subscription object back to the caller. This subscription
object can be re-used multiple times.
Among other things, this now allows callers of `broadcast` to decide
on what to wait for given the returned Subscription object.
The new API is also more concise which allows us to remove some of
the functions on the actor states in favor of simple inline calls.
Co-authored-by: rishflab <rishflab@hotmail.com>
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.
319: Alice sweeps refunded funds into default wallet r=da-kami a=da-kami
Alice's refund scenario starts with generating the temporary wallet
from keys to claim the XMR which results in Alice' unloading the wallet.
Alice then loads her original wallet to be able to handle more swaps.
Since Alice is in the role of the long running daemon handling concurrent
swaps, the operation to close, claim and re-open her default wallet must
be atomic.
This PR adds an additional step, that sweeps all the refunded XMR back into
the default wallet. In order to ensure that this is possible, Alice has to
ensure that the locked XMR got enough confirmations.
These changes allow us to assert Alice's balance after refunding.
Co-authored-by: Daniel Karzel <daniel@comit.network>
If we enter a punish scenario we can be sure the punish timelock is expired.
Thus, we must be able to punish unless Bob published the refund transaction.
There is no benefit in racing punish against refund here, because we cannot recover from a punish tx failure anyway.
The logic was changed to:
Try to broadcast punish tx and await finality.
If either punish broadcasting of finality fails, try to fetch the refund transaction.
If it is available extract Bob's Monero key part and transition to refund.
If refund tx is not available fail without a status update.
Note that we do not distinguish different errors upon failure of punish, because
we cannot recover anyway. If we fail to retrieve Bob's refund tx, we just exit without
a status update so punish can be retried by resuming the swap.
Since Alice's refund scenario starts with generating the temporary wallet
from keys to claim the XMR which results in Alice' unloading the wallet.
Alice then loads her original wallet to be able to handle more swaps.
Since Alice is in the role of the long running daemon handling concurrent
swaps, the operation to close, claim and re-open her default wallet must
be atomic.
This PR adds an additional step, that sweeps all the refunded XMR back into
the default wallet. In order to ensure that this is possible, Alice has to
ensure that the locked XMR got enough confirmations.
These changes allow us to assert Alice's balance after refunding.
To achieve this, we decompose `watch_for_locked_xmr` into two parts:
1. A non-self-consuming function to construct a `WatchRequest`
2. A state transition that can now consume `self` again because
it is only called once within the whole select! expression.
Ideally, we would move more logic onto this state transition (like
comparing the actual amounts and fail the transition if it is not
valid). Doing so would have an unfortunate side-effect: We would
always wait for the full confirmations before checking whether or
not we actually receive enough XMR.
This allows us to have state transitions that consume self.
Instead of calling this function in all the branches, we can simply
make the whole match statement evaluate to the new state and perform
this functionality at the very end.
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>
322: Refactor `ExecutionParams` and harmonize sync intervals of wallets r=thomaseizinger a=thomaseizinger
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.
