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.

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

Deprecate delegate in sol #2333

Merged
merged 4 commits into from
Jun 28, 2023
Merged

Deprecate delegate in sol #2333

merged 4 commits into from
Jun 28, 2023

Conversation

nbaztec
Copy link
Contributor

@nbaztec nbaztec commented Jun 7, 2023

What does it do?

deprecate delegate in solidity contract

⚠️ Breaking Changes ⚠️

❗ Deprecate delegate

Deprecates the delegate call in favor of delegateWithAutoCompound in the precompile.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@nbaztec nbaztec added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels Jun 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

Coverage generated "Mon Jun 26 15:41:24 UTC 2023":
https://s3.amazonaws.com/moonbeam-coverage/pulls/2333/html/index.html

Master coverage: 70.94%
Pull coverage: 70.94%

@@ -212,6 +206,7 @@ interface ParachainStaking {
/// @custom:selector b5ad5f07
function cancelCandidateBondLess() external;

/// @notice DEPRECATED use delegateWithAutoCompound instead
Copy link
Contributor

Choose a reason for hiding this comment

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

We could be more verbose and explain that we intend to remove this in the future, maybe also that it's more expensive (is that true?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is more expensive, and this method is a superset of delegate. I'll add that

Copy link
Contributor

@notlesh notlesh left a comment

Choose a reason for hiding this comment

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

I would make sure the whitespace changes are needed, but otherwise looks good

@nbaztec
Copy link
Contributor Author

nbaztec commented Jun 21, 2023

I would make sure the whitespace changes are needed, but otherwise looks good

@notlesh I was also surprised about it. Turns out that it was me who originally committed that file using the autoformat via the solidity extension suggested by Alan, seems that the same extension now formats things differently.

@nbaztec nbaztec merged commit eb434bb into master Jun 28, 2023
27 checks passed
@nbaztec nbaztec deleted the nish-deprecate-delegate-1 branch June 28, 2023 07:43
@noandrea noandrea changed the title deprecate delegate in sol Deprecate delegate in sol Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants