Skip to content

Commit

Permalink
Re-enable dedup and shuffling as defaults (for test networks) (#9397)
Browse files Browse the repository at this point in the history
### Description

Introduce a new `Missing` version for `OnChainExecutionConfig` that will maintain backwards compatibility for before the config was registered. Use this when the config is missing from epoch manager.

Otherwise, use `default_for_genesis` for all new networks e.g., forge, devnet.

### Test Plan

Observe that e2e tests use both shuffle and dedup.
  • Loading branch information
bchocho authored Aug 2, 2023
1 parent 429f4dd commit 285a61c
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 55 deletions.
6 changes: 3 additions & 3 deletions aptos-move/vm-genesis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub fn encode_aptos_mainnet_genesis_transaction(

// On-chain genesis process.
let consensus_config = OnChainConsensusConfig::default();
let execution_config = OnChainExecutionConfig::default();
let execution_config = OnChainExecutionConfig::default_for_genesis();
let gas_schedule = default_gas_schedule();
initialize(
&mut session,
Expand Down Expand Up @@ -798,7 +798,7 @@ pub fn generate_test_genesis(
employee_vesting_period_duration: 5 * 60, // 5 minutes
},
&OnChainConsensusConfig::default(),
&OnChainExecutionConfig::default(),
&OnChainExecutionConfig::default_for_genesis(),
&default_gas_schedule(),
);
(genesis, test_validators)
Expand All @@ -820,7 +820,7 @@ pub fn generate_mainnet_genesis(
ChainId::test(),
&mainnet_genesis_config(),
&OnChainConsensusConfig::default(),
&OnChainExecutionConfig::default(),
&OnChainExecutionConfig::default_for_genesis(),
&default_gas_schedule(),
);
(genesis, test_validators)
Expand Down
3 changes: 2 additions & 1 deletion consensus/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,8 @@ impl EpochManager {
match self.storage.start() {
LivenessStorageData::FullRecoveryData(initial_data) => {
let consensus_config = onchain_consensus_config.unwrap_or_default();
let execution_config = onchain_execution_config.unwrap_or_default();
let execution_config = onchain_execution_config
.unwrap_or_else(|_| OnChainExecutionConfig::default_if_missing());
self.quorum_store_enabled = self.enable_quorum_store(&consensus_config);
self.recovery_mode = false;
self.start_round_manager(
Expand Down
10 changes: 2 additions & 8 deletions crates/aptos-genesis/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ use aptos_keygen::KeyGen;
use aptos_logger::prelude::*;
use aptos_types::{
chain_id::ChainId,
on_chain_config::{
ExecutionConfigV1, GasScheduleV2, OnChainConsensusConfig, OnChainExecutionConfig,
TransactionShufflerType,
},
on_chain_config::{GasScheduleV2, OnChainConsensusConfig, OnChainExecutionConfig},
transaction::Transaction,
waypoint::Waypoint,
};
Expand Down Expand Up @@ -613,10 +610,7 @@ impl Builder {
employee_vesting_start: None,
employee_vesting_period_duration: None,
consensus_config: OnChainConsensusConfig::default(),
// Enable transaction shuffling by default in integration tests and Forge.
execution_config: OnChainExecutionConfig::V1(ExecutionConfigV1 {
transaction_shuffler_type: TransactionShufflerType::SenderAwareV2(32),
}),
execution_config: OnChainExecutionConfig::default_for_genesis(),
gas_schedule: default_gas_schedule(),
};
if let Some(init_genesis_config) = &self.init_genesis_config {
Expand Down
10 changes: 3 additions & 7 deletions crates/aptos/src/genesis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ use aptos_genesis::{
use aptos_logger::info;
use aptos_types::{
account_address::{AccountAddress, AccountAddressWithChecks},
on_chain_config::{
ExecutionConfigV1, OnChainConsensusConfig, OnChainExecutionConfig, TransactionShufflerType,
},
on_chain_config::{OnChainConsensusConfig, OnChainExecutionConfig},
};
use aptos_vm_genesis::{default_gas_schedule, AccountBalance, EmployeePool};
use async_trait::async_trait;
Expand Down Expand Up @@ -257,7 +255,7 @@ pub fn fetch_mainnet_genesis_info(git_options: GitOptions) -> CliTypedResult<Mai
employee_vesting_start: layout.employee_vesting_start,
employee_vesting_period_duration: layout.employee_vesting_period_duration,
consensus_config: OnChainConsensusConfig::default(),
execution_config: OnChainExecutionConfig::default(),
execution_config: OnChainExecutionConfig::default_for_genesis(),
gas_schedule: default_gas_schedule(),
},
)?)
Expand Down Expand Up @@ -297,9 +295,7 @@ pub fn fetch_genesis_info(git_options: GitOptions) -> CliTypedResult<GenesisInfo
employee_vesting_start: layout.employee_vesting_start,
employee_vesting_period_duration: layout.employee_vesting_period_duration,
consensus_config: OnChainConsensusConfig::default(),
execution_config: OnChainExecutionConfig::V1(ExecutionConfigV1 {
transaction_shuffler_type: TransactionShufflerType::SenderAwareV2(32),
}),
execution_config: OnChainExecutionConfig::default_for_genesis(),
gas_schedule: default_gas_schedule(),
},
)?)
Expand Down
4 changes: 2 additions & 2 deletions testsuite/smoke-test/src/rest_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use aptos_sdk::move_types::language_storage::StructTag;
use aptos_types::{
account_address::AccountAddress,
account_config::{AccountResource, CORE_CODE_ADDRESS},
on_chain_config::{ExecutionConfigV2, OnChainExecutionConfig},
on_chain_config::{ExecutionConfigV2, OnChainExecutionConfig, TransactionShufflerType},
transaction::{authenticator::AuthenticationKey, SignedTransaction, Transaction},
};
use std::{convert::TryFrom, str::FromStr, sync::Arc, time::Duration};
Expand Down Expand Up @@ -209,8 +209,8 @@ async fn test_gas_estimation_gas_used_limit() {
let mut swarm = SwarmBuilder::new_local(1)
.with_init_genesis_config(Arc::new(|conf| {
conf.execution_config = OnChainExecutionConfig::V2(ExecutionConfigV2 {
transaction_shuffler_type: TransactionShufflerType::NoShuffling,
block_gas_limit: Some(1),
..Default::default()
});
}))
.with_init_config(Arc::new(|_, conf, _| {
Expand Down
58 changes: 24 additions & 34 deletions types/src/on_chain_config/execution_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,18 @@ pub enum OnChainExecutionConfig {
V1(ExecutionConfigV1),
V2(ExecutionConfigV2),
V3(ExecutionConfigV3),
/// To maintain backwards compatibility on replay, we must ensure that any new features resolve
/// to previous behavior (before OnChainExecutionConfig was registered) in case of Missing.
Missing,
// Reminder: Add V4 and future versions here, after Missing (order matters for enums).
}

/// The public interface that exposes all values with safe fallback.
impl OnChainExecutionConfig {
/// The type of the transaction shuffler being used.
pub fn transaction_shuffler_type(&self) -> TransactionShufflerType {
match &self {
OnChainExecutionConfig::Missing => TransactionShufflerType::NoShuffling,
OnChainExecutionConfig::V1(config) => config.transaction_shuffler_type.clone(),
OnChainExecutionConfig::V2(config) => config.transaction_shuffler_type.clone(),
OnChainExecutionConfig::V3(config) => config.transaction_shuffler_type.clone(),
Expand All @@ -27,6 +32,7 @@ impl OnChainExecutionConfig {
/// The per-block gas limit being used.
pub fn block_gas_limit(&self) -> Option<u64> {
match &self {
OnChainExecutionConfig::Missing => None,
OnChainExecutionConfig::V1(_config) => None,
OnChainExecutionConfig::V2(config) => config.block_gas_limit,
OnChainExecutionConfig::V3(config) => config.block_gas_limit,
Expand All @@ -36,17 +42,28 @@ impl OnChainExecutionConfig {
/// The type of the transaction deduper being used.
pub fn transaction_deduper_type(&self) -> TransactionDeduperType {
match &self {
// Note, this behavior was enabled before OnChainExecutionConfig was registered.
OnChainExecutionConfig::Missing => TransactionDeduperType::TxnHashAndAuthenticatorV1,
OnChainExecutionConfig::V1(_config) => TransactionDeduperType::NoDedup,
OnChainExecutionConfig::V2(_config) => TransactionDeduperType::NoDedup,
OnChainExecutionConfig::V3(config) => config.transaction_deduper_type.clone(),
}
}
}

/// This is used when on-chain config is not initialized.
impl Default for OnChainExecutionConfig {
fn default() -> Self {
OnChainExecutionConfig::V3(ExecutionConfigV3::default())
/// The default values to use for new networks, e.g., devnet, forge.
/// Features that are ready for deployment can be enabled here.
pub fn default_for_genesis() -> Self {
OnChainExecutionConfig::V3(ExecutionConfigV3 {
transaction_shuffler_type: TransactionShufflerType::SenderAwareV2(32),
block_gas_limit: Some(35000),
transaction_deduper_type: TransactionDeduperType::TxnHashAndAuthenticatorV1,
})
}

/// The default values to use when on-chain config is not initialized.
/// This value should not be changed, for replay purposes.
pub fn default_if_missing() -> Self {
OnChainExecutionConfig::Missing
}
}

Expand All @@ -73,46 +90,19 @@ pub struct ExecutionConfigV1 {
pub transaction_shuffler_type: TransactionShufflerType,
}

impl Default for ExecutionConfigV1 {
fn default() -> Self {
Self {
transaction_shuffler_type: TransactionShufflerType::NoShuffling,
}
}
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)]
pub struct ExecutionConfigV2 {
pub transaction_shuffler_type: TransactionShufflerType,
pub block_gas_limit: Option<u64>,
}

impl Default for ExecutionConfigV2 {
fn default() -> Self {
Self {
transaction_shuffler_type: TransactionShufflerType::NoShuffling,
block_gas_limit: None,
}
}
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Serialize)]
pub struct ExecutionConfigV3 {
pub transaction_shuffler_type: TransactionShufflerType,
pub block_gas_limit: Option<u64>,
pub transaction_deduper_type: TransactionDeduperType,
}

impl Default for ExecutionConfigV3 {
fn default() -> Self {
Self {
transaction_shuffler_type: TransactionShufflerType::NoShuffling,
block_gas_limit: None,
transaction_deduper_type: TransactionDeduperType::TxnHashAndAuthenticatorV1,
}
}
}

#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
#[serde(rename_all = "snake_case")] // cannot use tag = "type" as nested enums cannot work, and bcs doesn't support it
pub enum TransactionShufflerType {
Expand All @@ -137,15 +127,15 @@ mod test {

#[test]
fn test_config_yaml_serialization() {
let config = OnChainExecutionConfig::default();
let config = OnChainExecutionConfig::default_for_genesis();
let s = serde_yaml::to_string(&config).unwrap();

serde_yaml::from_str::<OnChainExecutionConfig>(&s).unwrap();
}

#[test]
fn test_config_bcs_serialization() {
let config = OnChainExecutionConfig::default();
let config = OnChainExecutionConfig::default_for_genesis();
let s = bcs::to_bytes(&config).unwrap();

bcs::from_bytes::<OnChainExecutionConfig>(&s).unwrap();
Expand Down

0 comments on commit 285a61c

Please sign in to comment.