Liquidation-entry refactor #573
Conversation
startSignerFraudLiquidatoin and StartSignerAbortLiquidation have been consolidated to a single `startLiquidation` method a boolean is passed to differentiate between abort and fraud if coming from redemption, always setLiquidated and make signer bonds redeemer-withdrawable
This was only used to check if we're coming from a redemption flow. We now use `inRedemption` to check if we're coming from redemption flow.
reclaim some more state by zeroing out redeemerAddress
| /// @param _d Deposit storage pointer. | ||
| function startSignerFraudLiquidation(DepositUtils.Deposit storage _d) internal { | ||
| _d.logStartedLiquidation(true); | ||
| /// @notice Starts signer liquidation due to abort, undercollateralization, or fraud |
There was a problem hiding this comment.
How about something like “Starts signer liquidation by seizing signer bonds. If the deposit is currently being redeemed, the redeemer receives the full bond value; otherwise, a falling price auction begins to buy 1 TBTC in exchange for a portion of the seized bonds; see purchaseSignerBondsAtAuction.”
Since this is internal, I think the whole thing can be @dev.
| _d.logStartedLiquidation(true); | ||
| /// @notice Starts signer liquidation due to abort, undercollateralization, or fraud | ||
| /// @dev Liquidation is done by auction. | ||
| /// @param _wasFraud Boolean checking liquidation cause. True if fraud, false otherwise. |
There was a problem hiding this comment.
I think this could be more clearly stated as “True if liquidation is being started due to fraud, false if for any other reason.”
| _d.setLiquidated(); | ||
| _d.enableWithdrawal(_d.redeemerAddress, _seized); | ||
| _d.logLiquidated(); | ||
| // Reclaim used state for gas savings | ||
| _d.redemptionTeardown(); |
There was a problem hiding this comment.
We can still do this once alongside the bond seizure, right? redemptionTeardown doesn't change the deposit state, so the _d.inRedemption() check should still work.
There was a problem hiding this comment.
redemptionTeardown() zeroes out redeemerAddress
This is new, I just added it to claim more state back.
There was a problem hiding this comment.
I just saw comment below...
| _d.redeemerOutputScript = ""; | ||
| _d.initialRedemptionFee = 0; | ||
| _d.withdrawalRequestTime = 0; | ||
| _d.lastRequestedDigest = bytes32(0); | ||
| _d.redeemerAddress = address(0); |
There was a problem hiding this comment.
This affects my sinister plan above. I kind of like keeping the redeemer address as part of the final state of a deposit though… It means a closed deposit knows its original lot size, its final owner, and its redeemer. Feels clean somehow? Though I realize this may make the final tx cheaper 🤔
There was a problem hiding this comment.
It also feels like cleaning this up could inadvertently break some stuff that isn't worth spending time on fixing…
There was a problem hiding this comment.
I'd rather save the gas than have a redeemer value in storage.
It also feels like cleaning this up could inadvertently break some stuff that isn't worth spending time on fixing…
when we reclaim this state we are always wrapping up. as long as the state is reclaimed after its last use there should be no problem
There was a problem hiding this comment.
We're also assuming the gas saved will be saved, since there's a limit to how much gets refunded via state release. The transactions that do this aren't super expensive.
There was a problem hiding this comment.
that's a fair point, but we're definitely not reaching the cap for functions like provideRedemptionProof. We'll surely see the refund there
Clarify what's happening with the liquidation, as wella s the boolean parameter
if redeemerAddress is stored locally so it is not lost after redemptionTeardown, a shared call to redemptionTeardown() is enough for fraud and non-fraud startLiquidation paths.
startSignerFraudLiquidatoinandStartSignerAbortLiquidationhave been consolidated to a singlestartLiquidationmethod. a boolean is passed to differentiate between abort and fraud.if coming from redemption, always
setLiquidatedand make the signer bonds redeemer-withdrawable. Check the deposit state directly rather than relying onredeemerAddressbeing set to determine if we're coming from the liquidation flow.closes: #553
draft untill:
ensure redeemable state-> moved to Test liquidation flows #319 .startLiquidationpath doesn't introduce any holes