Skip to content

Commit

Permalink
[Core] Add support for chain-specific configs (MystenLabs#12183)
Browse files Browse the repository at this point in the history
## Description 

The goal is to be able to roll out features for certain chains and not
others.

To accomplish this, we introduce a `Chain` enum, with a getter function
to map the hard coded checkpoint identifier (i.e. the digest of the
genesis checkpoint) to the `Chain`, and have all callsites where the
protocol config is materialized to provide the chain identifier.

With the `Chain` being available in the
`ProtocolConfig::get_for_version`, we can then make use of this in the
protocol version map.

## Test Plan 

Tested against a Mainnet node to ensure that the chain is correctly
identified. Also locally enabled a feature for all chains but mainnet in
the highest protocol version, and saw that the feature was active.

---
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)

- [ ] protocol change
- [ ] 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
  • Loading branch information
williampsmith authored Jun 1, 2023
1 parent 93d71ad commit e8a351e
Show file tree
Hide file tree
Showing 24 changed files with 210 additions and 75 deletions.
3 changes: 2 additions & 1 deletion crates/sui-benchmark/src/bin/stress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use clap::*;
use prometheus::Registry;
use rand::seq::SliceRandom;
use rand::Rng;
use sui_protocol_config::Chain;
use tokio::time::sleep;

