-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/pooled staking integration #10
Conversation
contracts/ClaimsReward.sol
Outdated
tf.unlockStakerUnlockableTokens(msg.sender); | ||
|
||
pooledStaking.withdrawReward(pooledStaking.stakerReward(msg.sender)); | ||
pooledStaking.unstake(pooledStaking.getMaxUnstakable(msg.sender)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this, so we can keep this function only for claiming rewards (as the name says)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so basically we change the semantics of this. the button in the UI won't give you all things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed. the flows are pretty different from a user / UX point of view:
-> withdraw all rewards will be served by this function
-> withdraw tokens you staked for claim assessment, so you can use them in voting again or for something else
-> withdraw stake (unstake) tokens from pooled staking
function pushBurn(address contractAddress, uint amount) external; | ||
|
||
function contractStakedAmount(address contractAddress) external view returns (uint); | ||
function stakerReward(address staker) external view returns (uint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these 2 getters don't exist yet @shark0der need smth like this or any other name you see a good fit
contracts/TokenFunctions.sol
Outdated
@@ -182,17 +111,7 @@ contract TokenFunctions is Iupgradable { | |||
* @param _stakerAddress address of the Staker. | |||
*/ | |||
function getStakerAllLockedTokens(address _stakerAddress) external view returns (uint amount) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find this function being used anywhere but the UI. If that's the case it'd be worth deprecating
contracts/TokenFunctions.sol
Outdated
@@ -204,19 +123,9 @@ contract TokenFunctions is Iupgradable { | |||
) | |||
external | |||
view | |||
returns (uint amount) | |||
returns (uint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like it's only been used in line 227 of ClaimRewards.sol, but you replaced it. if that's the case, it can be deprecated
contracts/ClaimsReward.sol
Outdated
uint governanceReward = gv.getPendingReward(_add); | ||
total = caReward.add(unlockableStakedTokens).add(commissionEarned. | ||
sub(commissionReedmed)).add(governanceReward); | ||
total = caReward.add(stakedTokens).add(pooledStakingReward).add(governanceReward); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth adding on this branch the updates from https://github.com/NexusMutual/smart-contract-upgrades/tree/fix-issue-181-unlockBug. Looks like this is the only place where it impact you, there is an extra line missing (uint unlockableStakedTokens = tf.getStakerAllUnlockableStakedTokens(_add);
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can assume they will be on master when we deploy/merge this
7c1a59a
to
f3067c3
Compare
No description provided.