Skip to content
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.

Sign up for GitHub

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

Jump to bottom

First step to remove legacy #153

Merged
merged 6 commits into from
Nov 20, 2023
Merged

First step to remove legacy #153

merged 6 commits into from
Nov 20, 2023

Conversation

vzotova
Copy link
Contributor

@vzotova vzotova commented Sep 19, 2023 •
edited
Loading

Removes most of legacy support: staking Keep/Nu, top-up Keep/Nu, increasing authorization to more than T stake.
Also adds method to deauthorize+unstake legacy stakes in one tx using involuntary decrease call. This method is permissionless.
Description of how this all would work can be found here #141. Additional to that - forceUnstakeLegacy method.
Second step will be removing forceUnstakeLegacy and all other usage of legacy stakes in TokenStaking after unstaking all legacy tokens.
P.S. stakedNu used in NuCypher StakingEscrow, for compatibility and will be removed at step 2 (together with link to Nu vending machine)

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6240448509 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6240454444 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6240532295 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6240566891 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6241090131 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6241411525 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6241675809 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6266194951 check.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6267918668 check.

@vzotova vzotova changed the title [WIP] First step to remove legacy First step to remove legacy Sep 22, 2023
@vzotova vzotova marked this pull request as ready for review September 22, 2023 17:18
@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6343895490 check.

derekpierre
derekpierre reviewed Sep 29, 2023
View reviewed changes
Copy link
Member

@derekpierre derekpierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to do legacy removal as a potential EPIC branch instead of stages going into main? Or are there merge conflicts concerns of such a wide-reaching changes being in a separate branch for too long?

contracts/staking/TokenStaking.sol Outdated Show resolved Hide resolved
contracts/staking/IStaking.sol Outdated Show resolved Hide resolved
@vzotova
Copy link
Contributor Author

vzotova commented Sep 29, 2023

Would we want to do legacy removal as a potential EPIC branch instead of stages going into main? Or are there merge conflicts concerns of such a wide-reaching changes being in a separate branch for too long?

It's not really an EPIC, because next PR will be done only after upgrading code to this one and unstake all legacy.

@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6353000229 check.

@vzotova vzotova mentioned this pull request Sep 29, 2023
Merged
cygnusv
cygnusv reviewed Oct 24, 2023
View reviewed changes
Copy link
Member

@cygnusv cygnusv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass

contracts/staking/IStaking.sol
Comment on lines -67 to -71
function stakeNu(
address stakingProvider,
address payable beneficiary,
address authorizer
) external;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪓

contracts/staking/IStaking.sol Outdated Show resolved Hide resolved
contracts/staking/TokenStaking.sol
// slither-disable-next-line constable-states
uint96 private legacyStakeDiscrepancyPenalty;
// slither-disable-next-line constable-states
uint256 private legacyStakeDiscrepancyRewardMultiplier;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about removing these variables? Taking into consideration, of course, that they currently occupy a slot in the proxy storage layout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is whole point to have them - to not break slot layout for upgrades

contracts/staking/TokenStaking.sol
address indexed stakingProvider,
address indexed oldOwner,
address indexed newOwner
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels refreshing

contracts/staking/TokenStaking.sol
VendingMachine _nucypherVendingMachine,
KeepStake _keepStake
) {
constructor(T _token, VendingMachine _nucypherVendingMachine) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So clean!

contracts/staking/TokenStaking.sol Outdated Show resolved Hide resolved
docs/rfc-1-staking-contract.adoc Outdated Show resolved Hide resolved
docs/rfc-1-staking-contract.adoc Outdated Show resolved Hide resolved
docs/rfc-1-staking-contract.adoc Outdated Show resolved Hide resolved
docs/rfc-1-staking-contract.adoc Outdated Show resolved Hide resolved
@github-actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6632026461 check.

@cygnusv cygnusv mentioned this pull request Nov 8, 2023
Closed
lukasz-zimnoch
lukasz-zimnoch reviewed Nov 9, 2023
View reviewed changes
contracts/staking/TokenStaking.sol Show resolved Hide resolved
contracts/staking/TokenStaking.sol Show resolved Hide resolved
docs/rfc-1-staking-contract.adoc Outdated Show resolved Hide resolved
contracts/staking/TokenStaking.sol Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 9, 2023

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6813151919 check.

@github-actions GitHub Actions
Copy link

Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6864773014 check.

@cygnusv cygnusv merged commit 4b569b2 into main Nov 20, 2023
11 checks passed
@cygnusv cygnusv deleted the remove-legacy-step-1 branch November 20, 2023 10:16
cygnusv added a commit that referenced this pull request Nov 24, 2023
@cygnusv
Update TokenStaking upgrade verification script
1861cf7 
After #153, which drop support for legacy stakers, TokenStaking constructor requires less arguments
@vzotova vzotova mentioned this pull request Mar 23, 2024
Closed
cygnusv added a commit that referenced this pull request Sep 9, 2024
@cygnusv
Update TokenStaking upgrade verification script
1f6493b 
After #153, which drop support for legacy stakers, TokenStaking constructor requires less arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekpierre derekpierre derekpierre left review comments

@cygnusv cygnusv cygnusv approved these changes

@lukasz-zimnoch lukasz-zimnoch lukasz-zimnoch approved these changes

@pdyraga pdyraga Awaiting requested review from pdyraga pdyraga is a code owner

@arjunhassard arjunhassard Awaiting requested review from arjunhassard

@mswilkison mswilkison Awaiting requested review from mswilkison

@theref theref Awaiting requested review from theref

@nkuba nkuba Awaiting requested review from nkuba

@michalinacienciala michalinacienciala Awaiting requested review from michalinacienciala

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vzotova @cygnusv @lukasz-zimnoch @derekpierre