TIP-100 preparation #171
TIP-100 preparation #171
Conversation
…xisting deauthorization
vzotova
force-pushed
the
tip-100-preparation
branch
from
ce10810 to
50df19c
Compare
vzotova
requested review from
cygnusv,
lukasz-zimnoch and
pdyraga
as code owners
|
I am confused about the proposed changes and how they relate to TIP-92. The TIP-92 says:
What I believe this Pull Request currently proposes is to allow anyone to decrease their stake up to 15M T but it does not allow them to unstake everything. Also, I am not sure if using the I am also unsure about removing the functions for staking and increasing authorization. Why to prevent new T stakers from staking if they want to run TACo nodes or why to prevent the DAO from adding new beta stakers to tBTC? Until a change is made to how the wallet registry and sortition pool work - and note the sortition pool is not an upgradeable contract - this is the only way to add new beta stakers. Same question for increasing authorization. One that currently has 1M T stake in TACo, may want to increase it to 15M T stake, I think we should not be disallowing it. Last but not least, I fundamentally disagree with TIP-92 idea of locking the stake of an arbitrary group of stakers by majority vote. The beta stakers did not opt-in for the new rules and the majority shouldn't change them in the middle of the game, especially with such short notice. |
manumonti
left a comment
manumonti
left a comment
There was a problem hiding this comment.
Also, the event AuthorizationInvoluntaryDecreased is no longer necessary, right? can we get rid it of it?
Great work! I have some minor suggestions in case you think it's worthwhile
vzotova
force-pushed
the
tip-100-preparation
branch
4 times, most recently
from
3aa2b79 to
e42be37
Compare
vzotova
force-pushed
the
tip-100-preparation
branch
from
e42be37 to
74035ef
Compare
|
|
||
| /// @notice Forced deauthorization of Beta stakers. | ||
| /// Can be called only by the governance. | ||
| function forceBetaStakerDecreaseAuthorization(address[] memory betaStakers) |
There was a problem hiding this comment.
The PR description says:
(...) The main goals are:
- Unlock stakes of beta operators, while keeping the beta operator set untouched: From now on, beta operators node will be compensated directly by the DAO, so there's no need for locked tokens for them. Once the upgrade goes live, Council will call
forceBetaStakerDeauthorization(address[] betaStakers)with the final list of beta stakers. Given the criticality of beta operators for tBTC app contract, this PR unblocks tBTC and RB authorizations for beta stakers only from the staking contract side, without pushing these changes to the tBTC app contracts and maintaining the same views for authorization amounts for them
I am confused about this part. After d78cbda, forceBetaStakerDecreaseAuthorization removes TACo authorization for tBTC beta stakers - nothing more, nothing less. Random Beacon and tBTC authorizations are not unblocked.
The same case is for forceAuthorizationCap. The beta stakers are not reducing their authorization, as all applications but TACo are skipped in function forceDecreaseAuthorization(address stakingProvider, uint96 amountTo).
There was a problem hiding this comment.
Our understanding from your earlier feedback was that to keep sortition pool seats untouched it didn't suffice to simply not push authorization changes from the staking contract to apps; keeping authorization amounts in the staking contract was also important, since for some reason, tbtc and rb app contracts will pull authorization amounts from it. Hence, what this PR does is simply ignoring tbtc and rb apps when withdrawing tokens, which has the effect of unblocking all tokens blocked by tbtc and rb apps.
| function seize( | ||
| uint96 amount, | ||
| uint256 rewardMultiplier, | ||
| address notifier, | ||
| address[] memory _stakingProviders | ||
| ) external override { | ||
| notify(amount, rewardMultiplier, notifier, _stakingProviders); | ||
| } |
There was a problem hiding this comment.
tBTC has slashing and this function is used by tBTC application contracts. Slashing happens when the key generation result is challenged. This prevents taking over the newly created wallet. In case of key generation result challenge, non-existing seize function probably should not break the challenge given it is wrapped in a try-catch and we consider this step optional but this is NOT the case for other tBTC functions requiring the stake to be slashed to complete: notifyWalletMovingFundsTimeout, notifyWalletFraudChallengeDefeatTimeout, notifyWalletRedemptionTimeout, notifyWalletMovedFundsSweepTimeout.
There was a problem hiding this comment.
Added stubs for "slash" and "seize"
| /// application. | ||
| /// @dev Calls `authorizationDecreaseRequested` callback | ||
| /// for each authorized application. See `IApplication`. | ||
| function requestAuthorizationDecrease(address stakingProvider) external { |
There was a problem hiding this comment.
How tBTC stakers (beta or not) deauthorize after this change? forceBetaStakerDecreaseAuthorization(address[] memory betaStakers) applies only a change to TACo authorization. Same about forceDecreaseAuthorization(address stakingProvider, uint96 amountTo).
The only option seems to be the forceDecreaseAuthorization(address stakingProvider, address application) but it requires the application to be marked as disabled. Do we want to do it for tBTC contracts? Note this option will allow beta stakers to unstake immediately.
| if (maxAuthorization > MAX_STAKE) { | ||
| forceDecreaseAuthorization(stakingProvider, MAX_STAKE); | ||
| maxAuthorization = MAX_STAKE; | ||
| availableToOptOut = HALF_MAX_STAKE; | ||
| } |
There was a problem hiding this comment.
We already have a function to forceAuthorizationCap() reducing all authorizations above 15m T, which the Council will call in the same TX batch than the upgrade, so I guess this part of the code is just in case?
Goals and design decisions
This PR enacts the changes required after the approval of TIP-92 & TIP-100. The main goals are:
Unlock stakes of beta operators, while keeping the beta operator set untouched: From now on, beta operators node will be compensated directly by the DAO, so there's no need for locked tokens for them. Once the upgrade goes live, Council will call
forceBetaStakerDeauthorization(address[] betaStakers)with the final list of beta stakers. Given the criticality of beta operators for tBTC app contract, this PR unblocks tBTC and RB authorizations for beta stakers only from the staking contract side, without pushing these changes to the tBTC app contracts and maintaining the same views for authorization amounts for them. Hence, from the perspective of tBTC and RB app contracts, tBTC stakers (beta and non-beta) remain with the same authorization amounts, which means tBTC protocol can continue to work without disruption. At a later stage, tBTC app contract maintainers can upgrade their app contracts to become essentially an allowlist contract for beta stakers.Implement TACo authorizations rule from the transition period: As stated by TIP-92 & TIP-100, TACo is branching out from Threshold to a new setting, but we want this process to be as orderly and fair as possible, while keeping the TACo network fully operational, continuing to serve existing adopters. A transition period of 6 months was proposed by TIP-92 and TIP-100, which is exactly the current deauthorization delay for TACo (182 days), so a way to see this is as if all TACo nodes collectively started deauthorization on February 15th, which means their tokens will be completely unlocked in August 16th, 2025. In addition:
Restricting several functionalities of the staking contract, to reduce complexity and risks during the transition period. In particular, this PR deactivates the possibility of creating new stakes, top-ups, authorization increases, slashing, and everything that won’t be needed during and after transition.
Actions
Step 1. Council upgrades staking contract
Step 2. Council calls
forceBetaStakerDeauthorization(address[] betaStakers)with the final list of beta stakersStep 3. Council calls
forceAuthorizationCap(address[])with a list of all remaining stakes over 15m T.Next steps
After the transition period ends (August 16th, 2025), the staking contract will no longer have a critical role in the network.
All T tokens will be completely unlocked, and remaining stakers will be encouraged to unstake everything, reducing smart contract risk. That will require an additional upgrade close to the transition period deadline. In the meantime, tBTC app contracts should also transition to a pure allowlist model that doesn't require authorization amounts, since the corresponding T tokens are actually unlocked by the current upgrade. This can be done in an orderly way during transition.