Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harden moving funds against edge cases #3812

Closed
1 task done
lukasz-zimnoch opened this issue Apr 26, 2024 · 0 comments
Closed
1 task done

Harden moving funds against edge cases #3812

lukasz-zimnoch opened this issue Apr 26, 2024 · 0 comments
Assignees

Comments

@lukasz-zimnoch
Copy link
Member

lukasz-zimnoch commented Apr 26, 2024

The tBTC protocol has an edge case related to moving funds:

  1. Wallet A enters into the MovingFunds state
  2. Wallet A submits its moving funds commitment and sets Wallet B as one of the target wallets (Wallet B must be Live at that moment)
  3. Wallet B enters into the MovingFunds state
  4. SPV proof of the Wallet A moving funds transaction lands in the Bridge after Wallet B submits its own moving funds transaction
  5. Funds moved from Wallet A to Wallet B remain on Wallet B and are not moved as part of its own moving funds transaction. The Wallet B can finalize its moving funds process and switch to the Closing state without trouble. The moved funds sweep request representing funds from Wallet A remains Pending forever.

This case requires some unfortunate circumstances and its probability is very low on mainnet. Even if it occurs, funds would not be lost and members of Wallet B can coordinate to return them to the Bridge using the donation mechanism. However, we need to undertake some actions to minimize that probability even further.

There is no silver bullet allowing us to mitigate it completely. In general, target wallets (like Wallet B) cannot wait with their own moving funds processes for too long. First, moving funds is a process constrained by a timeout. Second, funds that are about to be moved are temporarily pulled out from the protocol liquidity pool and cannot be used to handle redemptions. That said, we need to find the right trade-off here.

PR #3810 introduced a 24-hour safety margin that forces wallets to preserve that time before executing their moving funds transactions. Apart from addressing the last-minute deposit problem, this helps minimize the probability of the aforementioned problematic scenario as Wallet B gives more time to Wallet A to complete their moving funds process and submit the SPV proof of it. However, we can do it better.

The tbtc.ValidateMovingFundsSafetyMargin function can do the following:

  1. Fetch past MovingFundsCommitmentSubmitted events from the last 30 days
  2. Take all events whose target wallets list contains the wallet the validation is performed for
  3. From the above set, check if there is at least one event whose source wallet is still in the MovingFunds state. That would mean at least one moving funds process targeting the validated wallet is in progress
  4. If the above is true, use a 14-day safety margin. Otherwise, use the default 24 hours.
  5. Additionally, the function should make sure that the computed margin is not greater than half of the Bridge.movingFundsTimeout (min(computedMargin, 0.5 * movingFundsTimeout). This is a must to ensure that the validated wallet would have enough time to complete its own moving funds process.

Side note regarding the implementation: The tbtc.ValidateMovingFundsSafetyMargin function takes *WalletChainData as a parameter currently. It should be refactored to take a chain handle exposing just the necessary methods (similarly to tbtc.ValidateMovingFundsProposal). Please also cover tbtc.ValidateMovingFundsSafetyMargin with proper unit tests.

Tasks

  1. 📟 client
    tomaszslabon
lukasz-zimnoch added a commit that referenced this issue May 6, 2024
#Refs #3812.
This PR modifies the safety margin validation process used during moving
funds.
It is possible that a wallet may receive deposits just before it changes
states to `MovingFunds`.
It is also possible another wallets in `MovingFunds` state may commit to
transfer their funds to it.
To avoid a situation where a wallet ends up with additional funds after
it has already moved their own funds we must apply a safety margin.
In #3810 we already added
a 24-hour safety margin.
In this PR we add a longer 14-days safety margin when the wallet is a
target of a moving funds process from another wallet. We also make sure
the calculated safety margin is not greater than half of the
`movingFundsTimeout`, so that a wallet has enough time to finish their
moving funds process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants