Let the staking provider to refresh KEEP stake owner #103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
T network staking contract supports existing KEEP stakes by allowing KEEP stakers to use their stakes in T network and weights them based on KEEP<>T token ratio. KEEP stake owner is cached in T staking contract and used to restrict access to all functions only owner should be able to execute. To cache KEEP stake owner in T staking contract, T staking contract first needs to resolve the owner. Resolving liquid KEEP stake owner is easy. Resolving token grant stake owner is complicated and not possible to do on-chain from a contract external to KEEP `TokenStaking` contract. Keep `TokenStaking` knows the grant ID but does not expose it externally. This problem is solved by `KeepStake` contract. It contains operator-owner pairs that were snapshotted 2021-11-11. If KEEP staker does not want to go liquid and instead they use their grant for delegation, and their grant was created after the snapshot date of 2021-11-11, `KeepStake` contract owner needs to register a new owner-operator pair for them. The problem though is when the owner staked *before* `KeepStake` owner registered a pair of them. An example is staking provider `0xA5f6822ef1A7DF72628259f9D1dc17eb2BCb2385` that received tokens delegated by owner `0x78dc7f84a8da5c96c4c54dcf5bd6e9ccd4fe2bb5` at transaction `0x262a7b173e2572e4494b3470c4978903e4d9f95141a18ad79c7d9090f16c9874`. Before that delegation was done, no owner-pair was registered in `KeepStake` for `0x78dc7f84a8da5c96c4c54dcf5bd6e9ccd4fe2bb5` and `KeepStake.resolveOwner(0xA5f6822ef1A7DF72628259f9D1dc17eb2BCb2385)` returns `0xF139B3137D5bF24848e4F8FbE1d6AB70dF90c5Fa` which is the master Keep `TokenGrantStake` contract. As a result, `0xA5f6822ef1A7DF72628259f9D1dc17eb2BCb2385` can not execute calls as an owner. The solution for this kind of situation would be to register the pair in `KeepStake` contract and call `TokenStaking.refreshKeepOwner()` but this function has a rule authorizing only the old owner to call this function. The owner is the master `TokenGrantStake` contract though. The fix proposed here is to allow `TokenStaking.refreshKeepOwner()` call from the staking provider as well. We could probably also allow the authorizer to call it, but the staking provider feels good enough and such situations should be rare.
|
System tests are in fact passing if you look at the execution log. It looks like some configuration problem, yet one issue after the fix from #102. |
vzotova
previously approved these changes
May 6, 2022
cygnusv
previously approved these changes
Aug 17, 2022
The versions were updated to more recent ones, the same as used by tbtc-v2. This update allows to elimimnate the problem when running system tests. Although all system tests were passing, the process kept failing with: Error: Cannot find module './test/system/staking.test.js' This was happening both on CI and locally. After the update, the problem is gone.
|
@vzotova @manumonti @cygnusv I updated hardhat, ethers, and hardhat plugin versions in 7c05a34 and now system tests job is passing. System tests were passing before as well but the job was exiting with some test-unrelated error. |
cygnusv
approved these changes
Aug 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
T network staking contract supports existing KEEP stakes by allowing KEEP stakers to use their stakes in T network and weights them based on KEEP<>T token ratio.
KEEP stake owner is cached in T staking contract and used to restrict access to all functions only the owner should be able to execute. To cache KEEP staking contract in T staking contract, T staking contract first needs to resolve the owner. Resolving liquid KEEP stake owner is easy. Resolving token grant stake owner is complicated and not possible to do on-chain from a contract external to KEEP
TokenStakingcontract. KeepTokenStakingknows the grant ID but does not expose it externally.This problem is solved by
KeepStakecontract. It contains operator-owner pairs that were snapshotted 2021-11-11. If KEEP staker does not want to go liquid and instead they use their grant for delegation, and their grant was created after the snapshot date of 2021-11-11,KeepStakecontract owner needs to register a new owner-operator pair for them.The problem though is when the owner staked before
KeepStakeowner registered a pair of them.An example is staking provider
0xA5f6822ef1A7DF72628259f9D1dc17eb2BCb2385that received tokens delegated by owner0x78dc7f84a8da5c96c4c54dcf5bd6e9ccd4fe2bb5at transaction0x262a7b173e2572e4494b3470c4978903e4d9f95141a18ad79c7d9090f16c9874.Before that delegation was done, no owner-pair was registered in
KeepStakefor0x78dc7f84a8da5c96c4c54dcf5bd6e9ccd4fe2bb5andKeepStake.resolveOwner(0xA5f6822ef1A7DF72628259f9D1dc17eb2BCb2385)returns0xF139B3137D5bF24848e4F8FbE1d6AB70dF90c5Fawhich is the master KeepTokenGrantStakecontract. As a result,0xA5f6822ef1A7DF72628259f9D1dc17eb2BCb2385can not execute calls as an owner.The solution for this kind of situation would be to register the pair in
KeepStakecontract and callTokenStaking.refreshKeepOwner()but this function has a rule authorizing only the old owner to call this function. The owner is the masterTokenGrantStakecontract though.The fix proposed here is to allow
TokenStaking.refreshKeepOwner()call from the staking provider as well. We could probably also allow the authorizer to call it, but the staking provider feels good enough and such situations should be rare.