Halting inflation #2822
Halting inflation #2822
Conversation
vzotova
force-pushed
the
inflation-stop
branch
from
0ed2fb7
to
910fb49
Compare
vzotova
force-pushed
the
inflation-stop
branch
from
6b998e7
to
a1be9e6
Compare
vzotova
requested review from
KPrasch,
cygnusv,
derekpierre,
arjunhassard and
mswilkison
| @@ -109,9 +67,9 @@ contract StakingEscrowStub is Upgradeable { | |||
| * @title StakingEscrow | |||
| * @notice Contract holds and locks stakers tokens. | |||
| * Each staker that locks their tokens will receive some compensation | |||
| * @dev |v5.7.1| | |||
| * @dev |v6.1.1| | |||
There was a problem hiding this comment.
Might be a silly question, but what's the reason this isn't just 6.0.0?
There was a problem hiding this comment.
I see that we are using min version as 1 (for previous contracts). But honestly can't remember 😅
| * @param period Previous period tokens minted for | ||
| * @param value Amount minted (in NuNits) | ||
| */ | ||
| event Minted(address indexed staker, uint16 indexed period, uint256 value); |
There was a problem hiding this comment.
Is it possible we may need these event definitions for historical purposes? In such case, we could move them to IStakingEscrow.
There was a problem hiding this comment.
not sure about that, in case we need it we can just use some old abi from registry
| } | ||
|
|
||
| /** | ||
| * @notice Get work that completed by the staker | ||
| */ | ||
| function getCompletedWork(address _staker) external view returns (uint256) { | ||
| return stakerInfo[_staker].completedWork; | ||
| return token.totalSupply(); |
There was a problem hiding this comment.
Just to double-check: this implies that we consider all pending work in WorkLock to be done, so people that still have ETH locked can claim it back. Correct? In such case, I think this should be included in the newsfragment.
| } | ||
| } | ||
| addSnapshot(info, int256(_value)); | ||
| emit Deposited(_staker, _value); |
There was a problem hiding this comment.
So, we are removing the possibility of depositing NU to StakingEscrow, but we're still allowing WorkLock deposits. Can this have implications on the Threshold side? Maybe a possibility is to somehow end WL completely (i.e. refund & no more deposits)?
There was a problem hiding this comment.
there is no any way to stop WorkLock, so I can't see how to avoid deposit. Re Threshold: all these "new" tokens are in WorkLock contract and they should be part of currentPeriodSupply. For me it looks like we already consider them as deposited in StakingEscrow
There was a problem hiding this comment.
all these "new" tokens are in WorkLock contract and they should be part of currentPeriodSupply. For me it looks like we already consider them as deposited in StakingEscrow
OK, this is a good reasoning.
There was a problem hiding this comment.
No bids after Sep 28th 2020 (https://dune.xyz/queries/271579) and only one deposit on Aug 30th 2020 (https://dune.xyz/queries/271593). Would the only implication be that someone loses money for (inexplicably) sending in ETH and there's no longer away to call worktoETH?
There was a problem hiding this comment.
I don't understand the question. Both events is only for initial worklock phase which was more than year ago
There was a problem hiding this comment.
Yep, that's what I was getting at with my question – what exactly is the undesirable corner case where someone makes a deposit post initial worklock phase? Has that occurred in the last 13 months?
There was a problem hiding this comment.
This deposit for WorkLock, not for StakingEscrow
Co-authored-by: David Núñez <david@nucypher.com>
There was a problem hiding this comment.
@vzotova - I assume the failing tests will be dealt with separately as part of the overall EPIC...?
newsfragments/2822.feature.rst
Outdated
| @@ -0,0 +1 @@ | |||
| Halting NU inflation, now refund in WorkLock is possible without work (claim stil needed) | |||
There was a problem hiding this comment.
| Halting NU inflation, now refund in WorkLock is possible without work (claim stil needed) | |
| Halting NU inflation; now refund in WorkLock is possible without work (claim still needed) |
Co-authored-by: David Núñez <david@nucypher.com> Co-authored-by: derekpierre <derek.pierre@gmail.com>
vzotova
force-pushed
the
inflation-stop
branch
from
60ad083
to
a47030b
Compare
Yeah, it's hard to fix those tests before PREApp contract will be ready (at least) |
| { | ||
| } | ||
|
|
||
| function depositAsStaker(uint256 _value, uint16 _periods) public onlyDelegateCall { |
There was a problem hiding this comment.
Are these tests for deposit/withdraw methods that will be used post-merge in some way? How is this related to the staking adaptor contract set?
There was a problem hiding this comment.
These lines to minimize changes in tests right now
Co-authored-by: David Núñez <david@nucypher.com> Co-authored-by: derekpierre <derek.pierre@gmail.com>
Co-authored-by: David Núñez <david@nucypher.com> Co-authored-by: derekpierre <derek.pierre@gmail.com>
Co-authored-by: David Núñez <david@nucypher.com> Co-authored-by: derekpierre <derek.pierre@gmail.com>
Type of PR:
Required reviews:
What this does:
StakingEscrowupdate only. Removes most of the code fromStakingEscrow: halting inflation, removing all methods to control stake (including withdraw), excludePolicyManagerandAdjudicatorfrom lifecycle (for now)Why it's needed:
First step in merging networks
Notes for reviewers:
Only tests for contracts have been adjusted. Some code for
PolicyMangerandAdjudicatorwas not removed to use it together withPREApplicationcontract later