From a55db3739ebe8f1ab36781762c474b1d4cdf59a0 Mon Sep 17 00:00:00 2001 From: Emma Zhong Date: Tue, 5 Nov 2024 16:13:13 -0800 Subject: [PATCH 1/4] [jsonrpc] fix estimated rewards during safe mode (#20182) ## Description Currently if the exchange rate for an epoch is not found, we would assign it the default exchange rate while we should've used the exchange rate from the previous existing epoch instead. This PR fixes that. ## Test plan Added some unit tests. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [x] JSON-RPC: fixed reward estimation in `getStakes` API so that we don't overestimate stake rewards of stakes activated during safe mode epochs. - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API: --- crates/sui-json-rpc/src/governance_api.rs | 77 +++++++++++++++++++- crates/sui-types/src/sui_system_state/mod.rs | 7 ++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/crates/sui-json-rpc/src/governance_api.rs b/crates/sui-json-rpc/src/governance_api.rs index a8a23a41952ed..f08d4366264ce 100644 --- a/crates/sui-json-rpc/src/governance_api.rs +++ b/crates/sui-json-rpc/src/governance_api.rs @@ -166,6 +166,7 @@ impl GovernanceReadApi { let status = if !exists { StakeStatus::Unstaked } else if system_state_summary.epoch >= stake.activation_epoch() { + // TODO: use dev_inspect to call a move function to get the estimated reward let estimated_reward = if let Some(current_rate) = current_rate { let stake_rate = rate_table .rates @@ -431,7 +432,8 @@ async fn exchange_rates( }) .collect::, _>>()?; - rates.sort_by(|(a, _), (b, _)| a.cmp(b).reverse()); + // Rates for some epochs might be missing due to safe mode, we need to backfill them. + rates = backfill_rates(rates); exchange_rates.push(ValidatorExchangeRates { address, @@ -451,6 +453,38 @@ pub struct ValidatorExchangeRates { pub rates: Vec<(EpochId, PoolTokenExchangeRate)>, } +/// Backfill missing rates for some epochs due to safe mode. If a rate is missing for epoch e, +/// we will use the rate for epoch e-1 to fill it. +/// Rates returned are in descending order by epoch. +fn backfill_rates( + rates: Vec<(EpochId, PoolTokenExchangeRate)>, +) -> Vec<(EpochId, PoolTokenExchangeRate)> { + if rates.is_empty() { + return rates; + } + + let min_epoch = *rates.iter().map(|(e, _)| e).min().unwrap(); + let max_epoch = *rates.iter().map(|(e, _)| e).max().unwrap(); + let mut filled_rates = Vec::new(); + let mut prev_rate = None; + + for epoch in min_epoch..=max_epoch { + match rates.iter().find(|(e, _)| *e == epoch) { + Some((e, rate)) => { + prev_rate = Some(rate.clone()); + filled_rates.push((*e, rate.clone())); + } + None => { + if let Some(rate) = prev_rate.clone() { + filled_rates.push((epoch, rate)); + } + } + } + } + filled_rates.reverse(); + filled_rates +} + impl SuiRpcModule for GovernanceReadApi { fn rpc(self) -> RpcModule { self.into_rpc() @@ -460,3 +494,44 @@ impl SuiRpcModule for GovernanceReadApi { GovernanceReadApiOpenRpc::module_doc() } } + +#[cfg(test)] +mod tests { + use super::*; + use sui_types::sui_system_state::PoolTokenExchangeRate; + + #[test] + fn test_backfill_rates_empty() { + let rates = vec![]; + assert_eq!(backfill_rates(rates), vec![]); + } + + #[test] + fn test_backfill_rates_no_gaps() { + let rate1 = PoolTokenExchangeRate::new(100, 100); + let rate2 = PoolTokenExchangeRate::new(200, 220); + let rate3 = PoolTokenExchangeRate::new(300, 330); + let rates = vec![(2, rate2.clone()), (3, rate3.clone()), (1, rate1.clone())]; + + let expected: Vec<(u64, PoolTokenExchangeRate)> = + vec![(3, rate3.clone()), (2, rate2), (1, rate1)]; + assert_eq!(backfill_rates(rates), expected); + } + + #[test] + fn test_backfill_rates_with_gaps() { + let rate1 = PoolTokenExchangeRate::new(100, 100); + let rate3 = PoolTokenExchangeRate::new(300, 330); + let rate5 = PoolTokenExchangeRate::new(500, 550); + let rates = vec![(3, rate3.clone()), (1, rate1.clone()), (5, rate5.clone())]; + + let expected = vec![ + (5, rate5.clone()), + (4, rate3.clone()), + (3, rate3.clone()), + (2, rate1.clone()), + (1, rate1), + ]; + assert_eq!(backfill_rates(rates), expected); + } +} diff --git a/crates/sui-types/src/sui_system_state/mod.rs b/crates/sui-types/src/sui_system_state/mod.rs index d647e7da495b6..7f91dbd292e98 100644 --- a/crates/sui-types/src/sui_system_state/mod.rs +++ b/crates/sui-types/src/sui_system_state/mod.rs @@ -416,6 +416,13 @@ impl PoolTokenExchangeRate { self.pool_token_amount as f64 / self.sui_amount as f64 } } + + pub fn new(sui_amount: u64, pool_token_amount: u64) -> Self { + Self { + sui_amount, + pool_token_amount, + } + } } #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)] From e5bf194fa627b308182bcbec4e8faf33081081dc Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Mon, 11 Nov 2024 14:37:41 +0000 Subject: [PATCH 2/4] chore(deny): allowlist advisories Waiting on dependent packages to update. --- deny.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deny.toml b/deny.toml index b396cb4eac121..6b358daff5eb3 100644 --- a/deny.toml +++ b/deny.toml @@ -39,6 +39,10 @@ ignore = [ # Temporarily allow until arrow family of crates updates `lexical-core` to 1.0 "RUSTSEC-2023-0086", + # allow unmaintained instant crate used in transitive dependencies (backoff, cached, fastrand, parking_lot_*) + "RUSTSEC-2024-0384", + # allow unmaintained derivative crate used in transitive dependencies (ark-*) + "RUSTSEC-2024-0388", ] # Threshold for security vulnerabilities, any vulnerability with a CVSS score # lower than the range specified will be ignored. Note that ignored advisories From 73dfdef410042fc5009f35fe6b859144df8ef78a Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Mon, 11 Nov 2024 16:50:32 +0000 Subject: [PATCH 3/4] chore(deny): Remove dependency on opentelemetry_api ## Description We are not using this dependency and `cargo deny` includes an advisory that it has been rolled into `opentelemetry`, which this change silences. ## Test plan CI --- Cargo.lock | 17 ----------------- crates/telemetry-subscribers/Cargo.toml | 2 -- 2 files changed, 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bee6edbbeff7e..b42ea83c46acc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9202,22 +9202,6 @@ dependencies = [ "tonic 0.12.3", ] -[[package]] -name = "opentelemetry_api" -version = "0.20.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a81f725323db1b1206ca3da8bb19874bbd3f57c3bcd59471bfb04525b265b9b" -dependencies = [ - "futures-channel", - "futures-util", - "indexmap 1.9.3", - "js-sys", - "once_cell", - "pin-project-lite", - "thiserror", - "urlencoding", -] - [[package]] name = "opentelemetry_sdk" version = "0.25.0" @@ -15956,7 +15940,6 @@ dependencies = [ "opentelemetry 0.25.0", "opentelemetry-otlp", "opentelemetry-proto", - "opentelemetry_api", "opentelemetry_sdk", "prometheus", "prost 0.13.3", diff --git a/crates/telemetry-subscribers/Cargo.toml b/crates/telemetry-subscribers/Cargo.toml index 1ed249129edac..452ea1f2b7a4d 100644 --- a/crates/telemetry-subscribers/Cargo.toml +++ b/crates/telemetry-subscribers/Cargo.toml @@ -18,7 +18,6 @@ tracing.workspace = true tracing-appender.workspace = true tracing-subscriber.workspace = true opentelemetry = { version = "0.25.0", optional = true } -opentelemetry_api = { version = "0.20.0", optional = true } opentelemetry_sdk = { version = "0.25.0", features = ["rt-tokio"], optional = true } opentelemetry-otlp = { version = "0.25.0", features = ["grpc-tonic"], optional = true } tracing-opentelemetry = { version = "0.26.0", optional = true } @@ -42,7 +41,6 @@ otlp = [ "opentelemetry", "opentelemetry-otlp", "opentelemetry-proto", - "opentelemetry_api", "opentelemetry_sdk" ] From ad814570f7e25ae554b498c4e5c8a882c197ad3d Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Mon, 11 Nov 2024 16:31:52 +0000 Subject: [PATCH 4/4] chore(deny): remove direct dep on derivative ## Description `derivative` is unmaintained. Switch existing uses to use `derive_more` instead, which is an existing dependency that has all the necessary features. This also required bumping `derive_more` to `1.0.0`, which introduced some small breaking changes, and means that we are pulling in two versions of `derive_more` due to transitive dependency constraints, but in this case, it should be okay because we are mainly using this dep for its proc macros, and one hopes that eventually the transitive deps will upgrade to `1.x` as well. ## Test plan CI --- Cargo.lock | 32 +++++++++++++++---- Cargo.toml | 3 +- crates/sui-cluster-test/Cargo.toml | 2 +- crates/sui-cluster-test/src/config.rs | 27 +++++----------- crates/sui-types/Cargo.toml | 3 +- crates/sui-types/src/move_package.rs | 9 +++--- .../sui_system_state_inner_v1.rs | 5 ++- 7 files changed, 43 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b42ea83c46acc..c6d6177a4c9d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3722,6 +3722,27 @@ dependencies = [ "syn 1.0.107", ] +[[package]] +name = "derive_more" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a9b99b9cbbe49445b21764dc0625032a89b145a2642e67603e1c936f5458d05" +dependencies = [ + "derive_more-impl", +] + +[[package]] +name = "derive_more-impl" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22" +dependencies = [ + "proc-macro2 1.0.87", + "quote 1.0.35", + "syn 2.0.79", + "unicode-xid 0.2.4", +] + [[package]] name = "determinator" version = "0.12.0" @@ -4758,7 +4779,7 @@ dependencies = [ "cbc", "ctr", "curve25519-dalek-ng", - "derive_more", + "derive_more 0.99.17", "digest 0.10.7", "ecdsa 0.16.9", "ed25519-consensus", @@ -4850,7 +4871,7 @@ dependencies = [ "ark-snark", "bcs", "byte-slice-cast", - "derive_more", + "derive_more 0.99.17", "fastcrypto", "ff 0.13.0", "im", @@ -11553,7 +11574,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f7d66a1128282b7ef025a8ead62a4a9fcf017382ec53b8ffbf4d7bf77bd3c60" dependencies = [ "cfg-if", - "derive_more", + "derive_more 0.99.17", "parity-scale-codec 3.6.5", "scale-info-derive", ] @@ -13166,7 +13187,7 @@ dependencies = [ "anyhow", "async-trait", "clap", - "derivative", + "derive_more 1.0.0", "fastcrypto", "futures", "jsonrpsee", @@ -15485,8 +15506,7 @@ dependencies = [ "consensus-config", "coset", "criterion", - "derivative", - "derive_more", + "derive_more 1.0.0", "enum_dispatch", "expect-test", "eyre", diff --git a/Cargo.toml b/Cargo.toml index 190202c9cf6a2..a63ef7035cc49 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -328,10 +328,9 @@ cynic-codegen = "= 3.7.3" dashmap = "5.5.3" # datatest-stable = "0.1.2" datatest-stable = { git = "https://github.com/nextest-rs/datatest-stable.git", rev = "72db7f6d1bbe36a5407e96b9488a581f763e106f" } -derivative = "2.2.0" derive-syn-parse = "0.1.5" derive_builder = "0.12.0" -derive_more = "0.99.17" +derive_more = "1.0.0" diesel = "2.2" diesel_migrations = "2.2" diesel-async = "0.5" diff --git a/crates/sui-cluster-test/Cargo.toml b/crates/sui-cluster-test/Cargo.toml index 93bf428007108..8482c6c4d6f48 100644 --- a/crates/sui-cluster-test/Cargo.toml +++ b/crates/sui-cluster-test/Cargo.toml @@ -22,7 +22,7 @@ uuid.workspace = true prometheus.workspace = true fastcrypto.workspace = true shared-crypto.workspace = true -derivative.workspace = true +derive_more = { workspace = true, features = ["debug"] } regex.workspace = true sui-indexer.workspace = true diff --git a/crates/sui-cluster-test/src/config.rs b/crates/sui-cluster-test/src/config.rs index 9df8facf131d3..a8a4d91e3afd2 100644 --- a/crates/sui-cluster-test/src/config.rs +++ b/crates/sui-cluster-test/src/config.rs @@ -3,7 +3,7 @@ use clap::*; use regex::Regex; -use std::{fmt, path::PathBuf}; +use std::path::PathBuf; #[derive(Parser, Clone, ValueEnum, Debug)] pub enum Env { @@ -16,8 +16,7 @@ pub enum Env { NewLocal, } -#[derive(derivative::Derivative, Parser)] -#[derivative(Debug)] +#[derive(derive_more::Debug, Parser)] #[clap(name = "", rename_all = "kebab-case")] pub struct ClusterTestOpt { #[clap(value_enum)] @@ -33,7 +32,12 @@ pub struct ClusterTestOpt { pub indexer_address: Option, /// URL for the Indexer Postgres DB #[clap(long)] - #[derivative(Debug(format_with = "obfuscated_pg_address"))] + #[debug("{}", match pg_address { + None => "None".into(), + Some(val) => Regex::new(r":.*@") + .unwrap() + .replace_all(val.as_str(), ":*****@"), + })] pub pg_address: Option, #[clap(long)] pub config_dir: Option, @@ -47,21 +51,6 @@ pub struct ClusterTestOpt { pub with_indexer_and_graphql: bool, } -fn obfuscated_pg_address(val: &Option, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match val { - None => write!(f, "None"), - Some(val) => { - write!( - f, - "{}", - Regex::new(r":.*@") - .unwrap() - .replace_all(val.as_str(), ":*****@") - ) - } - } -} - impl ClusterTestOpt { pub fn new_local() -> Self { Self { diff --git a/crates/sui-types/Cargo.toml b/crates/sui-types/Cargo.toml index fcb060ce2bd7c..4b29e276ec05e 100644 --- a/crates/sui-types/Cargo.toml +++ b/crates/sui-types/Cargo.toml @@ -42,7 +42,6 @@ roaring.workspace = true enum_dispatch.workspace = true eyre.workspace = true indexmap.workspace = true -derivative.workspace = true jsonrpsee.workspace = true move-binary-format.workspace = true move-bytecode-utils.workspace = true @@ -70,7 +69,7 @@ fastcrypto-zkp.workspace = true passkey-types.workspace = true typed-store-error.workspace = true -derive_more.workspace = true +derive_more = { workspace = true, features = ["as_ref", "debug", "display", "from"] } proptest.workspace = true proptest-derive.workspace = true better_any.workspace = true diff --git a/crates/sui-types/src/move_package.rs b/crates/sui-types/src/move_package.rs index 8b12643e4ec16..06bd929471916 100644 --- a/crates/sui-types/src/move_package.rs +++ b/crates/sui-types/src/move_package.rs @@ -10,7 +10,6 @@ use crate::{ object::OBJECT_START_VERSION, SUI_FRAMEWORK_ADDRESS, }; -use derive_more::Display; use fastcrypto::hash::HashFunction; use move_binary_format::binary_config::BinaryConfig; use move_binary_format::file_format::CompiledModule; @@ -114,13 +113,13 @@ pub struct MovePackage { // associated constants before storing in any serialization setting. /// Rust representation of upgrade policy constants in `sui::package`. #[repr(u8)] -#[derive(Display, Debug, Clone, Copy)] +#[derive(derive_more::Display, Debug, Clone, Copy)] pub enum UpgradePolicy { - #[display(fmt = "COMPATIBLE")] + #[display("COMPATIBLE")] Compatible = 0, - #[display(fmt = "ADDITIVE")] + #[display("ADDITIVE")] Additive = 128, - #[display(fmt = "DEP_ONLY")] + #[display("DEP_ONLY")] DepOnly = 192, } diff --git a/crates/sui-types/src/sui_system_state/sui_system_state_inner_v1.rs b/crates/sui-types/src/sui_system_state/sui_system_state_inner_v1.rs index c759b9254490e..c1cd3056388b5 100644 --- a/crates/sui-types/src/sui_system_state/sui_system_state_inner_v1.rs +++ b/crates/sui-types/src/sui_system_state/sui_system_state_inner_v1.rs @@ -88,14 +88,13 @@ pub struct ValidatorMetadataV1 { pub extra_fields: Bag, } -#[derive(derivative::Derivative, Clone, Eq, PartialEq)] -#[derivative(Debug)] +#[derive(derive_more::Debug, Clone, Eq, PartialEq)] pub struct VerifiedValidatorMetadataV1 { pub sui_address: SuiAddress, pub protocol_pubkey: AuthorityPublicKey, pub network_pubkey: NetworkPublicKey, pub worker_pubkey: NetworkPublicKey, - #[derivative(Debug = "ignore")] + #[debug(ignore)] pub proof_of_possession_bytes: Vec, pub name: String, pub description: String,