Skip to content

Commit

Permalink
Improve documentation comments around redemptions veto
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasz-zimnoch committed Mar 5, 2024
1 parent b6aef91 commit c6c03a3
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 5 deletions.
5 changes: 4 additions & 1 deletion solidity/contracts/bridge/BridgeState.sol
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ library BridgeState {
// timed out. It is counted from the moment when the redemption
// request was created via `requestRedemption` call. Reported
// timed out requests are cancelled and locked TBTC is returned
// to the redeemer in full amount.
// to the redeemer in full amount. If a redemption watchtower
// is set, the redemption timeout should be greater than the maximum
// value of the redemption delay that can be enforced by the watchtower.
// Consult `IRedemptionWatchtower.getRedemptionDelay` for more details.
uint32 redemptionTimeout;
// The amount of stake slashed from each member of a wallet for a
// redemption timeout.
Expand Down
17 changes: 15 additions & 2 deletions solidity/contracts/bridge/Redemption.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ interface IRedemptionWatchtower {
/// receive the redeemed amount.
/// @param balanceOwner The address of the Bank balance owner whose balance
/// is getting redeemed.
/// @param redeemer The address that requested the redemption.
/// @param redeemer The address that requested the redemption and will be
/// able to claim Bank balance if anything goes wrong during the
/// redemption. In the most basic case, when someone redeems their
/// Bitcoin balance from the Bank, `balanceOwner` is the same
/// as `redeemer`. However, when a Vault is redeeming part of its
/// balance for some redeemer address (for example, someone who has
/// earlier deposited into that Vault), `balanceOwner` is the Vault,
/// and `redeemer` is the address for which the vault is redeeming
/// its balance to.
/// @return True if the redemption request is safe, false otherwise.
/// Specific safety criteria depend on the implementation.
function isSafeRedemption(
Expand Down Expand Up @@ -413,6 +421,8 @@ library Redemption {
/// `amount - (amount / redemptionTreasuryFeeDivisor) - redemptionTxMaxFee`.
/// Fees values are taken at the moment of request creation.
/// @dev Requirements:
/// - If the redemption watchtower is set, the redemption request must
/// be considered safe by the watchtower,
/// - Wallet behind `walletPubKeyHash` must be live,
/// - `mainUtxo` components must point to the recent main UTXO
/// of the given wallet, as currently known on the Ethereum chain,
Expand Down Expand Up @@ -1169,7 +1179,10 @@ library Redemption {
redemption.treasuryFee;

// Capture the amount that should be transferred to the
// redemption watchtower.
// redemption watchtower. Use the whole requested amount as a detained
// amount because the treasury fee is deducted in `submitRedemptionProof`.
// Since the redemption did not happen, the treasury fee was not
// deducted and the whole requested amount should be detained.
uint64 detainedAmount = redemption.requestedAmount;

// Delete the redemption request from the pending redemptions
Expand Down
8 changes: 6 additions & 2 deletions solidity/contracts/bridge/RedemptionWatchtower.sol
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,9 @@ contract RedemptionWatchtower is OwnableUpgradeable {
Redemption.RedemptionRequest memory redemption = bridge
.pendingRedemptions(redemptionKey);

// This also handles the case when the redemption was already processed
// given we delete successful ones from the `pendingRedemptions` mapping
// of the `Bridge` state.
require(
redemption.requestedAt != 0,
"Redemption request does not exist"
Expand All @@ -343,8 +346,9 @@ contract RedemptionWatchtower is OwnableUpgradeable {
if (redemption.requestedAt >= watchtowerEnabledAt) {
require(
// Use < instead of <= to avoid a theoretical edge case
// where the delay is 0 (veto disabled) but three objections
// are raised in the same block as the redemption request.
// where the delay is 0 (watchtower disabled OR amount below
// the veto threshold) but three objections are raised in the
// same block as the redemption request.
/* solhint-disable-next-line not-rely-on-time */
block.timestamp <
redemption.requestedAt +
Expand Down
8 changes: 8 additions & 0 deletions solidity/contracts/bridge/WalletProposalValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,14 @@ contract WalletProposalValidator {
.getRedemptionDelay(redemptionKey);
// If the delay is greater than the usual minimum age, use it.
// This way both the min age and the watchtower delay are preserved.
//
// We do not need to bother about last-minute objections issued
// by the watchtower. Objections can be issued up to one second
// before the min age is achieved while this validation will
// pass only one second after the min age is achieved. Even if
// a single objection stays longer in the mempool, this won't
// be a problem for `Bridge.submitRedemptionProof` which ignores
// single objections as long as the veto threshold is not reached.
if (delay > minAge) {
minAge = delay;
}
Expand Down

0 comments on commit c6c03a3

Please sign in to comment.