[jsonrpc] - update governance api #8848
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
patrickkuo
force-pushed
the
pat/staking_api
branch
from
4fd57e8 to
085af1a
Compare
crates/sui-core/src/authority.rs
Outdated
| @@ -1952,6 +1953,20 @@ impl AuthorityState { | |||
| } | |||
| } | |||
|
|
|||
| pub async fn read_table<K, V>(&self, table: &Table) -> SuiResult<BTreeMap<K, V>> | |||
There was a problem hiding this comment.
We will need a bound on the number of results?
There was a problem hiding this comment.
I have changed read_table to read_table_value, which use a iterator to find the value instead of reading the whole table in mem.
| #[derive(Clone, Serialize, Deserialize, JsonSchema, Debug)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct DynamicFieldInfo { | ||
| pub name: DynamicFieldName, | ||
| #[schemars(with = "Base58")] | ||
| #[serde_as(as = "Readable<Base58, _>")] | ||
| pub bcs_name: Vec<u8>, |
There was a problem hiding this comment.
n00b q: what is this used for?
There was a problem hiding this comment.
this is the name of the dynamic field in bcs, DynamicFieldName is the parsed json version of this.
This is added so we can use it in get_table_value to get the bcs name without reading the Field object.
sui/crates/sui-core/src/authority.rs
Lines 1962 to 1974 in 417d448
This will also be useful for client with bcs support where they can use the bcs bytes instead of the parsed json dynamic field name.
| .get_move_objects(owner, &Delegation::type_()) | ||
| .await?) | ||
|
|
||
| async fn get_delegated_stakes(&self, owner: SuiAddress) -> Result<Vec<DelegatedStake>, Error> { |
There was a problem hiding this comment.
I am trying to understand the delta between the previous return type vs the current return type:
- Previously the return type was "flat": a list of
DelegatedStakewhere there could be multipleDelegatedStakefor the same validator. - This PR aggregates the result by the validator address. It still returns a list of
DelegatedStake, but there will only be exactly oneDelegatedStakefor the same validator. EachDelegatedStakenow contains a list ofDelegationobjects that share the same validator.
This change makes it a bit involved to change on the wallet side since there are a lot of places in the wallet code that has strong assumptions(example) on this and it would take us some time to fully understand the wallet logic and make the right changes.
cc @Jibz-Mysten @Jordan-Mysten it would be great if we could get some help from the FE team to advise on these changes. Thanks!
There was a problem hiding this comment.
@666lcz I think these changes are fine. I am working on staking changes this week, I can make this update on the wallet site.
|
One thing worth discussing is how do we plan to evolve all these types. |
improve get_sui_system_state json response
* change read_table to read_table_value to avoid reading the whole table in memory
## Description Describe the changes or additions included in this PR. ## Test Plan How did you test the new or updated feature? --- 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) - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes --------- Co-authored-by: patrick <[email protected]> Co-authored-by: Chris Li <[email protected]>
patrickkuo
force-pushed
the
pat/staking_api
branch
from
18618f8 to
53b8085
Compare
| @@ -356,6 +356,19 @@ pub struct StakingPoolV1 { | |||
| pub pending_pool_token_withdraw: u64, | |||
| } | |||
|
|
|||
| #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)] | |||
| pub struct PoolTokenExchangeRate { | |||
There was a problem hiding this comment.
This is no longer used, right?
| let inactive_validators = system_state.inactive_pools_id; | ||
| let inactive_validators = self | ||
| .state | ||
| .read_table_value::<ObjectID, ValidatorV1>(inactive_validators, pool_id) |
There was a problem hiding this comment.
This is not going to work once we have a new version for the Validator type.
There are two ways to go about this:
- Rely on SuiSystemStateSummary exclusively.
- Add a trait function to SuiSystemStateTrait to get some basic info to construct this, but then be mindful that that API will have to be respected for all future versions
Description
ValidatorMetadatareadable json response, which affects getSuiSystemState as well.sui genesisfor configuring localnet epoch durationTest Plan
Running local network to confirm staking json response
sui_getDelegatedStake response
sui_getLatestSystemState response