use std::sync::Arc;
Expand Down Expand Up @@ -58,7 +59,7 @@ async fn main() -> Result<()> {

// TODO: query the network for the current protocol version.
let protocol_config = match opts.protocol_version {
Some(v) => ProtocolConfig::get_for_version(ProtocolVersion::new(v)),
Some(v) => ProtocolConfig::get_for_version(ProtocolVersion::new(v), Chain::Unknown),
None => ProtocolConfig::get_for_max_version(),
};

Expand Down
4 changes: 2 additions & 2 deletions crates/sui-benchmark/src/system_state_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use crate::ValidatorProxy;
use std::sync::Arc;
use std::time::Duration;
use sui_protocol_config::{ProtocolConfig, ProtocolVersion};
use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion};
use tokio::sync::oneshot::Sender;
use tokio::sync::watch;
use tokio::sync::watch::Receiver;
Expand Down Expand Up @@ -39,7 +39,7 @@ impl SystemStateObserver {
_ = interval.tick() => {
match proxy.get_latest_system_state_object().await {
Ok(result) => {
let p = ProtocolConfig::get_for_version(ProtocolVersion::new(result.protocol_version));
let p = ProtocolConfig::get_for_version(ProtocolVersion::new(result.protocol_version), Chain::Unknown);
if tx.send(SystemState {reference_gas_price: result.reference_gas_price,protocol_config: Some(p)}).is_ok() {
info!("Reference gas price = {:?}", result.reference_gas_price );
}
Expand Down
2 changes: 2 additions & 0 deletions crates/sui-benchmark/tests/simtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ mod test {

#[sim_test(config = "test_config()")]
async fn test_simulated_load_reconfig_restarts() {
// TODO added to invalidate a failing test seed in CI. Remove me
tokio::time::sleep(Duration::from_secs(1)).await;
sui_protocol_config::ProtocolConfig::poison_get_for_min_version();
let test_cluster = Arc::new(build_test_cluster(4, 1000).await);
let node_restarter = test_cluster
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2098,7 +2098,7 @@ impl AuthorityState {

if let Err(err) = self
.database
.expensive_check_sui_conservation(cur_epoch_store.move_vm())
.expensive_check_sui_conservation(cur_epoch_store)
{
if cfg!(debug_assertions) {
panic!("{}", err);
Expand Down Expand Up @@ -3814,6 +3814,7 @@ impl AuthorityState {
epoch_start_configuration,
self.db(),
expensive_safety_check_config,
cur_epoch_store.get_chain_identifier(),
);
self.epoch_store.store(new_epoch_store.clone());
cur_epoch_store.epoch_terminated().await;
Expand Down
15 changes: 14 additions & 1 deletion crates/sui-core/src/authority/authority_per_epoch_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use sui_types::accumulator::Accumulator;
use sui_types::base_types::{AuthorityName, EpochId, ObjectID, SequenceNumber, TransactionDigest};
use sui_types::committee::Committee;
use sui_types::crypto::{AuthoritySignInfo, AuthorityStrongQuorumSignInfo};
use sui_types::digests::ChainIdentifier;
use sui_types::error::{SuiError, SuiResult};
use sui_types::signature::GenericSignature;
use sui_types::transaction::{
Expand Down Expand Up @@ -172,6 +173,9 @@ pub struct AuthorityPerEpochStore {

/// Execution state that has to restart at each epoch change
execution_component: ExecutionComponents,

/// Chain identifier
chain_identifier: ChainIdentifier,
}

/// AuthorityEpochTables contains tables that contain data that is only valid within an epoch.
Expand Down Expand Up @@ -373,6 +377,7 @@ impl AuthorityPerEpochStore {
cache_metrics: Arc<ResolverMetrics>,
signature_verifier_metrics: Arc<SignatureVerifierMetrics>,
expensive_safety_check_config: &ExpensiveSafetyCheckConfig,
chain_identifier: ChainIdentifier,
) -> Arc<Self> {
let current_time = Instant::now();
let epoch_id = committee.epoch;
Expand Down Expand Up @@ -406,7 +411,8 @@ impl AuthorityPerEpochStore {
let protocol_version = epoch_start_configuration
.epoch_start_state()
.protocol_version();
let protocol_config = ProtocolConfig::get_for_version(protocol_version);
let protocol_config =
ProtocolConfig::get_for_version(protocol_version, chain_identifier.chain());

let execution_component = ExecutionComponents::new(
&protocol_config,
Expand Down Expand Up @@ -443,6 +449,7 @@ impl AuthorityPerEpochStore {
metrics,
epoch_start_configuration,
execution_component,
chain_identifier,
});
s.update_buffer_stake_metric();
s
Expand All @@ -462,13 +469,18 @@ impl AuthorityPerEpochStore {
self.epoch_start_configuration.epoch_start_state()
}

pub fn get_chain_identifier(&self) -> ChainIdentifier {
self.chain_identifier
}

pub fn new_at_next_epoch(
&self,
name: AuthorityName,
new_committee: Committee,
epoch_start_configuration: EpochStartConfiguration,
store: Arc<AuthorityStore>,
expensive_safety_check_config: &ExpensiveSafetyCheckConfig,
chain_identifier: ChainIdentifier,
) -> Arc<Self> {
assert_eq!(self.epoch() + 1, new_committee.epoch);
self.record_reconfig_halt_duration_metric();
Expand All @@ -484,6 +496,7 @@ impl AuthorityPerEpochStore {
self.execution_component.metrics(),
self.signature_verifier.metrics.clone(),
expensive_safety_check_config,
chain_identifier,
)
}

Expand Down
13 changes: 10 additions & 3 deletions crates/sui-core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use fastcrypto::hash::{HashFunction, MultisetHash, Sha3_256};
use futures::stream::FuturesUnordered;
use move_bytecode_utils::module_cache::GetModule;
use move_core_types::resolver::ModuleResolver;
use move_vm_runtime::move_vm::MoveVM;
use once_cell::sync::OnceCell;
use serde::{Deserialize, Serialize};
use sui_types::messages_checkpoint::ECMHLiveObjectSetDigest;
Expand Down Expand Up @@ -1575,16 +1574,24 @@ impl AuthorityStore {
self.perpetual_tables.iter_live_object_set()
}

pub fn expensive_check_sui_conservation(self: &Arc<Self>, move_vm: &Arc<MoveVM>) -> SuiResult {
pub fn expensive_check_sui_conservation(
self: &Arc<Self>,
old_epoch_store: &AuthorityPerEpochStore,
) -> SuiResult {
if !self.enable_epoch_sui_conservation_check {
return Ok(());
}

let move_vm = old_epoch_store.move_vm();
let chain_identifier = old_epoch_store.get_chain_identifier();

let protocol_version = ProtocolVersion::new(
self.get_sui_system_state_object()
.expect("Read sui system state object cannot fail")
.protocol_version(),
);
let protocol_config = ProtocolConfig::get_for_version(protocol_version);
let protocol_config =
ProtocolConfig::get_for_version(protocol_version, chain_identifier.chain());
// Prior to gas model v2, SUI conservation is not guaranteed.
if protocol_config.gas_model_version() <= 1 {
return Ok(());
Expand Down
2 changes: 2 additions & 0 deletions crates/sui-core/src/authority/test_authority_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use sui_storage::IndexStore;
use sui_swarm_config::network_config::NetworkConfig;
use sui_types::base_types::{AuthorityName, ObjectID};
use sui_types::crypto::AuthorityKeyPair;
use sui_types::digests::ChainIdentifier;
use sui_types::error::SuiResult;
use sui_types::executable_transaction::VerifiedExecutableTransaction;
use sui_types::object::Object;
Expand Down Expand Up @@ -193,6 +194,7 @@ impl<'a> TestAuthorityBuilder<'a> {
cache_metrics,
signature_verifier_metrics,
&expensive_safety_checks,
ChainIdentifier::from(*genesis.checkpoint().digest()),
);

let committee_store = Arc::new(CommitteeStore::new(
Expand Down
10 changes: 7 additions & 3 deletions crates/sui-core/src/scoring_decision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,15 +304,19 @@ mod tests {
use rand::SeedableRng;
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use sui_protocol_config::{ProtocolConfig, ProtocolVersion};
use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion};
use sui_types::crypto::NetworkPublicKey;

fn protocol_v4() -> ProtocolConfig {
ProtocolConfig::get_for_version(ProtocolVersion::new(4))
// There are no chain specific protocol config options at this version
// so the chain is irrelevant
ProtocolConfig::get_for_version(ProtocolVersion::new(4), Chain::Unknown)
}

fn protocol_v5() -> ProtocolConfig {
ProtocolConfig::get_for_version(ProtocolVersion::new(5))
// There are no chain specific protocol config options at this version
// so the chain is irrelevant
ProtocolConfig::get_for_version(ProtocolVersion::new(5), Chain::Unknown)
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-core/src/unit_tests/move_package_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use sui_adapter::adapter::{
};
use sui_framework::BuiltInFramework;
use sui_move_build::{BuildConfig, CompiledPackage, SuiPackageHooks};
use sui_protocol_config::ProtocolConfig;
use sui_protocol_config::{Chain, ProtocolConfig};
use sui_types::{
base_types::{random_object_ref, ObjectID},
crypto::{get_key_pair, AccountKeyPair},
Expand Down Expand Up @@ -438,7 +438,7 @@ fn test_fail_on_upgrade_missing_type() {
.new_upgraded(
c_id2,
&build_test_modules("Cv1"),
&ProtocolConfig::get_for_version(4.into()),
&ProtocolConfig::get_for_version(4.into(), Chain::Unknown),
[],
)
.unwrap_err();
Expand Down
9 changes: 7 additions & 2 deletions crates/sui-e2e-tests/tests/reconfiguration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use sui_core::safe_client::SafeClientMetricsBase;
use sui_json_rpc_types::SuiTransactionBlockEffectsAPI;
use sui_macros::sim_test;
use sui_node::SuiNodeHandle;
use sui_protocol_config::ProtocolConfig;
use sui_swarm_config::network_config_builder::ConfigBuilder;
use sui_test_transaction_builder::TestTransactionBuilder;
use sui_types::base_types::{AuthorityName, ObjectRef, SuiAddress};
Expand Down Expand Up @@ -279,7 +280,11 @@ async fn reconfig_with_revert_end_to_end_test() {
#[sim_test]
async fn test_passive_reconfig() {
telemetry_subscribers::init_for_testing();
sui_protocol_config::ProtocolConfig::poison_get_for_min_version();
let _commit_root_state_digest = ProtocolConfig::apply_overrides_for_testing(|_, mut config| {
config.set_commit_root_state_digest_supported(true);
config
});
ProtocolConfig::poison_get_for_min_version();

let test_cluster = TestClusterBuilder::new()
.with_epoch_duration_ms(1000)
Expand Down Expand Up @@ -307,7 +312,7 @@ async fn test_passive_reconfig() {
.get_epoch_state_commitments(0)
.unwrap()
.unwrap();
assert_eq!(commitments.len(), 0);
assert_eq!(commitments.len(), 1);
});
}

Expand Down
5 changes: 3 additions & 2 deletions crates/sui-framework-snapshot/tests/compatibility_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ mod compatibility_tests {
use std::collections::BTreeMap;
use sui_framework::{compare_system_package, BuiltInFramework};
use sui_framework_snapshot::{load_bytecode_snapshot, load_bytecode_snapshot_manifest};
use sui_protocol_config::{ProtocolConfig, ProtocolVersion};
use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion};

#[tokio::test]
async fn test_framework_compatibility() {
// This test checks that the current framework is compatible with all previous framework
// bytecode snapshots.
for (version, _snapshots) in load_bytecode_snapshot_manifest() {
let config = ProtocolConfig::get_for_version(ProtocolVersion::new(version));
let config =
ProtocolConfig::get_for_version(ProtocolVersion::new(version), Chain::Unknown);
let max_binary_format_version = config.move_binary_format_version();
let no_extraneous_module_bytes = config.no_extraneous_module_bytes();
let framework = load_bytecode_snapshot(version).unwrap();
Expand Down
29 changes: 22 additions & 7 deletions crates/sui-genesis-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use sui_config::genesis::{
UnsignedGenesis,
};
use sui_framework::BuiltInFramework;
use sui_protocol_config::{ProtocolConfig, ProtocolVersion};
use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion};
use sui_types::base_types::{
ExecutionDigests, ObjectID, SequenceNumber, SuiAddress, TransactionDigest, TxContext,
};
Expand Down Expand Up @@ -720,7 +720,13 @@ fn build_unsigned_genesis_data(
metrics.clone(),
);

let protocol_config = ProtocolConfig::get_for_version(parameters.protocol_version);
// We have a circular dependency here. Protocol config depends on chain ID, which
// depends on genesis checkpoint (digest), which depends on genesis transaction, which
// depends on protocol config.
// However since we know there are no chain specific protocol config options in genesis,
// we use Chain::Unknown here.
let protocol_config =
ProtocolConfig::get_for_version(parameters.protocol_version, Chain::Unknown);

let (genesis_transaction, genesis_effects, genesis_events, objects) =
create_genesis_transaction(objects, &protocol_config, metrics, &epoch_data);
Expand Down Expand Up @@ -870,8 +876,13 @@ fn create_genesis_objects(
metrics: Arc<LimitsMetrics>,
) -> Vec<Object> {
let mut store = InMemoryStorage::new(Vec::new());
let protocol_config =
ProtocolConfig::get_for_version(ProtocolVersion::new(parameters.protocol_version));
// We don't know the chain ID here since we haven't yet created the genesis checkpoint.
// However since we know there are no chain specific protool config options in genesis,
// we use Chain::Unknown here.
let protocol_config = ProtocolConfig::get_for_version(
ProtocolVersion::new(parameters.protocol_version),
Chain::Unknown,
);

let native_functions = sui_move_natives::all_natives(/* silent */ true);
// paranoid checks are a last line of defense for malicious code, no need to run them in genesis
Expand Down Expand Up @@ -1006,9 +1017,13 @@ pub fn generate_genesis_system_object(
metrics: Arc<LimitsMetrics>,
) -> anyhow::Result<()> {
let genesis_digest = genesis_ctx.digest();
let protocol_config = ProtocolConfig::get_for_version(ProtocolVersion::new(
genesis_chain_parameters.protocol_version,
));
// We don't know the chain ID here since we haven't yet created the genesis checkpoint.
// However since we know there are no chain specific protocol config options in genesis,
// we use Chain::Unknown here.
let protocol_config = ProtocolConfig::get_for_version(
ProtocolVersion::new(genesis_chain_parameters.protocol_version),
sui_protocol_config::Chain::Unknown,
);
let mut temporary_store = TemporaryStore::new(
&*store,
InputObjects::new(vec![]),
Expand Down
15 changes: 11 additions & 4 deletions crates/sui-json-rpc/src/read_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,12 +920,19 @@ impl ReadApiServer for ReadApi {
with_tracing!(async move {
Ok(version
.map(|v| {
ProtocolConfig::get_for_version_if_supported((*v).into()).ok_or(
Error::SuiRpcInputError(SuiRpcInputError::ProtocolVersionUnsupported(
ProtocolConfig::get_for_version_if_supported(
(*v).into(),
self.state
.get_chain_identifier()
.ok_or(anyhow!("Chain identifier not found"))?
.chain(),
)
.ok_or(Error::SuiRpcInputError(
SuiRpcInputError::ProtocolVersionUnsupported(
ProtocolVersion::MIN.as_u64(),
ProtocolVersion::MAX.as_u64(),
)),
)
),
))
})
.unwrap_or(Ok(self
.state
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-move-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use move_package::{
};
use move_symbol_pool::Symbol;
use serde_reflection::Registry;
use sui_protocol_config::{ProtocolConfig, ProtocolVersion};
use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion};
use sui_types::{
base_types::ObjectID,
error::{SuiError, SuiResult},
Expand Down Expand Up @@ -202,7 +202,7 @@ pub fn build_from_resolution_graph(
})?;
// TODO make this configurable
sui_bytecode_verifier::sui_verify_module_unmetered(
&ProtocolConfig::get_for_version(ProtocolVersion::MAX),
&ProtocolConfig::get_for_version(ProtocolVersion::MAX, Chain::Unknown),
m,
&fn_info,
)?;
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ impl SuiNode {
cache_metrics,
signature_verifier_metrics,
&config.expensive_safety_check_config,
ChainIdentifier::from(*genesis.checkpoint().digest()),
);

// the database is empty at genesis time
Expand All @@ -310,7 +311,7 @@ impl SuiNode {
// an epoch and the SUI conservation check will fail. This also initialize
// the expected_network_sui_amount table.
store
.expensive_check_sui_conservation(epoch_store.move_vm())
.expensive_check_sui_conservation(&epoch_store)
.expect("SUI conservation check cannot fail at genesis");
}

Expand Down
Loading

0 comments on commit e8a351e

Please sign in to comment.