-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
[sui-framework] Remove "delegate" and replace with stake
#9059
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
the governance api PR #8848 has some of the same renaming changes on the RPC layer, SDK and the FE apps, this will create quite a bit of conflicts, can we put this on hold 🙏? |
/// Add delegated stake to a validator's staking pool using multiple coins. | ||
public entry fun request_add_delegation_mul_coin( | ||
/// Add stake to a validator's staking pool using multiple coins. | ||
public entry fun request_add_stake_mul_coin( |
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.
We should really just remove this method.
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.
To keep things clean I might leave this in for now so this is a strict renaming pass, but will do a fast follow to remove this.
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.
But I believe this function is used in our sui-transaction-builder
as well as sui-rosetta
:
sui/crates/sui-transaction-builder/src/lib.rs
Line 663 in ffc34df
ADD_DELEGATION_MUL_COIN_FUN_NAME |
cc @patrickkuo
6147162
to
f48e225
Compare
f48e225
to
90fa9f0
Compare
The plan is to wait for #9058 to land, and once it does this PR will be rebased on that and then landed. In the meantime please review and let me know if there is anywhere I have missed or that you think an error was made. |
90fa9f0
to
e3c6945
Compare
e3c6945
to
1176fa1
Compare
1176fa1
to
0153f2d
Compare
0153f2d
to
c1aa325
Compare
@@ -578,7 +578,7 @@ module sui::sui_system { | |||
/// 1. Add storage charge to the storage fund. | |||
/// 2. Burn the storage rebates from the storage fund. These are already refunded to transaction sender's | |||
/// gas coins. | |||
/// 3. Distribute computation charge to validator stake and delegation stake. | |||
/// 3. Distribute computation charge to validator stake and stake stake. |
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.
stake stake 🤣 but let's land the PR first and fix this later
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.
Changed this at the same time as I was reverting the safe_tests
file.
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.
We need to revert the changes to one file but other than that LGTM, approving to unblock. Thank you so much! @tzakian
c1aa325
to
4a8c6e5
Compare
This does a large rename of "delegation/delegate" (etc) to the equivalent "stake" name, e.g.,
request_add_delegation
becomesrequest_add_stake
.Move
Entry functions renamed (all within
sui_system.move
):Move Object fields
Sui types
This renames a couple sui types since they no longer made sense to be called what they were:
This then performs the core set of renamings to make the system happy with this. So there are still a number of functions in Rust and Typescript that will contain their old delegation name.
Note that on the typescript side we keep DelegatedStake since (1) there was already a type called Stake (which meant something else) and in the context that I could see it being used it appeared as though DelegatedStake really did mean "this is my stake that I have delegated to this validator."
Test Plan
Make sure existing tests still pass.
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes