From fdd0612aaca7f2c3bc8cecf6c82326f83770db05 Mon Sep 17 00:00:00 2001 From: John C Date: Tue, 4 Jun 2024 20:27:12 +0100 Subject: [PATCH 01/26] Assert collection creator prior to token creation (#13550) --- .../aptos-token-objects/doc/token.md | 26 +++++--- .../aptos-token-objects/sources/token.move | 63 ++++++++++++++++--- 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/aptos-move/framework/aptos-token-objects/doc/token.md b/aptos-move/framework/aptos-token-objects/doc/token.md index bf1e5976301d0..b4c53eb85ef63 100644 --- a/aptos-move/framework/aptos-token-objects/doc/token.md +++ b/aptos-move/framework/aptos-token-objects/doc/token.md @@ -473,7 +473,7 @@ The token name is over the maximum length -
fun create_common(constructor_ref: &object::ConstructorRef, creator_address: address, collection_name: string::String, description: string::String, name_prefix: string::String, name_with_index_suffix: option::Option<string::String>, royalty: option::Option<royalty::Royalty>, uri: string::String)
+
fun create_common(creator: &signer, constructor_ref: &object::ConstructorRef, collection_name: string::String, description: string::String, name_prefix: string::String, name_with_index_suffix: option::Option<string::String>, royalty: option::Option<royalty::Royalty>, uri: string::String)
 
@@ -483,8 +483,8 @@ The token name is over the maximum length
inline fun create_common(
+    creator: &signer,
     constructor_ref: &ConstructorRef,
-    creator_address: address,
     collection_name: String,
     description: String,
     name_prefix: String,
@@ -494,10 +494,12 @@ The token name is over the maximum length
     royalty: Option<Royalty>,
     uri: String,
 ) {
+    let creator_address = signer::address_of(creator);
     let collection_addr = collection::create_collection_address(&creator_address, &collection_name);
     let collection = object::address_to_object<Collection>(collection_addr);
 
     create_common_with_collection(
+        creator,
         constructor_ref,
         collection,
         description,
@@ -519,7 +521,7 @@ The token name is over the maximum length
 
 
 
-
fun create_common_with_collection(constructor_ref: &object::ConstructorRef, collection: object::Object<collection::Collection>, description: string::String, name_prefix: string::String, name_with_index_suffix: option::Option<string::String>, royalty: option::Option<royalty::Royalty>, uri: string::String)
+
fun create_common_with_collection(creator: &signer, constructor_ref: &object::ConstructorRef, collection: object::Object<collection::Collection>, description: string::String, name_prefix: string::String, name_with_index_suffix: option::Option<string::String>, royalty: option::Option<royalty::Royalty>, uri: string::String)
 
@@ -529,6 +531,7 @@ The token name is over the maximum length
inline fun create_common_with_collection(
+    creator: &signer,
     constructor_ref: &ConstructorRef,
     collection: Object<Collection>,
     description: String,
@@ -539,6 +542,8 @@ The token name is over the maximum length
     royalty: Option<Royalty>,
     uri: String,
 ) {
+    assert!(collection::creator(collection) == signer::address_of(creator), error::unauthenticated(ENOT_CREATOR));
+
     if (option::is_some(&name_with_index_suffix)) {
         // Be conservative, as we don't know what length the index will be, and assume worst case (20 chars in MAX_U64)
         assert!(
@@ -626,6 +631,7 @@ This function must be called if the collection name has been previously changed.
     let creator_address = signer::address_of(creator);
     let constructor_ref = object::create_object(creator_address);
     create_common_with_collection(
+        creator,
         &constructor_ref,
         collection,
         description,
@@ -670,8 +676,8 @@ for additional specialization.
     let creator_address = signer::address_of(creator);
     let constructor_ref = object::create_object(creator_address);
     create_common(
+        creator,
         &constructor_ref,
-        creator_address,
         collection_name,
         description,
         name,
@@ -722,6 +728,7 @@ This function must be called if the collection name has been previously changed.
     let creator_address = signer::address_of(creator);
     let constructor_ref = object::create_object(creator_address);
     create_common_with_collection(
+        creator,
         &constructor_ref,
         collection,
         description,
@@ -770,8 +777,8 @@ while providing sequential names.
     let creator_address = signer::address_of(creator);
     let constructor_ref = object::create_object(creator_address);
     create_common(
+        creator,
         &constructor_ref,
-        creator_address,
         collection_name,
         description,
         name_with_index_prefix,
@@ -816,6 +823,7 @@ This function must be called if the collection name has been previously changed.
     let seed = create_token_seed(&collection::name(collection), &name);
     let constructor_ref = object::create_named_object(creator, seed);
     create_common_with_collection(
+        creator,
         &constructor_ref,
         collection,
         description,
@@ -857,13 +865,12 @@ additional specialization.
     royalty: Option<Royalty>,
     uri: String,
 ): ConstructorRef {
-    let creator_address = signer::address_of(creator);
     let seed = create_token_seed(&collection_name, &name);
 
     let constructor_ref = object::create_named_object(creator, seed);
     create_common(
+        creator,
         &constructor_ref,
-        creator_address,
         collection_name,
         description,
         name,
@@ -908,7 +915,7 @@ This function must be called if the collection name has been previously changed.
 ): ConstructorRef {
     let seed = create_token_name_with_seed(&collection::name(collection), &name, &seed);
     let constructor_ref = object::create_named_object(creator, seed);
-    create_common_with_collection(&constructor_ref, collection, description, name, option::none(), royalty, uri);
+    create_common_with_collection(creator, &constructor_ref, collection, description, name, option::none(), royalty, uri);
     constructor_ref
 }
 
@@ -945,11 +952,10 @@ additional specialization. royalty: Option<Royalty>, uri: String, ): ConstructorRef { - let creator_address = signer::address_of(creator); let constructor_ref = object::create_object_from_account(creator); create_common( + creator, &constructor_ref, - creator_address, collection_name, description, name, diff --git a/aptos-move/framework/aptos-token-objects/sources/token.move b/aptos-move/framework/aptos-token-objects/sources/token.move index b8ad7f3804cca..0d944c99c5cfc 100644 --- a/aptos-move/framework/aptos-token-objects/sources/token.move +++ b/aptos-move/framework/aptos-token-objects/sources/token.move @@ -115,8 +115,8 @@ module aptos_token_objects::token { } inline fun create_common( + creator: &signer, constructor_ref: &ConstructorRef, - creator_address: address, collection_name: String, description: String, name_prefix: String, @@ -126,10 +126,12 @@ module aptos_token_objects::token { royalty: Option, uri: String, ) { + let creator_address = signer::address_of(creator); let collection_addr = collection::create_collection_address(&creator_address, &collection_name); let collection = object::address_to_object(collection_addr); create_common_with_collection( + creator, constructor_ref, collection, description, @@ -141,6 +143,7 @@ module aptos_token_objects::token { } inline fun create_common_with_collection( + creator: &signer, constructor_ref: &ConstructorRef, collection: Object, description: String, @@ -151,6 +154,8 @@ module aptos_token_objects::token { royalty: Option, uri: String, ) { + assert!(collection::creator(collection) == signer::address_of(creator), error::unauthenticated(ENOT_CREATOR)); + if (option::is_some(&name_with_index_suffix)) { // Be conservative, as we don't know what length the index will be, and assume worst case (20 chars in MAX_U64) assert!( @@ -218,6 +223,7 @@ module aptos_token_objects::token { let creator_address = signer::address_of(creator); let constructor_ref = object::create_object(creator_address); create_common_with_collection( + creator, &constructor_ref, collection, description, @@ -242,8 +248,8 @@ module aptos_token_objects::token { let creator_address = signer::address_of(creator); let constructor_ref = object::create_object(creator_address); create_common( + creator, &constructor_ref, - creator_address, collection_name, description, name, @@ -274,6 +280,7 @@ module aptos_token_objects::token { let creator_address = signer::address_of(creator); let constructor_ref = object::create_object(creator_address); create_common_with_collection( + creator, &constructor_ref, collection, description, @@ -302,8 +309,8 @@ module aptos_token_objects::token { let creator_address = signer::address_of(creator); let constructor_ref = object::create_object(creator_address); create_common( + creator, &constructor_ref, - creator_address, collection_name, description, name_with_index_prefix, @@ -328,6 +335,7 @@ module aptos_token_objects::token { let seed = create_token_seed(&collection::name(collection), &name); let constructor_ref = object::create_named_object(creator, seed); create_common_with_collection( + creator, &constructor_ref, collection, description, @@ -349,13 +357,12 @@ module aptos_token_objects::token { royalty: Option, uri: String, ): ConstructorRef { - let creator_address = signer::address_of(creator); let seed = create_token_seed(&collection_name, &name); let constructor_ref = object::create_named_object(creator, seed); create_common( + creator, &constructor_ref, - creator_address, collection_name, description, name, @@ -380,7 +387,7 @@ module aptos_token_objects::token { ): ConstructorRef { let seed = create_token_name_with_seed(&collection::name(collection), &name, &seed); let constructor_ref = object::create_named_object(creator, seed); - create_common_with_collection(&constructor_ref, collection, description, name, option::none(), royalty, uri); + create_common_with_collection(creator, &constructor_ref, collection, description, name, option::none(), royalty, uri); constructor_ref } @@ -397,11 +404,10 @@ module aptos_token_objects::token { royalty: Option, uri: String, ): ConstructorRef { - let creator_address = signer::address_of(creator); let constructor_ref = object::create_object_from_account(creator); create_common( + creator, &constructor_ref, - creator_address, collection_name, description, name, @@ -715,6 +721,47 @@ module aptos_token_objects::token { assert!(option::some(expected_royalty) == royalty(token), 2); } + #[test(creator = @0x123, trader = @0x456)] + #[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)] + fun test_create_token_non_creator(creator: &signer, trader: &signer) { + let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5); + let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref)); + create_token( + trader, collection, string::utf8(b"token description"), string::utf8(b"token name"), + option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"), + ); + } + + #[test(creator = @0x123, trader = @0x456)] + #[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)] + fun test_create_named_token_non_creator(creator: &signer, trader: &signer) { + let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5); + let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref)); + create_token_with_collection_helper(trader, collection, string::utf8(b"token name")); + } + + #[test(creator = @0x123, trader = @0x456)] + #[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)] + fun test_create_named_token_object_non_creator(creator: &signer, trader: &signer) { + let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5); + let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref)); + create_named_token_object( + trader, collection, string::utf8(b"token description"), string::utf8(b"token name"), + option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"), + ); + } + + #[test(creator = @0x123, trader = @0x456)] + #[expected_failure(abort_code = 0x40002, location = aptos_token_objects::token)] + fun test_create_named_token_from_seed_non_creator(creator: &signer, trader: &signer) { + let constructor_ref = &create_fixed_collection(creator, string::utf8(b"collection name"), 5); + let collection = get_collection_from_ref(&object::generate_extend_ref(constructor_ref)); + create_named_token_object( + trader, collection, string::utf8(b"token description"), string::utf8(b"token name"), + option::some(royalty::create(25, 10000, signer::address_of(creator))), string::utf8(b"uri"), + ); + } + #[test(creator = @0x123, trader = @0x456)] fun test_create_and_transfer_token_with_seed(creator: &signer, trader: &signer) acquires Token { let collection_name = string::utf8(b"collection name"); From 951c5975472d7b503e2e077b4ac5d0d217232595 Mon Sep 17 00:00:00 2001 From: Alden Hu Date: Tue, 4 Jun 2024 13:36:53 -0700 Subject: [PATCH 02/26] trivial: fix build warning when fuzzing feature is not enabled (#13541) --- consensus/consensus-types/src/wrapped_ledger_info.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/consensus/consensus-types/src/wrapped_ledger_info.rs b/consensus/consensus-types/src/wrapped_ledger_info.rs index 70f65ecc8991b..dbfec7cfc8233 100644 --- a/consensus/consensus-types/src/wrapped_ledger_info.rs +++ b/consensus/consensus-types/src/wrapped_ledger_info.rs @@ -6,9 +6,7 @@ use crate::{quorum_cert::QuorumCert, vote_data::VoteData}; use anyhow::{ensure, Context}; use aptos_crypto::hash::CryptoHash; use aptos_types::{ - aggregate_signature::AggregateSignature, - block_info::BlockInfo, - ledger_info::{LedgerInfo, LedgerInfoWithSignatures}, + aggregate_signature, block_info::BlockInfo, ledger_info::LedgerInfoWithSignatures, validator_verifier::ValidatorVerifier, }; use serde::{Deserialize, Serialize}; @@ -51,8 +49,8 @@ impl WrappedLedgerInfo { Self { vote_data: VoteData::dummy(), signed_ledger_info: LedgerInfoWithSignatures::new( - LedgerInfo::dummy(), - AggregateSignature::empty(), + aptos_types::ledger_info::LedgerInfo::dummy(), + aggregate_signature::AggregateSignature::empty(), ), } } From 95e49b79ca5e3344443c2413e5fbd11016f5efc0 Mon Sep 17 00:00:00 2001 From: Alden Hu Date: Tue, 4 Jun 2024 14:44:08 -0700 Subject: [PATCH 03/26] install pigz in tools image (#13548) --- docker/builder/tools.Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker/builder/tools.Dockerfile b/docker/builder/tools.Dockerfile index 1cc2663d8fd15..a62e67e099f0c 100644 --- a/docker/builder/tools.Dockerfile +++ b/docker/builder/tools.Dockerfile @@ -18,7 +18,8 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ socat \ python3-botocore/bullseye \ awscli/bullseye \ - gnupg2 + gnupg2 \ + pigz RUN echo "deb [signed-by=/usr/share/keyrings/cloud.google.gpg] http://packages.cloud.google.com/apt cloud-sdk main" | tee -a /etc/apt/sources.list.d/google-cloud-sdk.list && \ curl https://packages.cloud.google.com/apt/doc/apt-key.gpg | apt-key --keyring /usr/share/keyrings/cloud.google.gpg add - && \ From 47606b9571880546b84c0d43c68b5e77b4a51002 Mon Sep 17 00:00:00 2001 From: Teng Zhang Date: Tue, 4 Jun 2024 18:03:12 -0400 Subject: [PATCH 04/26] fix warning (#13520) --- .../checking/naming/warning_dependency.exp | 33 +++++++++++++++++++ .../checking/naming/warning_dependency.move | 4 +++ third_party/move/move-model/src/model.rs | 14 ++++---- 3 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.exp create mode 100644 third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.move diff --git a/third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.exp b/third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.exp new file mode 100644 index 0000000000000..d18cbc88a3820 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.exp @@ -0,0 +1,33 @@ + +Diagnostics: +warning: unused alias + ┌─ tests/checking/naming/warning_dependency.move:3:21 + │ +3 │ use 0x42::test::S0; + │ ^^ Unused 'use' of alias 'S0'. Consider removing it + +// -- Model dump before bytecode pipeline +module 0x42::dependency { + use 0x42::test::{S0}; +} // end 0x42::dependency +module 0x42::test { + struct S0 { + dummy_field: bool, + } + struct S1 { + dummy_field: bool, + } + struct S2 { + f: test::S3<#1>, + } + struct S3 { + dummy_field: bool, + } + struct S4 { + f: vector<#0>, + } + struct S5 { + f: vector<#0>, + g: vector<#1>, + } +} // end 0x42::test diff --git a/third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.move b/third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.move new file mode 100644 index 0000000000000..176719eaa0256 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/checking/naming/warning_dependency.move @@ -0,0 +1,4 @@ +// dep: tests/checking/naming/unused_type_parameter_struct.move +module 0x42::dependency { + use 0x42::test::S0; +} diff --git a/third_party/move/move-model/src/model.rs b/third_party/move/move-model/src/model.rs index d38bf8c168532..9d0a1cd40a060 100644 --- a/third_party/move/move-model/src/model.rs +++ b/third_party/move/move-model/src/model.rs @@ -1249,12 +1249,14 @@ impl GlobalEnv { reported_ordering } }); - for (diag, reported) in self - .diags - .borrow_mut() - .iter_mut() - .filter(|(d, reported)| !reported && filter(d)) - { + for (diag, reported) in self.diags.borrow_mut().iter_mut().filter(|(d, reported)| { + !reported + && filter(d) + && (d.severity >= Severity::Error + || d.labels + .iter() + .any(|label| self.file_id_is_primary_target.contains(&label.file_id))) + }) { if !*reported { // Avoid showing the same message twice. This can happen e.g. because of // duplication of expressions via schema inclusion. From d76d602459a84079260110c90ad4e30df36f539d Mon Sep 17 00:00:00 2001 From: Josh Lind Date: Sun, 2 Jun 2024 21:10:24 -0400 Subject: [PATCH 05/26] [Consensus Observer] Remove CO flag from state sync config. --- aptos-node/src/lib.rs | 3 --- aptos-node/src/state_sync.rs | 4 ++-- config/src/config/mod.rs | 1 + config/src/config/observer_config.rs | 2 +- config/src/config/state_sync_config.rs | 3 --- state-sync/state-sync-driver/src/driver.rs | 18 +++++++++++++++--- .../state-sync-driver/src/driver_factory.rs | 1 + .../state-sync-driver/src/tests/utils.rs | 4 +++- 8 files changed, 23 insertions(+), 13 deletions(-) diff --git a/aptos-node/src/lib.rs b/aptos-node/src/lib.rs index f7ce9e938591b..75f2aa243d6c2 100644 --- a/aptos-node/src/lib.rs +++ b/aptos-node/src/lib.rs @@ -604,9 +604,6 @@ pub fn setup_environment_and_start_node( remote_log_rx: Option>, logger_filter_update_job: Option, ) -> anyhow::Result { - // Set the observer mode for state sync - node_config.state_sync.state_sync_driver.observer_enabled = - node_config.consensus_observer.observer_enabled; // Log the node config at node startup node_config.log_all_configs(); diff --git a/aptos-node/src/state_sync.rs b/aptos-node/src/state_sync.rs index 45db82f2ac527..d92d0dad3c97c 100644 --- a/aptos-node/src/state_sync.rs +++ b/aptos-node/src/state_sync.rs @@ -64,9 +64,9 @@ pub fn create_event_subscription_service( .subscribe_to_reconfigurations() .expect("Mempool must subscribe to reconfigurations"); - // Create a reconfiguration subscription for consensus (if this is a validator) + // Create a reconfiguration subscription for consensus let consensus_reconfig_subscription = if node_config.base.role.is_validator() - || node_config.state_sync.state_sync_driver.observer_enabled + || node_config.consensus_observer.observer_enabled { Some( event_subscription_service diff --git a/config/src/config/mod.rs b/config/src/config/mod.rs index 2ed9087a7b6d9..0ea2cead65578 100644 --- a/config/src/config/mod.rs +++ b/config/src/config/mod.rs @@ -59,6 +59,7 @@ pub use netbench_config::*; pub use network_config::*; pub use node_config::*; pub use node_config_loader::sanitize_node_config; +pub use observer_config::*; pub use override_node_config::*; pub use peer_monitoring_config::*; pub use persistable_config::*; diff --git a/config/src/config/observer_config.rs b/config/src/config/observer_config.rs index 6fa55707de52b..7bfbf6943f427 100644 --- a/config/src/config/observer_config.rs +++ b/config/src/config/observer_config.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; -#[derive(Default, Clone, Debug, Deserialize, PartialEq, Serialize)] +#[derive(Default, Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] #[serde(default, deny_unknown_fields)] pub struct ObserverConfig { pub observer_enabled: bool, diff --git a/config/src/config/state_sync_config.rs b/config/src/config/state_sync_config.rs index cbad726bb770e..7dc5b296f1678 100644 --- a/config/src/config/state_sync_config.rs +++ b/config/src/config/state_sync_config.rs @@ -120,8 +120,6 @@ pub struct StateSyncDriverConfig { pub max_stream_wait_time_ms: u64, /// The version lag we'll tolerate before snapshot syncing pub num_versions_to_skip_snapshot_sync: u64, - /// Whether consensus observer mode is enabled - pub observer_enabled: bool, } /// The default state sync driver config will be the one that gets (and keeps) @@ -142,7 +140,6 @@ impl Default for StateSyncDriverConfig { max_pending_mempool_notifications: 100, max_stream_wait_time_ms: 5000, num_versions_to_skip_snapshot_sync: 100_000_000, // At 5k TPS, this allows a node to fail for about 6 hours. - observer_enabled: false, } } } diff --git a/state-sync/state-sync-driver/src/driver.rs b/state-sync/state-sync-driver/src/driver.rs index c35d7a7dd23a9..6401fe1d54f81 100644 --- a/state-sync/state-sync-driver/src/driver.rs +++ b/state-sync/state-sync-driver/src/driver.rs @@ -20,7 +20,7 @@ use crate::{ utils, utils::{OutputFallbackHandler, PENDING_DATA_LOG_FREQ_SECS}, }; -use aptos_config::config::{RoleType, StateSyncDriverConfig}; +use aptos_config::config::{ObserverConfig, RoleType, StateSyncDriverConfig}; use aptos_consensus_notifications::{ ConsensusCommitNotification, ConsensusNotification, ConsensusSyncNotification, }; @@ -53,6 +53,9 @@ pub struct DriverConfiguration { // The config file of the driver pub config: StateSyncDriverConfig, + // The config for consensus observer + pub consensus_observer_config: ObserverConfig, + // The role of the node pub role: RoleType, @@ -61,9 +64,15 @@ pub struct DriverConfiguration { } impl DriverConfiguration { - pub fn new(config: StateSyncDriverConfig, role: RoleType, waypoint: Waypoint) -> Self { + pub fn new( + config: StateSyncDriverConfig, + consensus_observer_config: ObserverConfig, + role: RoleType, + waypoint: Waypoint, + ) -> Self { Self { config, + consensus_observer_config, role, waypoint, } @@ -538,7 +547,10 @@ impl< /// Returns true iff this node enables consensus fn is_consensus_enabled(&self) -> bool { self.driver_configuration.role == RoleType::Validator - || self.driver_configuration.config.observer_enabled + || self + .driver_configuration + .consensus_observer_config + .observer_enabled } /// Returns true iff consensus is currently executing diff --git a/state-sync/state-sync-driver/src/driver_factory.rs b/state-sync/state-sync-driver/src/driver_factory.rs index ed272387b3b7e..0e904075aacde 100644 --- a/state-sync/state-sync-driver/src/driver_factory.rs +++ b/state-sync/state-sync-driver/src/driver_factory.rs @@ -158,6 +158,7 @@ impl DriverFactory { // Create the driver configuration let driver_configuration = DriverConfiguration::new( node_config.state_sync.state_sync_driver, + node_config.consensus_observer, node_config.base.role, waypoint, ); diff --git a/state-sync/state-sync-driver/src/tests/utils.rs b/state-sync/state-sync-driver/src/tests/utils.rs index 1a481e7323876..7c934756b898d 100644 --- a/state-sync/state-sync-driver/src/tests/utils.rs +++ b/state-sync/state-sync-driver/src/tests/utils.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::driver::DriverConfiguration; -use aptos_config::config::{RoleType, StateSyncDriverConfig}; +use aptos_config::config::{ObserverConfig, RoleType, StateSyncDriverConfig}; use aptos_crypto::{ ed25519::{Ed25519PrivateKey, Ed25519Signature}, HashValue, PrivateKey, Uniform, @@ -84,11 +84,13 @@ pub fn create_event(event_key: Option) -> ContractEvent { /// Creates a test driver configuration for full nodes pub fn create_full_node_driver_configuration() -> DriverConfiguration { let config = StateSyncDriverConfig::default(); + let consensus_observer_config = ObserverConfig::default(); let role = RoleType::FullNode; let waypoint = Waypoint::default(); DriverConfiguration { config, + consensus_observer_config, role, waypoint, } From 02896bc75f99007ec66b8a295fe3be6a0e5e7b89 Mon Sep 17 00:00:00 2001 From: Josh Lind Date: Sun, 2 Jun 2024 21:24:37 -0400 Subject: [PATCH 06/26] [Consensus Observer] Separate consensus and CO runtimes. --- aptos-node/src/lib.rs | 71 +++++++++++++++++------------ aptos-node/src/state_sync.rs | 20 ++++++-- consensus/src/consensus_provider.rs | 5 +- 3 files changed, 62 insertions(+), 34 deletions(-) diff --git a/aptos-node/src/lib.rs b/aptos-node/src/lib.rs index 75f2aa243d6c2..9d1b5beeb8654 100644 --- a/aptos-node/src/lib.rs +++ b/aptos-node/src/lib.rs @@ -200,6 +200,7 @@ pub struct AptosHandle { _admin_service: AdminService, _api_runtime: Option, _backup_runtime: Option, + _consensus_observer_runtime: Option, _consensus_runtime: Option, _dkg_runtime: Option, _indexer_grpc_runtime: Option, @@ -637,6 +638,7 @@ pub fn setup_environment_and_start_node( let ( mut event_subscription_service, mempool_reconfig_subscription, + consensus_observer_reconfig_subscription, consensus_reconfig_subscription, dkg_subscriptions, jwk_consensus_subscriptions, @@ -652,7 +654,7 @@ pub fn setup_environment_and_start_node( mempool_network_interfaces, peer_monitoring_service_network_interfaces, storage_service_network_interfaces, - maybe_observer_network_interfaces, + consensus_observer_network_interfaces, ) = network::setup_networks_and_get_interfaces( &node_config, chain_id, @@ -782,45 +784,54 @@ pub fn setup_environment_and_start_node( state_sync_runtimes.block_until_initialized(); debug!("State sync initialization complete."); - // Create the consensus runtime (this blocks on state sync first) - let consensus_runtime = match consensus_network_interfaces { - // validator consensus + // Create the consensus and consensus observer runtimes + let (consensus_runtime, consensus_observer_runtime) = match consensus_network_interfaces { Some(consensus_network_interfaces) => { - // Initialize and start consensus - let (runtime, consensus_db, quorum_store_db) = services::start_consensus_runtime( - &node_config, - db_rw, - consensus_reconfig_subscription, - consensus_network_interfaces, - consensus_notifier, - consensus_to_mempool_sender, - vtxn_pool, - maybe_observer_network_interfaces.map(|network| network.network_client), - ); + // Consensus is enabled, start the consensus runtime + let consensus_observer_network_client = + consensus_observer_network_interfaces.map(|network| network.network_client.clone()); + let (consensus_runtime, consensus_db, quorum_store_db) = + services::start_consensus_runtime( + &node_config, + db_rw, + consensus_reconfig_subscription, + consensus_network_interfaces, + consensus_notifier, + consensus_to_mempool_sender, + vtxn_pool, + consensus_observer_network_client, + ); admin_service.set_consensus_dbs(consensus_db, quorum_store_db); - Some(runtime) + + (Some(consensus_runtime), None) }, - // consensus observer - None if node_config.consensus_observer.observer_enabled => { - let observer_network_interfaces = maybe_observer_network_interfaces.unwrap(); - let runtime = start_consensus_observer( - &node_config, - observer_network_interfaces.network_client, - observer_network_interfaces.network_service_events, - Arc::new(consensus_notifier), - consensus_to_mempool_sender, - db_rw, - consensus_reconfig_subscription.unwrap(), - ); - Some(runtime) + None => { + if node_config.consensus_observer.observer_enabled { + // Consensus observer is enabled, start the consensus observer runtime + let consensus_observer_network_interfaces = consensus_observer_network_interfaces + .expect("Consensus observer is enabled, but network interfaces are missing!"); + let consensus_observer_runtime = start_consensus_observer( + &node_config, + consensus_observer_network_interfaces.network_client, + consensus_observer_network_interfaces.network_service_events, + Arc::new(consensus_notifier), + consensus_to_mempool_sender, + db_rw, + consensus_observer_reconfig_subscription, + ); + + (None, Some(consensus_observer_runtime)) + } else { + (None, None) + } }, - _ => None, }; Ok(AptosHandle { _admin_service: admin_service, _api_runtime: api_runtime, _backup_runtime: backup_service, + _consensus_observer_runtime: consensus_observer_runtime, _consensus_runtime: consensus_runtime, _dkg_runtime: dkg_runtime, _indexer_grpc_runtime: indexer_grpc_runtime, diff --git a/aptos-node/src/state_sync.rs b/aptos-node/src/state_sync.rs index d92d0dad3c97c..d98667977a0cc 100644 --- a/aptos-node/src/state_sync.rs +++ b/aptos-node/src/state_sync.rs @@ -46,6 +46,7 @@ pub fn create_event_subscription_service( EventSubscriptionService, ReconfigNotificationListener, Option>, + Option>, Option<( ReconfigNotificationListener, EventNotificationListener, @@ -64,10 +65,20 @@ pub fn create_event_subscription_service( .subscribe_to_reconfigurations() .expect("Mempool must subscribe to reconfigurations"); + // Create a reconfiguration subscription for consensus observer (if enabled) + let consensus_observer_reconfig_subscription = + if node_config.consensus_observer.observer_enabled { + Some( + event_subscription_service + .subscribe_to_reconfigurations() + .expect("Consensus observer must subscribe to reconfigurations"), + ) + } else { + None + }; + // Create a reconfiguration subscription for consensus - let consensus_reconfig_subscription = if node_config.base.role.is_validator() - || node_config.consensus_observer.observer_enabled - { + let consensus_reconfig_subscription = if node_config.base.role.is_validator() { Some( event_subscription_service .subscribe_to_reconfigurations() @@ -77,6 +88,7 @@ pub fn create_event_subscription_service( None }; + // Create reconfiguration subscriptions for DKG let dkg_subscriptions = if node_config.base.role.is_validator() { let reconfig_events = event_subscription_service .subscribe_to_reconfigurations() @@ -89,6 +101,7 @@ pub fn create_event_subscription_service( None }; + // Create reconfiguration subscriptions for JWK consensus let jwk_consensus_subscriptions = if node_config.base.role.is_validator() { let reconfig_events = event_subscription_service .subscribe_to_reconfigurations() @@ -104,6 +117,7 @@ pub fn create_event_subscription_service( ( event_subscription_service, mempool_reconfig_subscription, + consensus_observer_reconfig_subscription, consensus_reconfig_subscription, dkg_subscriptions, jwk_consensus_subscriptions, diff --git a/consensus/src/consensus_provider.rs b/consensus/src/consensus_provider.rs index d61002677f167..87a38c05e38fa 100644 --- a/consensus/src/consensus_provider.rs +++ b/consensus/src/consensus_provider.rs @@ -119,7 +119,7 @@ pub fn start_consensus_observer( state_sync_notifier: Arc, consensus_to_mempool_sender: mpsc::Sender, aptos_db: DbReaderWriter, - reconfig_events: ReconfigNotificationListener, + reconfig_events: Option>, ) -> Runtime { let publisher_enabled = node_config.consensus_observer.publisher_enabled; let runtime = aptos_runtimes::spawn_named_runtime("observer".into(), None); @@ -170,6 +170,9 @@ pub fn start_consensus_observer( .collect(); let network_events = Box::new(select_all(events)); + let reconfig_events = + reconfig_events.expect("Reconfig events are required for the consensus observer!"); + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let observer = Observer::new( root, From 2a70887cb6e8ea502fa3bc4bb0777d4ab06ad424 Mon Sep 17 00:00:00 2001 From: Josh Lind Date: Sun, 2 Jun 2024 21:28:24 -0400 Subject: [PATCH 07/26] [Consensus Observer] Rename ObserverConfig -> ConsensusObserverConfig --- .../{observer_config.rs => consensus_observer_config.rs} | 2 +- config/src/config/mod.rs | 4 ++-- config/src/config/node_config.rs | 8 ++++---- state-sync/state-sync-driver/src/driver.rs | 6 +++--- state-sync/state-sync-driver/src/tests/utils.rs | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) rename config/src/config/{observer_config.rs => consensus_observer_config.rs} (88%) diff --git a/config/src/config/observer_config.rs b/config/src/config/consensus_observer_config.rs similarity index 88% rename from config/src/config/observer_config.rs rename to config/src/config/consensus_observer_config.rs index 7bfbf6943f427..6315a3f0e4cd7 100644 --- a/config/src/config/observer_config.rs +++ b/config/src/config/consensus_observer_config.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; #[derive(Default, Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] #[serde(default, deny_unknown_fields)] -pub struct ObserverConfig { +pub struct ConsensusObserverConfig { pub observer_enabled: bool, pub publisher_enabled: bool, } diff --git a/config/src/config/mod.rs b/config/src/config/mod.rs index 0ea2cead65578..5d50c8dd84264 100644 --- a/config/src/config/mod.rs +++ b/config/src/config/mod.rs @@ -9,6 +9,7 @@ mod base_config; mod config_optimizer; mod config_sanitizer; mod consensus_config; +mod consensus_observer_config; mod dag_consensus_config; mod dkg_config; mod error; @@ -27,7 +28,6 @@ mod network_config; mod node_config; mod node_config_loader; mod node_startup_config; -mod observer_config; mod override_node_config; mod peer_monitoring_config; mod persistable_config; @@ -44,6 +44,7 @@ pub use admin_service_config::*; pub use api_config::*; pub use base_config::*; pub use consensus_config::*; +pub use consensus_observer_config::*; pub use dag_consensus_config::*; pub use error::*; pub use execution_config::*; @@ -59,7 +60,6 @@ pub use netbench_config::*; pub use network_config::*; pub use node_config::*; pub use node_config_loader::sanitize_node_config; -pub use observer_config::*; pub use override_node_config::*; pub use peer_monitoring_config::*; pub use persistable_config::*; diff --git a/config/src/config/node_config.rs b/config/src/config/node_config.rs index 008343fb53896..de05334025249 100644 --- a/config/src/config/node_config.rs +++ b/config/src/config/node_config.rs @@ -4,9 +4,9 @@ use super::{DagConsensusConfig, IndexerTableInfoConfig}; use crate::{ config::{ - dkg_config::DKGConfig, jwk_consensus_config::JWKConsensusConfig, - netbench_config::NetbenchConfig, node_config_loader::NodeConfigLoader, - node_startup_config::NodeStartupConfig, observer_config::ObserverConfig, + consensus_observer_config::ConsensusObserverConfig, dkg_config::DKGConfig, + jwk_consensus_config::JWKConsensusConfig, netbench_config::NetbenchConfig, + node_config_loader::NodeConfigLoader, node_startup_config::NodeStartupConfig, persistable_config::PersistableConfig, utils::RootPath, AdminServiceConfig, ApiConfig, BaseConfig, ConsensusConfig, Error, ExecutionConfig, IndexerConfig, IndexerGrpcConfig, InspectionServiceConfig, LoggerConfig, MempoolConfig, NetworkConfig, @@ -42,7 +42,7 @@ pub struct NodeConfig { #[serde(default)] pub consensus: ConsensusConfig, #[serde(default)] - pub consensus_observer: ObserverConfig, + pub consensus_observer: ConsensusObserverConfig, #[serde(default)] pub dag_consensus: DagConsensusConfig, #[serde(default)] diff --git a/state-sync/state-sync-driver/src/driver.rs b/state-sync/state-sync-driver/src/driver.rs index 6401fe1d54f81..3338cb223837e 100644 --- a/state-sync/state-sync-driver/src/driver.rs +++ b/state-sync/state-sync-driver/src/driver.rs @@ -20,7 +20,7 @@ use crate::{ utils, utils::{OutputFallbackHandler, PENDING_DATA_LOG_FREQ_SECS}, }; -use aptos_config::config::{ObserverConfig, RoleType, StateSyncDriverConfig}; +use aptos_config::config::{ConsensusObserverConfig, RoleType, StateSyncDriverConfig}; use aptos_consensus_notifications::{ ConsensusCommitNotification, ConsensusNotification, ConsensusSyncNotification, }; @@ -54,7 +54,7 @@ pub struct DriverConfiguration { pub config: StateSyncDriverConfig, // The config for consensus observer - pub consensus_observer_config: ObserverConfig, + pub consensus_observer_config: ConsensusObserverConfig, // The role of the node pub role: RoleType, @@ -66,7 +66,7 @@ pub struct DriverConfiguration { impl DriverConfiguration { pub fn new( config: StateSyncDriverConfig, - consensus_observer_config: ObserverConfig, + consensus_observer_config: ConsensusObserverConfig, role: RoleType, waypoint: Waypoint, ) -> Self { diff --git a/state-sync/state-sync-driver/src/tests/utils.rs b/state-sync/state-sync-driver/src/tests/utils.rs index 7c934756b898d..71649dd0af976 100644 --- a/state-sync/state-sync-driver/src/tests/utils.rs +++ b/state-sync/state-sync-driver/src/tests/utils.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::driver::DriverConfiguration; -use aptos_config::config::{ObserverConfig, RoleType, StateSyncDriverConfig}; +use aptos_config::config::{ConsensusObserverConfig, RoleType, StateSyncDriverConfig}; use aptos_crypto::{ ed25519::{Ed25519PrivateKey, Ed25519Signature}, HashValue, PrivateKey, Uniform, @@ -84,7 +84,7 @@ pub fn create_event(event_key: Option) -> ContractEvent { /// Creates a test driver configuration for full nodes pub fn create_full_node_driver_configuration() -> DriverConfiguration { let config = StateSyncDriverConfig::default(); - let consensus_observer_config = ObserverConfig::default(); + let consensus_observer_config = ConsensusObserverConfig::default(); let role = RoleType::FullNode; let waypoint = Waypoint::default(); From 5cc6b5d7a27f75f99bca77fcb29d30ead69df264 Mon Sep 17 00:00:00 2001 From: Josh Lind Date: Sun, 2 Jun 2024 21:36:35 -0400 Subject: [PATCH 08/26] [Consensus Observer] Add metrics file, network counter and cleanups. --- aptos-node/src/lib.rs | 2 +- aptos-node/src/network.rs | 107 +++++++++------ aptos-node/src/services.rs | 13 +- .../src/config/consensus_observer_config.rs | 23 +++- consensus/src/consensus_observer/metrics.rs | 15 +++ consensus/src/consensus_observer/mod.rs | 1 + consensus/src/consensus_observer/observer.rs | 8 +- .../smoke-test/src/consensus_observer.rs | 86 ++++++++++++ testsuite/smoke-test/src/lib.rs | 4 + testsuite/smoke-test/src/state_sync.rs | 126 ++++-------------- testsuite/smoke-test/src/state_sync_utils.rs | 87 ++++++++++++ 11 files changed, 316 insertions(+), 156 deletions(-) create mode 100644 consensus/src/consensus_observer/metrics.rs create mode 100644 testsuite/smoke-test/src/consensus_observer.rs create mode 100644 testsuite/smoke-test/src/state_sync_utils.rs diff --git a/aptos-node/src/lib.rs b/aptos-node/src/lib.rs index 9d1b5beeb8654..0c751ef1b3157 100644 --- a/aptos-node/src/lib.rs +++ b/aptos-node/src/lib.rs @@ -649,12 +649,12 @@ pub fn setup_environment_and_start_node( let ( network_runtimes, consensus_network_interfaces, + consensus_observer_network_interfaces, dkg_network_interfaces, jwk_consensus_network_interfaces, mempool_network_interfaces, peer_monitoring_service_network_interfaces, storage_service_network_interfaces, - consensus_observer_network_interfaces, ) = network::setup_networks_and_get_interfaces( &node_config, chain_id, diff --git a/aptos-node/src/network.rs b/aptos-node/src/network.rs index 2131e8e660427..1629f2f96e54d 100644 --- a/aptos-node/src/network.rs +++ b/aptos-node/src/network.rs @@ -8,7 +8,8 @@ use aptos_config::{ network_id::NetworkId, }; use aptos_consensus::{ - consensus_observer::network::ObserverMessage, network_interface::ConsensusMsg, + consensus_observer, consensus_observer::network::ObserverMessage, + network_interface::ConsensusMsg, }; use aptos_dkg_runtime::DKGMessage; use aptos_event_notifications::EventSubscriptionService; @@ -70,6 +71,7 @@ pub fn consensus_network_configuration(node_config: &NodeConfig) -> NetworkAppli NetworkApplicationConfig::new(network_client_config, network_service_config) } +/// Returns the network application config for the DKG client and service pub fn dkg_network_configuration(node_config: &NodeConfig) -> NetworkApplicationConfig { let direct_send_protocols: Vec = aptos_dkg_runtime::network_interface::DIRECT_SEND.into(); @@ -86,6 +88,7 @@ pub fn dkg_network_configuration(node_config: &NodeConfig) -> NetworkApplication NetworkApplicationConfig::new(network_client_config, network_service_config) } +/// Returns the network application config for the JWK consensus client and service pub fn jwk_consensus_network_configuration(node_config: &NodeConfig) -> NetworkApplicationConfig { let direct_send_protocols: Vec = aptos_jwk_consensus::network_interface::DIRECT_SEND.into(); @@ -163,23 +166,27 @@ pub fn storage_service_network_configuration(node_config: &NodeConfig) -> Networ NetworkApplicationConfig::new(network_client_config, network_service_config) } -pub fn observer_service_network_configuration( - _node_config: &NodeConfig, +/// Returns the network application config for the consensus observer client and server +pub fn consensus_observer_network_configuration( + node_config: &NodeConfig, ) -> NetworkApplicationConfig { let direct_send_protocols = vec![ProtocolId::ConsensusObserver]; let rpc_protocols = vec![]; - let max_network_channel_size = 100; + let max_network_channel_size = node_config.consensus_observer.max_network_channel_size as usize; let network_client_config = NetworkClientConfig::new(direct_send_protocols.clone(), rpc_protocols.clone()); let network_service_config = NetworkServiceConfig::new( direct_send_protocols, rpc_protocols, - aptos_channel::Config::new(max_network_channel_size).queue_style(QueueStyle::FIFO), + aptos_channel::Config::new(max_network_channel_size) + .queue_style(QueueStyle::FIFO) + .counters(&consensus_observer::metrics::PENDING_CONSENSUS_OBSERVER_NETWORK_EVENTS), ); NetworkApplicationConfig::new(network_client_config, network_service_config) } +/// Returns the network application config for the netbench client and server pub fn netbench_network_configuration( node_config: &NodeConfig, ) -> Option { @@ -244,24 +251,24 @@ pub fn setup_networks_and_get_interfaces( ) -> ( Vec, Option>, + Option>, Option>, Option>, ApplicationNetworkInterfaces, ApplicationNetworkInterfaces, ApplicationNetworkInterfaces, - Option>, ) { // Gather all network configs let network_configs = extract_network_configs(node_config); - let observer_enabled = node_config.consensus_observer.publisher_enabled - || node_config.consensus_observer.observer_enabled; // Create each network and register the application handles let mut network_runtimes = vec![]; let mut consensus_network_handle = None; + let mut consensus_observer_network_handles: Option< + Vec>, + > = None; let mut dkg_network_handle = None; let mut jwk_consensus_network_handle = None; - let mut observer_network_handles = vec![]; let mut mempool_network_handles = vec![]; let mut peer_monitoring_service_network_handles = vec![]; let mut storage_service_network_handles = vec![]; @@ -290,37 +297,61 @@ pub fn setup_networks_and_get_interfaces( if consensus_network_handle.is_some() { panic!("There can be at most one validator network!"); } else { - consensus_network_handle = Some(register_client_and_service_with_network( + let network_handle = register_client_and_service_with_network( &mut network_builder, network_id, &network_config, consensus_network_configuration(node_config), false, - )); + ); + consensus_network_handle = Some(network_handle); } if dkg_network_handle.is_some() { panic!("There can be at most one validator network!"); } else { - dkg_network_handle = Some(register_client_and_service_with_network( + let network_handle = register_client_and_service_with_network( &mut network_builder, network_id, &network_config, dkg_network_configuration(node_config), false, - )); + ); + dkg_network_handle = Some(network_handle); } if jwk_consensus_network_handle.is_some() { panic!("There can be at most one validator network!"); } else { - jwk_consensus_network_handle = Some(register_client_and_service_with_network( + let network_handle = register_client_and_service_with_network( &mut network_builder, network_id, &network_config, jwk_consensus_network_configuration(node_config), false, - )); + ); + jwk_consensus_network_handle = Some(network_handle); + } + } + + // Register consensus observer (both client and server) with the network + if node_config.consensus_observer.is_enabled() { + // Create the network handle for this network type + let network_handle = register_client_and_service_with_network( + &mut network_builder, + network_id, + &network_config, + consensus_observer_network_configuration(node_config), + true, + ); + + // Add the network handle to the set of handles + if let Some(consensus_observer_network_handles) = + &mut consensus_observer_network_handles + { + consensus_observer_network_handles.push(network_handle); + } else { + consensus_observer_network_handles = Some(vec![network_handle]); } } @@ -354,18 +385,7 @@ pub fn setup_networks_and_get_interfaces( ); storage_service_network_handles.push(storage_service_network_handle); - if observer_enabled { - let observer_network_handle = register_client_and_service_with_network( - &mut network_builder, - network_id, - &network_config, - observer_service_network_configuration(node_config), - true, - ); - observer_network_handles.push(observer_network_handle); - } - - // Register benchmark test service + // Register the network benchmark test service if let Some(app_config) = netbench_network_configuration(node_config) { let netbench_handle = register_client_and_service_with_network( &mut network_builder, @@ -390,21 +410,21 @@ pub fn setup_networks_and_get_interfaces( // Transform all network handles into application interfaces let ( consensus_interfaces, + consensus_observer_interfaces, dkg_interfaces, jwk_consensus_interfaces, mempool_interfaces, peer_monitoring_service_interfaces, storage_service_interfaces, - observer_interfaces, ) = transform_network_handles_into_interfaces( node_config, consensus_network_handle, + consensus_observer_network_handles, dkg_network_handle, jwk_consensus_network_handle, mempool_network_handles, peer_monitoring_service_network_handles, storage_service_network_handles, - observer_network_handles, peers_and_metadata.clone(), ); @@ -421,20 +441,15 @@ pub fn setup_networks_and_get_interfaces( network_runtimes.push(netbench_runtime); } - let observer_interfaces = if observer_enabled { - Some(observer_interfaces) - } else { - None - }; ( network_runtimes, consensus_interfaces, + consensus_observer_interfaces, dkg_interfaces, jwk_consensus_interfaces, mempool_interfaces, peer_monitoring_service_interfaces, storage_service_interfaces, - observer_interfaces, ) } @@ -480,6 +495,7 @@ fn register_client_and_service_with_network< fn transform_network_handles_into_interfaces( node_config: &NodeConfig, consensus_network_handle: Option>, + consensus_observer_network_handles: Option>>, dkg_network_handle: Option>, jwk_consensus_network_handle: Option>, mempool_network_handles: Vec>, @@ -487,16 +503,15 @@ fn transform_network_handles_into_interfaces( ApplicationNetworkHandle, >, storage_service_network_handles: Vec>, - observer_network_handles: Vec>, peers_and_metadata: Arc, ) -> ( Option>, + Option>, Option>, Option>, ApplicationNetworkInterfaces, ApplicationNetworkInterfaces, ApplicationNetworkInterfaces, - ApplicationNetworkInterfaces, ) { let consensus_interfaces = consensus_network_handle.map(|consensus_network_handle| { create_network_interfaces( @@ -506,6 +521,15 @@ fn transform_network_handles_into_interfaces( ) }); + let consensus_observer_interfaces = + consensus_observer_network_handles.map(|consensus_observer_network_handles| { + create_network_interfaces( + consensus_observer_network_handles, + consensus_observer_network_configuration(node_config), + peers_and_metadata.clone(), + ) + }); + let dkg_interfaces = dkg_network_handle.map(|handle| { create_network_interfaces( vec![handle], @@ -527,30 +551,27 @@ fn transform_network_handles_into_interfaces( mempool_network_configuration(node_config), peers_and_metadata.clone(), ); + let peer_monitoring_service_interfaces = create_network_interfaces( peer_monitoring_service_network_handles, peer_monitoring_network_configuration(node_config), peers_and_metadata.clone(), ); + let storage_service_interfaces = create_network_interfaces( storage_service_network_handles, storage_service_network_configuration(node_config), peers_and_metadata.clone(), ); - let observer_interfaces = create_network_interfaces( - observer_network_handles, - observer_service_network_configuration(node_config), - peers_and_metadata, - ); ( consensus_interfaces, + consensus_observer_interfaces, dkg_interfaces, jwk_consensus_interfaces, mempool_interfaces, peer_monitoring_service_interfaces, storage_service_interfaces, - observer_interfaces, ) } diff --git a/aptos-node/src/services.rs b/aptos-node/src/services.rs index cea0de1b84963..bcd35a0fdd251 100644 --- a/aptos-node/src/services.rs +++ b/aptos-node/src/services.rs @@ -121,11 +121,10 @@ pub fn start_consensus_runtime( observer_network_client: Option>, ) -> (Runtime, Arc, Arc) { let instant = Instant::now(); - let observer_network_client = if node_config.consensus_observer.publisher_enabled { - observer_network_client - } else { - None - }; + + let reconfig_subscription = consensus_reconfig_subscription + .expect("Consensus requires a reconfiguration subscription!"); + let consensus = aptos_consensus::consensus_provider::start_consensus( node_config, consensus_network_interfaces.network_client, @@ -133,12 +132,12 @@ pub fn start_consensus_runtime( Arc::new(consensus_notifier), consensus_to_mempool_sender, db_rw, - consensus_reconfig_subscription - .expect("Consensus requires a reconfiguration subscription!"), + reconfig_subscription, vtxn_pool, observer_network_client, ); debug!("Consensus started in {} ms", instant.elapsed().as_millis()); + consensus } diff --git a/config/src/config/consensus_observer_config.rs b/config/src/config/consensus_observer_config.rs index 6315a3f0e4cd7..92bfddd0bf338 100644 --- a/config/src/config/consensus_observer_config.rs +++ b/config/src/config/consensus_observer_config.rs @@ -3,9 +3,30 @@ use serde::{Deserialize, Serialize}; -#[derive(Default, Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] #[serde(default, deny_unknown_fields)] pub struct ConsensusObserverConfig { + /// Whether the consensus observer is enabled pub observer_enabled: bool, + /// Whether the consensus observer publisher is enabled pub publisher_enabled: bool, + /// Maximum number of pending network messages + pub max_network_channel_size: u64, +} + +impl Default for ConsensusObserverConfig { + fn default() -> Self { + Self { + observer_enabled: false, + publisher_enabled: false, + max_network_channel_size: 1000, + } + } +} + +impl ConsensusObserverConfig { + /// Returns true iff the observer or publisher is enabled + pub fn is_enabled(&self) -> bool { + self.observer_enabled || self.publisher_enabled + } } diff --git a/consensus/src/consensus_observer/metrics.rs b/consensus/src/consensus_observer/metrics.rs new file mode 100644 index 0000000000000..c493dfa84960e --- /dev/null +++ b/consensus/src/consensus_observer/metrics.rs @@ -0,0 +1,15 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use aptos_metrics_core::{register_int_counter_vec, IntCounterVec}; +use once_cell::sync::Lazy; + +/// Counter for pending network events to the consensus observer +pub static PENDING_CONSENSUS_OBSERVER_NETWORK_EVENTS: Lazy = Lazy::new(|| { + register_int_counter_vec!( + "aptos_consensus_observer_pending_network_events", + "Counters for pending network events for consensus observer", + &["state"] + ) + .unwrap() +}); diff --git a/consensus/src/consensus_observer/mod.rs b/consensus/src/consensus_observer/mod.rs index ec2ef3af57548..44056da869c46 100644 --- a/consensus/src/consensus_observer/mod.rs +++ b/consensus/src/consensus_observer/mod.rs @@ -1,6 +1,7 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 +pub mod metrics; pub mod network; pub mod observer; pub mod publisher; diff --git a/consensus/src/consensus_observer/observer.rs b/consensus/src/consensus_observer/observer.rs index 3ae03d2eb991a..5c8c1000bd489 100644 --- a/consensus/src/consensus_observer/observer.rs +++ b/consensus/src/consensus_observer/observer.rs @@ -18,7 +18,7 @@ use aptos_consensus_types::pipeline::commit_decision::CommitDecision; use aptos_crypto::{bls12381, Genesis, HashValue}; use aptos_event_notifications::{DbBackedOnChainConfig, ReconfigNotificationListener}; use aptos_infallible::Mutex; -use aptos_logger::{error, info}; +use aptos_logger::{debug, error, info}; use aptos_network::protocols::{network::Event, wire::handshake::v1::ProtocolId}; use aptos_reliable_broadcast::DropGuard; use aptos_types::{ @@ -226,6 +226,10 @@ impl Observer { } async fn process_sync_notify(&mut self, epoch: u64, round: Round) { + info!( + "[Observer] Received sync notify to epoch {}, round: {}", + epoch, round + ); { let lock = self.root.lock(); let expected = (lock.commit_info().epoch(), lock.commit_info().round()); @@ -366,6 +370,8 @@ impl Observer { } } } + } else { + debug!("[Observer] Received untracked event: {:?}", event); } }, Some((epoch, round)) = notifier_rx.recv() => { diff --git a/testsuite/smoke-test/src/consensus_observer.rs b/testsuite/smoke-test/src/consensus_observer.rs new file mode 100644 index 0000000000000..8c0d856e53539 --- /dev/null +++ b/testsuite/smoke-test/src/consensus_observer.rs @@ -0,0 +1,86 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use crate::{ + smoke_test_environment::SwarmBuilder, + state_sync_utils, + test_utils::{create_test_accounts, execute_transactions, wait_for_all_nodes}, +}; +use aptos_config::config::NodeConfig; +use aptos_forge::NodeExt; +use std::sync::Arc; + +#[tokio::test] +async fn test_consensus_observer_fullnode_simple_sync() { + // Create a validator swarm of 1 validator with consensus publisher enabled + let mut swarm = SwarmBuilder::new_local(1) + .with_aptos() + .with_init_config(Arc::new(|_, config, _| { + config.consensus_observer.publisher_enabled = true; + })) + .build() + .await; + + // Create a fullnode config that uses consensus observer + let mut vfn_config = NodeConfig::get_default_vfn_config(); + vfn_config.consensus_observer.observer_enabled = true; + + // Create the fullnode + let _ = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + + // Execute a number of transactions on the validator + let validator_peer_id = swarm.validators().next().unwrap().peer_id(); + let validator_client = swarm.validator(validator_peer_id).unwrap().rest_client(); + let (mut account_0, account_1) = create_test_accounts(&mut swarm).await; + execute_transactions( + &mut swarm, + &validator_client, + &mut account_0, + &account_1, + false, + ) + .await; + + // Verify the fullnode is up-to-date + wait_for_all_nodes(&mut swarm).await; +} + +#[tokio::test] +async fn test_consensus_observer_fullnode_restart() { + // Create a validator swarm of 1 validator with consensus publisher enabled + let mut swarm = SwarmBuilder::new_local(1) + .with_aptos() + .with_init_config(Arc::new(|_, config, _| { + config.consensus_observer.publisher_enabled = true; + })) + .build() + .await; + + // Create a fullnode config that uses consensus observer + let mut vfn_config = NodeConfig::get_default_vfn_config(); + vfn_config.consensus_observer.observer_enabled = true; + + // Create the fullnode and test its ability to stay up-to-date + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; +} + +#[tokio::test] +async fn test_consensus_observer_fullnode_restart_wipe() { + // Create a validator swarm of 1 validator with consensus publisher enabled + let mut swarm = SwarmBuilder::new_local(1) + .with_aptos() + .with_init_config(Arc::new(|_, config, _| { + config.consensus_observer.publisher_enabled = true; + })) + .build() + .await; + + // Create a fullnode config that uses consensus observer + let mut vfn_config = NodeConfig::get_default_vfn_config(); + vfn_config.consensus_observer.observer_enabled = true; + + // Create the fullnode and test its ability to stay up-to-date (after a data wipe) + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, true).await; +} diff --git a/testsuite/smoke-test/src/lib.rs b/testsuite/smoke-test/src/lib.rs index 1bc84980668a5..e91884589a911 100644 --- a/testsuite/smoke-test/src/lib.rs +++ b/testsuite/smoke-test/src/lib.rs @@ -13,6 +13,8 @@ mod client; #[cfg(test)] mod consensus; #[cfg(test)] +mod consensus_observer; +#[cfg(test)] mod execution; #[cfg(test)] mod full_nodes; @@ -39,6 +41,8 @@ mod rosetta; #[cfg(test)] mod state_sync; #[cfg(test)] +mod state_sync_utils; +#[cfg(test)] mod storage; #[cfg(test)] mod test_smoke_tests; diff --git a/testsuite/smoke-test/src/state_sync.rs b/testsuite/smoke-test/src/state_sync.rs index 92dc5929c9fc3..ce8d4381b5818 100644 --- a/testsuite/smoke-test/src/state_sync.rs +++ b/testsuite/smoke-test/src/state_sync.rs @@ -4,16 +4,15 @@ use crate::{ smoke_test_environment::{new_local_swarm_with_aptos, SwarmBuilder}, + state_sync_utils, test_utils::{ create_test_accounts, execute_transactions, execute_transactions_and_wait, - wait_for_all_nodes, MAX_CATCH_UP_WAIT_SECS, MAX_HEALTHY_WAIT_SECS, + wait_for_all_nodes, MAX_CATCH_UP_WAIT_SECS, }, }; -use aptos_config::config::{ - BootstrappingMode, ContinuousSyncingMode, NodeConfig, OverrideNodeConfig, -}; +use aptos_config::config::{BootstrappingMode, ContinuousSyncingMode, NodeConfig}; use aptos_db::AptosDB; -use aptos_forge::{LocalNode, LocalSwarm, Node, NodeExt, Swarm}; +use aptos_forge::{LocalNode, LocalSwarm, Node, NodeExt}; use aptos_inspection_service::inspection_client::InspectionClient; use aptos_rest_client::Client as RestClient; use aptos_storage_interface::DbReader; @@ -23,12 +22,8 @@ use aptos_types::{ ConsensusConfigV1, LeaderReputationType, OnChainConsensusConfig, ProposerAndVoterConfig, ProposerElectionType, }, - PeerId, -}; -use std::{ - sync::Arc, - time::{Duration, Instant}, }; +use std::{sync::Arc, time::Duration}; // TODO: Go through the existing tests, identify any gaps, and clean up the rest. @@ -60,8 +55,8 @@ async fn test_fullnode_output_sync_epoch_changes() { vfn_config.state_sync.aptos_data_client.use_compression = true; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; } #[tokio::test] @@ -80,8 +75,8 @@ async fn test_fullnode_output_sync_no_compression() { vfn_config.state_sync.aptos_data_client.use_compression = false; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; } #[tokio::test] @@ -100,8 +95,8 @@ async fn test_fullnode_output_sync_exponential_backoff() { vfn_config.state_sync.aptos_data_client.response_timeout_ms = 1; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; } #[tokio::test] @@ -130,8 +125,8 @@ async fn test_fullnode_intelligent_sync_epoch_changes() { vfn_config.state_sync.aptos_data_client.response_timeout_ms = 1; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; } #[tokio::test] @@ -160,8 +155,8 @@ async fn test_fullnode_fast_and_intelligent_sync_epoch_changes() { vfn_config.state_sync.aptos_data_client.response_timeout_ms = 1; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; } #[tokio::test] @@ -180,8 +175,8 @@ async fn test_fullnode_execution_sync_epoch_changes() { vfn_config.state_sync.aptos_data_client.use_compression = true; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, true, false).await; } #[tokio::test] @@ -197,8 +192,8 @@ async fn test_fullnode_output_sync_no_epoch_changes() { .continuous_syncing_mode = ContinuousSyncingMode::ApplyTransactionOutputs; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, false, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, false, false).await; } #[tokio::test] @@ -214,8 +209,8 @@ async fn test_fullnode_execution_sync_no_epoch_changes() { .continuous_syncing_mode = ContinuousSyncingMode::ExecuteTransactions; // Create the fullnode and test its ability to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, false, false).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, false, false).await; } #[tokio::test] @@ -570,25 +565,6 @@ async fn test_all_validators_fast_and_execution_sync() { test_all_validator_failures(swarm).await; } -/// Creates a new full node using the given config and swarm -async fn create_fullnode(full_node_config: NodeConfig, swarm: &mut LocalSwarm) -> PeerId { - let validator_peer_id = swarm.validators().next().unwrap().peer_id(); - let vfn_peer_id = swarm - .add_validator_fullnode( - &swarm.versions().max().unwrap(), - OverrideNodeConfig::new_with_default_base(full_node_config), - validator_peer_id, - ) - .unwrap(); - for fullnode in swarm.full_nodes_mut() { - fullnode - .wait_until_healthy(Instant::now() + Duration::from_secs(MAX_HEALTHY_WAIT_SECS)) - .await - .unwrap(); - } - vfn_peer_id -} - /// A test method that verifies that a fullnode can fast sync from /// a validator after a data wipe. If `epoch_changes` are enabled /// then epoch changes can occur during test execution and fullnode syncing. @@ -617,8 +593,8 @@ async fn test_fullnode_fast_sync(epoch_changes: bool) { BootstrappingMode::DownloadLatestStates; // Test the ability of a fullnode to sync - let vfn_peer_id = create_fullnode(vfn_config, &mut swarm).await; - test_fullnode_sync(vfn_peer_id, &mut swarm, epoch_changes, true).await; + let vfn_peer_id = state_sync_utils::create_fullnode(vfn_config, &mut swarm).await; + state_sync_utils::test_fullnode_sync(vfn_peer_id, &mut swarm, epoch_changes, true).await; // Verify the oldest ledger info and pruning metrics for the fullnode if epoch_changes { @@ -653,54 +629,6 @@ async fn test_validator_sync_exponential_backoff(epoch_changes: bool) { test_validator_sync(&mut swarm, 1, epoch_changes).await; } -/// A helper method that tests that a full node can sync from a validator after -/// a failure and continue to stay up-to-date. -async fn test_fullnode_sync( - vfn_peer_id: PeerId, - swarm: &mut LocalSwarm, - epoch_changes: bool, - clear_storage: bool, -) { - // Stop the fullnode and potentially clear storage - if clear_storage { - stop_fullnode_and_delete_storage(swarm, vfn_peer_id).await; - } else { - swarm.fullnode_mut(vfn_peer_id).unwrap().stop(); - } - - // Execute a number of transactions on the validator - let validator_peer_id = swarm.validators().next().unwrap().peer_id(); - let validator_client = swarm.validator(validator_peer_id).unwrap().rest_client(); - let (mut account_0, mut account_1) = create_test_accounts(swarm).await; - execute_transactions( - swarm, - &validator_client, - &mut account_0, - &account_1, - epoch_changes, - ) - .await; - - // Restart the fullnode and verify it can sync - swarm - .fullnode_mut(vfn_peer_id) - .unwrap() - .restart() - .await - .unwrap(); - wait_for_all_nodes(swarm).await; - - // Execute more transactions on the validator and verify the fullnode catches up - execute_transactions_and_wait( - swarm, - &validator_client, - &mut account_1, - &account_0, - epoch_changes, - ) - .await; -} - /// A helper method that tests that a validator can sync after a failure and /// continue to stay up-to-date. async fn test_validator_sync( @@ -914,14 +842,6 @@ pub async fn test_all_validator_failures(mut swarm: LocalSwarm) { .await; } -/// Stops the specified fullnode and deletes storage -async fn stop_fullnode_and_delete_storage(swarm: &mut LocalSwarm, fullnode: AccountAddress) { - let fullnode = swarm.full_node_mut(fullnode).unwrap(); - - // The fullnode is stopped during the clear_storage() call - fullnode.clear_storage().await.unwrap(); -} - /// Stops the specified validator and deletes storage async fn stop_validator_and_delete_storage(swarm: &mut LocalSwarm, validator: AccountAddress) { let validator = swarm.validator_mut(validator).unwrap(); diff --git a/testsuite/smoke-test/src/state_sync_utils.rs b/testsuite/smoke-test/src/state_sync_utils.rs new file mode 100644 index 0000000000000..ae10d9d2aa603 --- /dev/null +++ b/testsuite/smoke-test/src/state_sync_utils.rs @@ -0,0 +1,87 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +use crate::test_utils::{ + create_test_accounts, execute_transactions, execute_transactions_and_wait, wait_for_all_nodes, + MAX_HEALTHY_WAIT_SECS, +}; +use aptos_config::config::{NodeConfig, OverrideNodeConfig}; +use aptos_forge::{LocalSwarm, NodeExt, Swarm}; +use aptos_sdk::types::PeerId; +use move_core_types::account_address::AccountAddress; +use std::time::{Duration, Instant}; + +/// Creates a new full node using the given config and swarm +pub async fn create_fullnode(full_node_config: NodeConfig, swarm: &mut LocalSwarm) -> PeerId { + let validator_peer_id = swarm.validators().next().unwrap().peer_id(); + let vfn_peer_id = swarm + .add_validator_fullnode( + &swarm.versions().max().unwrap(), + OverrideNodeConfig::new_with_default_base(full_node_config), + validator_peer_id, + ) + .unwrap(); + for fullnode in swarm.full_nodes_mut() { + fullnode + .wait_until_healthy(Instant::now() + Duration::from_secs(MAX_HEALTHY_WAIT_SECS)) + .await + .unwrap(); + } + vfn_peer_id +} + +/// Stops the specified fullnode and deletes storage +pub async fn stop_fullnode_and_delete_storage(swarm: &mut LocalSwarm, fullnode: AccountAddress) { + let fullnode = swarm.full_node_mut(fullnode).unwrap(); + + // The fullnode is stopped during the clear_storage() call + fullnode.clear_storage().await.unwrap(); +} + +/// A helper method that tests that a full node can sync from a validator after +/// a failure and continue to stay up-to-date. +pub async fn test_fullnode_sync( + vfn_peer_id: PeerId, + swarm: &mut LocalSwarm, + epoch_changes: bool, + clear_storage: bool, +) { + // Stop the fullnode and potentially clear storage + if clear_storage { + stop_fullnode_and_delete_storage(swarm, vfn_peer_id).await; + } else { + swarm.fullnode_mut(vfn_peer_id).unwrap().stop(); + } + + // Execute a number of transactions on the validator + let validator_peer_id = swarm.validators().next().unwrap().peer_id(); + let validator_client = swarm.validator(validator_peer_id).unwrap().rest_client(); + let (mut account_0, mut account_1) = create_test_accounts(swarm).await; + execute_transactions( + swarm, + &validator_client, + &mut account_0, + &account_1, + epoch_changes, + ) + .await; + + // Restart the fullnode and verify it can sync + swarm + .fullnode_mut(vfn_peer_id) + .unwrap() + .restart() + .await + .unwrap(); + wait_for_all_nodes(swarm).await; + + // Execute more transactions on the validator and verify the fullnode catches up + execute_transactions_and_wait( + swarm, + &validator_client, + &mut account_1, + &account_0, + epoch_changes, + ) + .await; +} From 8e0b30711ac1efd692805bde51032ac1baead7eb Mon Sep 17 00:00:00 2001 From: Josh Lind Date: Mon, 3 Jun 2024 00:04:45 -0400 Subject: [PATCH 09/26] [Consensus Observer] Add config optimizer support for CO. --- config/src/config/config_optimizer.rs | 13 +++++++++---- config/src/config/consensus_observer_config.rs | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/config/src/config/config_optimizer.rs b/config/src/config/config_optimizer.rs index 8947a4c7644c6..addd1087b1435 100644 --- a/config/src/config/config_optimizer.rs +++ b/config/src/config/config_optimizer.rs @@ -1,7 +1,9 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use super::{Identity, IdentityFromConfig, IdentitySource, IndexerGrpcConfig}; +use super::{ + ConsensusObserverConfig, Identity, IdentityFromConfig, IdentitySource, IndexerGrpcConfig, +}; use crate::{ config::{ node_config_loader::NodeType, utils::get_config_name, AdminServiceConfig, Error, @@ -105,6 +107,12 @@ impl ConfigOptimizer for NodeConfig { // Optimize only the relevant sub-configs let mut optimizers_with_modifications = vec![]; + if AdminServiceConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? { + optimizers_with_modifications.push(AdminServiceConfig::get_optimizer_name()); + } + if ConsensusObserverConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? { + optimizers_with_modifications.push(ConsensusObserverConfig::get_optimizer_name()); + } if ExecutionConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? { optimizers_with_modifications.push(ExecutionConfig::get_optimizer_name()); } @@ -114,9 +122,6 @@ impl ConfigOptimizer for NodeConfig { if IndexerGrpcConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? { optimizers_with_modifications.push(IndexerGrpcConfig::get_optimizer_name()); } - if AdminServiceConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? { - optimizers_with_modifications.push(AdminServiceConfig::get_optimizer_name()); - } if InspectionServiceConfig::optimize(node_config, local_config_yaml, node_type, chain_id)? { optimizers_with_modifications.push(InspectionServiceConfig::get_optimizer_name()); } diff --git a/config/src/config/consensus_observer_config.rs b/config/src/config/consensus_observer_config.rs index 92bfddd0bf338..8da6ccefddbd4 100644 --- a/config/src/config/consensus_observer_config.rs +++ b/config/src/config/consensus_observer_config.rs @@ -1,7 +1,12 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 +use crate::config::{ + config_optimizer::ConfigOptimizer, node_config_loader::NodeType, Error, NodeConfig, +}; +use aptos_types::chain_id::ChainId; use serde::{Deserialize, Serialize}; +use serde_yaml::Value; #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] #[serde(default, deny_unknown_fields)] @@ -30,3 +35,16 @@ impl ConsensusObserverConfig { self.observer_enabled || self.publisher_enabled } } + +impl ConfigOptimizer for ConsensusObserverConfig { + fn optimize( + _node_config: &mut NodeConfig, + _local_config_yaml: &Value, + _node_type: NodeType, + _chain_id: Option, + ) -> Result { + // TODO: use me to enable consensus observer for validators and VFNs + // in controlled environments, e.g., devnet. + Ok(false) + } +} From 10a6611544e3a68ba43680fb71493f71977ce6ae Mon Sep 17 00:00:00 2001 From: "Brian (Sunghoon) Cho" Date: Tue, 4 Jun 2024 18:44:21 -0700 Subject: [PATCH 10/26] [forge] reduce mempool backlog as haproxy is slower (#13562) --- testsuite/forge-cli/src/main.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/testsuite/forge-cli/src/main.rs b/testsuite/forge-cli/src/main.rs index 74c23f0fc7780..89d134fd69f4b 100644 --- a/testsuite/forge-cli/src/main.rs +++ b/testsuite/forge-cli/src/main.rs @@ -1979,14 +1979,13 @@ fn realistic_env_max_load_test( } // Create the test + let mempool_backlog = if ha_proxy { 30000 } else { 40000 }; ForgeConfig::default() .with_initial_validator_count(NonZeroUsize::new(num_validators).unwrap()) .with_initial_fullnode_count(num_fullnodes) .add_network_test(wrap_with_realistic_env(TwoTrafficsTest { inner_traffic: EmitJobRequest::default() - .mode(EmitJobMode::MaxLoad { - mempool_backlog: 40000, - }) + .mode(EmitJobMode::MaxLoad { mempool_backlog }) .init_gas_price_multiplier(20), inner_success_criteria: SuccessCriteria::new( if ha_proxy { From 2a458b5ffaaf6a9de6fac679a53912c0be9fe217 Mon Sep 17 00:00:00 2001 From: Jianyi <129789810+0xaptosj@users.noreply.github.com> Date: Wed, 5 Jun 2024 10:42:49 +0800 Subject: [PATCH 11/26] add getter for icon uri and project uri in FA (#13565) --- .../aptos-framework/doc/fungible_asset.md | 54 +++++++++++++++++++ .../sources/fungible_asset.move | 18 ++++++- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/aptos-move/framework/aptos-framework/doc/fungible_asset.md b/aptos-move/framework/aptos-framework/doc/fungible_asset.md index 53750d9b2b51b..b3e4e9c3c1861 100644 --- a/aptos-move/framework/aptos-framework/doc/fungible_asset.md +++ b/aptos-move/framework/aptos-framework/doc/fungible_asset.md @@ -41,6 +41,8 @@ metadata object can be any object that equipped with + +## Function `icon_uri` + +Get the icon uri from the metadata object. + + +
#[view]
+public fun icon_uri<T: key>(metadata: object::Object<T>): string::String
+
+ + + +
+Implementation + + +
public fun icon_uri<T: key>(metadata: Object<T>): String acquires Metadata {
+    borrow_fungible_metadata(&metadata).icon_uri
+}
+
+ + + +
+ + + +## Function `project_uri` + +Get the project uri from the metadata object. + + +
#[view]
+public fun project_uri<T: key>(metadata: object::Object<T>): string::String
+
+ + + +
+Implementation + + +
public fun project_uri<T: key>(metadata: Object<T>): String acquires Metadata {
+    borrow_fungible_metadata(&metadata).project_uri
+}
+
+ + +
diff --git a/aptos-move/framework/aptos-framework/sources/fungible_asset.move b/aptos-move/framework/aptos-framework/sources/fungible_asset.move index d29ecb0f7b832..a629fb29acd1b 100644 --- a/aptos-move/framework/aptos-framework/sources/fungible_asset.move +++ b/aptos-move/framework/aptos-framework/sources/fungible_asset.move @@ -455,6 +455,18 @@ module aptos_framework::fungible_asset { borrow_fungible_metadata(&metadata).decimals } + #[view] + /// Get the icon uri from the `metadata` object. + public fun icon_uri(metadata: Object): String acquires Metadata { + borrow_fungible_metadata(&metadata).icon_uri + } + + #[view] + /// Get the project uri from the `metadata` object. + public fun project_uri(metadata: Object): String acquires Metadata { + borrow_fungible_metadata(&metadata).project_uri + } + #[view] /// Return whether the provided address has a store initialized. public fun store_exists(store: address): bool { @@ -1112,11 +1124,13 @@ module aptos_framework::fungible_asset { assert!(name(metadata) == string::utf8(b"TEST"), 3); assert!(symbol(metadata) == string::utf8(b"@@"), 4); assert!(decimals(metadata) == 0, 5); + assert!(icon_uri(metadata) == string::utf8(b"http://www.example.com/favicon.ico"), 6); + assert!(project_uri(metadata) == string::utf8(b"http://www.example.com"), 7); increase_supply(&metadata, 50); - assert!(supply(metadata) == option::some(50), 6); + assert!(supply(metadata) == option::some(50), 8); decrease_supply(&metadata, 30); - assert!(supply(metadata) == option::some(20), 7); + assert!(supply(metadata) == option::some(20), 9); } #[test(creator = @0xcafe)] From af896e34577014cc6254c321fcec4574ccd47039 Mon Sep 17 00:00:00 2001 From: Greg Nazario Date: Wed, 5 Jun 2024 19:14:29 +0700 Subject: [PATCH 12/26] [api] Update max page size for blocks to be receiving block size (#13547) --- config/src/config/api_config.rs | 4 ++-- config/src/config/consensus_config.rs | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/config/src/config/api_config.rs b/config/src/config/api_config.rs index 65e202a457cf1..1758d8c227994 100644 --- a/config/src/config/api_config.rs +++ b/config/src/config/api_config.rs @@ -6,7 +6,7 @@ use super::transaction_filter_type::{Filter, Matcher}; use crate::{ config::{ config_sanitizer::ConfigSanitizer, gas_estimation_config::GasEstimationConfig, - node_config_loader::NodeType, Error, NodeConfig, MAX_SENDING_BLOCK_TXNS, + node_config_loader::NodeType, Error, NodeConfig, MAX_RECEIVING_BLOCK_TXNS, }, utils, }; @@ -125,7 +125,7 @@ impl Default for ApiConfig { transaction_submission_enabled: default_enabled(), transaction_simulation_enabled: default_enabled(), max_submit_transaction_batch_size: DEFAULT_MAX_SUBMIT_TRANSACTION_BATCH_SIZE, - max_block_transactions_page_size: MAX_SENDING_BLOCK_TXNS as u16, + max_block_transactions_page_size: *MAX_RECEIVING_BLOCK_TXNS as u16, max_transactions_page_size: DEFAULT_MAX_PAGE_SIZE, max_events_page_size: DEFAULT_MAX_PAGE_SIZE, max_account_resources_page_size: DEFAULT_MAX_ACCOUNT_RESOURCES_PAGE_SIZE, diff --git a/config/src/config/consensus_config.rs b/config/src/config/consensus_config.rs index 240c108db5075..501a80c73f852 100644 --- a/config/src/config/consensus_config.rs +++ b/config/src/config/consensus_config.rs @@ -6,13 +6,16 @@ use crate::config::{ config_sanitizer::ConfigSanitizer, node_config_loader::NodeType, Error, NodeConfig, QuorumStoreConfig, ReliableBroadcastConfig, SafetyRulesConfig, BATCH_PADDING_BYTES, }; +use aptos_crypto::_once_cell::sync::Lazy; use aptos_types::chain_id::ChainId; use cfg_if::cfg_if; use serde::{Deserialize, Serialize}; use std::path::PathBuf; // NOTE: when changing, make sure to update QuorumStoreBackPressureConfig::backlog_txn_limit_count as well. -pub(crate) const MAX_SENDING_BLOCK_TXNS: u64 = 1900; +const MAX_SENDING_BLOCK_TXNS: u64 = 1900; +pub(crate) static MAX_RECEIVING_BLOCK_TXNS: Lazy = + Lazy::new(|| 10000.max(2 * MAX_SENDING_BLOCK_TXNS)); // stop reducing size at this point, so 1MB transactions can still go through const MIN_BLOCK_BYTES_OVERRIDE: u64 = 1024 * 1024 + BATCH_PADDING_BYTES as u64; @@ -151,7 +154,7 @@ impl Default for ConsensusConfig { max_network_channel_size: 1024, max_sending_block_txns: MAX_SENDING_BLOCK_TXNS, max_sending_block_bytes: 3 * 1024 * 1024, // 3MB - max_receiving_block_txns: 10000.max(2 * MAX_SENDING_BLOCK_TXNS), + max_receiving_block_txns: *MAX_RECEIVING_BLOCK_TXNS, max_sending_inline_txns: 100, max_sending_inline_bytes: 200 * 1024, // 200 KB max_receiving_block_bytes: 6 * 1024 * 1024, // 6MB From 6e4a3f9aeeb6d3afab3f3ae9efc625a008382223 Mon Sep 17 00:00:00 2001 From: JinLiang Guo <33477878+jlguochn@users.noreply.github.com> Date: Thu, 6 Jun 2024 00:41:32 +0800 Subject: [PATCH 13/26] fix:Correct ECDSA signature malleability handling (#13544) * Correct ECDSA signature malleability handling * Rename function check_s_lt_order_half to check_s_le_order_half --- .../src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs | 10 +++++----- .../src/unit_tests/secp256r1_ecdsa_test.rs | 10 ++++------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs b/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs index 28cfb76105170..40b5c230b5eed 100644 --- a/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs +++ b/crates/aptos-crypto/src/secp256r1_ecdsa/secp256r1_ecdsa_sigs.rs @@ -65,14 +65,14 @@ impl Signature { if bytes.len() != SIGNATURE_LENGTH { return Err(CryptoMaterialError::WrongLengthError); } - if !Signature::check_s_lt_order_half(&bytes[32..]) { + if !Signature::check_s_le_order_half(&bytes[32..]) { return Err(CryptoMaterialError::CanonicalRepresentationError); } Ok(()) } - /// Check if S < ORDER_HALF to capture invalid signatures. - fn check_s_lt_order_half(s: &[u8]) -> bool { + /// Check if S <= ORDER_HALF to capture invalid signatures. + fn check_s_le_order_half(s: &[u8]) -> bool { for i in 0..32 { match s[i].cmp(&ORDER_HALF[i]) { Ordering::Less => return true, @@ -80,8 +80,8 @@ impl Signature { _ => {}, } } - // At this stage S == ORDER_HALF which implies a non canonical S. - false + // At this stage S == ORDER_HALF , it is still considered a canonical value. + true } /// If the signature {R,S} does not have S < n/2 where n is the Ristretto255 order, return diff --git a/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs b/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs index 79d5d32fddf95..b4581a54ae4d9 100644 --- a/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs +++ b/crates/aptos-crypto/src/unit_tests/secp256r1_ecdsa_test.rs @@ -208,13 +208,11 @@ proptest! { let sig_unchecked = Signature::from_bytes_unchecked(&serialized); prop_assert!(sig_unchecked.is_ok()); - // Update the signature by setting S = L to make it invalid. + // S = ORDER_HALF should be a canonical signature. serialized[32..].copy_from_slice(&ORDER_HALF); - let serialized_malleable_l: &[u8] = &serialized; - // try_from will fail with CanonicalRepresentationError. - prop_assert_eq!( - Signature::try_from(serialized_malleable_l), - Err(CryptoMaterialError::CanonicalRepresentationError) + let canonical: &[u8] = &serialized; + prop_assert!( + Signature::try_from(canonical).is_ok() ); } } From 01f3324b49313d2e849a651ef065b2fb7dcd8486 Mon Sep 17 00:00:00 2001 From: "Daniel Porteous (dport)" Date: Wed, 5 Jun 2024 18:50:12 +0100 Subject: [PATCH 14/26] [data service] Add support for configuring in memory cache size (#13570) --- .../indexer-grpc-data-service/src/config.rs | 9 ++- .../src/main.rs | 3 +- .../indexer-grpc-utils/src/in_memory_cache.rs | 71 +++++++++++++++++-- 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/ecosystem/indexer-grpc/indexer-grpc-data-service/src/config.rs b/ecosystem/indexer-grpc/indexer-grpc-data-service/src/config.rs index 71460c361f1fa..6824343676579 100644 --- a/ecosystem/indexer-grpc/indexer-grpc-data-service/src/config.rs +++ b/ecosystem/indexer-grpc/indexer-grpc-data-service/src/config.rs @@ -5,7 +5,8 @@ use crate::service::RawDataServerWrapper; use anyhow::{bail, Result}; use aptos_indexer_grpc_server_framework::RunnableConfig; use aptos_indexer_grpc_utils::{ - compression_util::StorageFormat, config::IndexerGrpcFileStoreConfig, types::RedisUrl, + compression_util::StorageFormat, config::IndexerGrpcFileStoreConfig, + in_memory_cache::InMemoryCacheConfig, types::RedisUrl, }; use aptos_protos::{ indexer::v1::FILE_DESCRIPTOR_SET as INDEXER_V1_FILE_DESCRIPTOR_SET, @@ -66,6 +67,8 @@ pub struct IndexerGrpcDataServiceConfig { /// Support compressed cache data. #[serde(default = "IndexerGrpcDataServiceConfig::default_enable_cache_compression")] pub enable_cache_compression: bool, + #[serde(default)] + pub in_memory_cache_config: InMemoryCacheConfig, /// Sender addresses to ignore. Transactions from these addresses will not be indexed. #[serde(default = "IndexerGrpcDataServiceConfig::default_sender_addresses_to_ignore")] pub sender_addresses_to_ignore: Vec, @@ -80,6 +83,7 @@ impl IndexerGrpcDataServiceConfig { file_store_config: IndexerGrpcFileStoreConfig, redis_read_replica_address: RedisUrl, enable_cache_compression: bool, + in_memory_cache_config: InMemoryCacheConfig, sender_addresses_to_ignore: Vec, ) -> Self { Self { @@ -92,6 +96,7 @@ impl IndexerGrpcDataServiceConfig { file_store_config, redis_read_replica_address, enable_cache_compression, + in_memory_cache_config, sender_addresses_to_ignore, } } @@ -117,6 +122,7 @@ impl RunnableConfig for IndexerGrpcDataServiceConfig { { bail!("At least one of data_service_grpc_non_tls_config and data_service_grpc_tls_config must be set"); } + self.in_memory_cache_config.validate()?; Ok(()) } @@ -146,6 +152,7 @@ impl RunnableConfig for IndexerGrpcDataServiceConfig { // InMemoryCache. let in_memory_cache = aptos_indexer_grpc_utils::in_memory_cache::InMemoryCache::new_with_redis_connection( + self.in_memory_cache_config.clone(), redis_conn, cache_storage_format, ) diff --git a/ecosystem/indexer-grpc/indexer-grpc-in-memory-cache-benchmark/src/main.rs b/ecosystem/indexer-grpc/indexer-grpc-in-memory-cache-benchmark/src/main.rs index d5efe9f0e5835..46aa6b554dbd3 100644 --- a/ecosystem/indexer-grpc/indexer-grpc-in-memory-cache-benchmark/src/main.rs +++ b/ecosystem/indexer-grpc/indexer-grpc-in-memory-cache-benchmark/src/main.rs @@ -10,7 +10,7 @@ use aptos_indexer_grpc_utils::{ compression_util::{CacheEntry, StorageFormat}, - in_memory_cache::{InMemoryCache, MAX_REDIS_FETCH_BATCH_SIZE}, + in_memory_cache::{InMemoryCache, InMemoryCacheConfig, MAX_REDIS_FETCH_BATCH_SIZE}, }; use aptos_protos::transaction::v1::{Transaction, TransactionInfo}; use lazy_static::lazy_static; @@ -132,6 +132,7 @@ async fn run_transaction_test( let redis_connection = create_mock_redis(transaction_size); let cache = Arc::new( InMemoryCache::new_with_redis_connection( + InMemoryCacheConfig::default(), redis_connection, StorageFormat::Lz4CompressedProto, ) diff --git a/ecosystem/indexer-grpc/indexer-grpc-utils/src/in_memory_cache.rs b/ecosystem/indexer-grpc/indexer-grpc-utils/src/in_memory_cache.rs index b8427737a8744..6a19e0c23bbb6 100644 --- a/ecosystem/indexer-grpc/indexer-grpc-utils/src/in_memory_cache.rs +++ b/ecosystem/indexer-grpc/indexer-grpc-utils/src/in_memory_cache.rs @@ -8,22 +8,75 @@ use dashmap::DashMap; use itertools::Itertools; use prost::Message; use redis::AsyncCommands; +use serde::{Deserialize, Serialize}; use std::sync::Arc; use tokio::sync::RwLock; // Internal lookup retry interval for in-memory cache. const IN_MEMORY_CACHE_LOOKUP_RETRY_INTERVAL_MS: u64 = 10; const IN_MEMORY_CACHE_GC_INTERVAL_MS: u64 = 100; -// Max cache size in bytes: 3 GB. -const IN_MEMORY_CACHE_TARGET_MAX_CAPACITY_IN_BYTES: u64 = 3_000_000_000; -// Eviction cache size in bytes: 3.5 GB. Evict the map to 3 GB. -const IN_MEMORY_CACHE_EVICTION_TRIGGER_SIZE_IN_BYTES: u64 = 3_500_000_000; // Max cache entry TTL: 30 seconds. // const MAX_IN_MEMORY_CACHE_ENTRY_TTL: u64 = 30; // Warm-up cache entries. Pre-fetch the cache entries to warm up the cache. pub const WARM_UP_CACHE_ENTRIES: u64 = 20_000; pub const MAX_REDIS_FETCH_BATCH_SIZE: usize = 500; +/// Configuration for when we want to explicitly declare how large the cache should be. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +#[serde(default)] +pub struct InMemoryCacheSizeConfig { + /// The maximum size of the cache in bytes. + cache_target_size_bytes: u64, + /// The maximum size of the cache in bytes before eviction is triggered, at which + /// point we reduce the size of the cache back to `cache_target_size_bytes`. + cache_eviction_trigger_size_bytes: u64, +} + +impl Default for InMemoryCacheSizeConfig { + fn default() -> Self { + Self { + // 3 GB. + cache_target_size_bytes: 3_000_000_000, + // 3.5 GB. + cache_eviction_trigger_size_bytes: 3_500_000_000, + } + } +} + +impl InMemoryCacheSizeConfig { + pub fn validate(&self) -> anyhow::Result<()> { + if self.cache_target_size_bytes == 0 { + return Err(anyhow::anyhow!("Cache target size must be greater than 0")); + } + if self.cache_eviction_trigger_size_bytes == 0 { + return Err(anyhow::anyhow!( + "Cache eviction trigger size must be greater than 0" + )); + } + if self.cache_eviction_trigger_size_bytes < self.cache_target_size_bytes { + return Err(anyhow::anyhow!( + "Cache eviction trigger size must be greater than cache target size" + )); + } + Ok(()) + } +} + +/// Configuration for the in memory cache. +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +#[serde(default)] +pub struct InMemoryCacheConfig { + size_config: InMemoryCacheSizeConfig, +} + +impl InMemoryCacheConfig { + pub fn validate(&self) -> anyhow::Result<()> { + self.size_config.validate() + } +} + #[derive(Debug, Clone, Copy)] struct CacheMetadata { total_size_in_bytes: u64, @@ -41,6 +94,7 @@ pub struct InMemoryCache { impl InMemoryCache { pub async fn new_with_redis_connection( + cache_config: InMemoryCacheConfig, conn: C, storage_format: StorageFormat, ) -> anyhow::Result @@ -68,6 +122,7 @@ impl InMemoryCache { cancellation_token.clone(), ); spawn_cleanup_task( + cache_config.size_config.clone(), cache.clone(), cache_metadata.clone(), cancellation_token.clone(), @@ -228,6 +283,7 @@ fn spawn_update_task( } fn spawn_cleanup_task( + cache_size_config: InMemoryCacheSizeConfig, cache: Arc>>, cache_metadata: Arc>, cancellation_token: tokio_util::sync::CancellationToken, @@ -241,7 +297,7 @@ fn spawn_cleanup_task( let mut current_cache_metadata = { *cache_metadata.read().await }; let should_evict = current_cache_metadata .total_size_in_bytes - .saturating_sub(IN_MEMORY_CACHE_EVICTION_TRIGGER_SIZE_IN_BYTES) + .saturating_sub(cache_size_config.cache_eviction_trigger_size_bytes) > 0; if !should_evict { tokio::time::sleep(std::time::Duration::from_millis( @@ -253,7 +309,7 @@ fn spawn_cleanup_task( let mut actual_bytes_removed = 0; let mut bytes_to_remove = current_cache_metadata .total_size_in_bytes - .saturating_sub(IN_MEMORY_CACHE_TARGET_MAX_CAPACITY_IN_BYTES); + .saturating_sub(cache_size_config.cache_target_size_bytes); while bytes_to_remove > 0 { let key_to_remove = current_cache_metadata.first_version; let (_k, v) = cache @@ -374,6 +430,7 @@ mod tests { Ok(0), )]); let in_memory_cache = InMemoryCache::new_with_redis_connection( + InMemoryCacheConfig::default(), mock_connection.clone(), StorageFormat::Base64UncompressedProto, ) @@ -401,6 +458,7 @@ mod tests { ), ]); let in_memory_cache = InMemoryCache::new_with_redis_connection( + InMemoryCacheConfig::default(), mock_connection.clone(), StorageFormat::Base64UncompressedProto, ) @@ -445,6 +503,7 @@ mod tests { MockCmd::new(redis::cmd("GET").arg("latest_version"), Ok(2)), ]); let in_memory_cache = InMemoryCache::new_with_redis_connection( + InMemoryCacheConfig::default(), mock_connection.clone(), StorageFormat::Base64UncompressedProto, ) From f21aee5f92e976c15fffb541e71696c63d371a2f Mon Sep 17 00:00:00 2001 From: Stelian Ionescu Date: Wed, 5 Jun 2024 13:10:48 -0400 Subject: [PATCH 15/26] Sync Terraform & Helm changes GitOrigin-RevId: 6a44870f14805780a129d9f7891804bb57682e43 --- terraform/aptos-node-testnet/gcp/forge.tf | 111 +----------------- terraform/aptos-node-testnet/gcp/main.tf | 6 +- terraform/aptos-node-testnet/gcp/variables.tf | 12 ++ terraform/aptos-node/gcp/network.tf | 14 ++- terraform/aptos-node/gcp/variables.tf | 12 ++ terraform/fullnode/gcp/network.tf | 14 ++- terraform/fullnode/gcp/variables.tf | 12 ++ .../helm/aptos-node/templates/fullnode.yaml | 6 + .../helm/aptos-node/templates/validator.yaml | 6 + terraform/helm/aptos-node/values.yaml | 4 + terraform/helm/fullnode/files/backup/gcs.yaml | 27 ++++- .../fullnode/templates/backup-compaction.yaml | 4 + .../fullnode/templates/backup-verify.yaml | 4 + terraform/helm/fullnode/templates/backup.yaml | 8 +- .../helm/fullnode/templates/fullnode.yaml | 6 + terraform/helm/fullnode/values.yaml | 4 + 16 files changed, 121 insertions(+), 129 deletions(-) diff --git a/terraform/aptos-node-testnet/gcp/forge.tf b/terraform/aptos-node-testnet/gcp/forge.tf index 2b77f6c915512..c70186596f199 100644 --- a/terraform/aptos-node-testnet/gcp/forge.tf +++ b/terraform/aptos-node-testnet/gcp/forge.tf @@ -1,6 +1,7 @@ locals { forge_helm_chart_path = "${path.module}/../../helm/forge" } + resource "helm_release" "forge" { count = var.enable_forge ? 1 : 0 name = "forge" @@ -25,113 +26,3 @@ resource "helm_release" "forge" { value = sha1(join("", [for f in fileset(local.forge_helm_chart_path, "**") : filesha1("${local.forge_helm_chart_path}/${f}")])) } } - - -resource "kubernetes_secret" "grafana_credentials" { - metadata { - name = "credentials" - namespace = "grafana" - } - - # Ignore changes to the data field to prevent replacing the manually updated password - lifecycle { - ignore_changes = [ - data, - ] - } - - # Create a placeholder password. This should be set manually in each cluster - data = { - password = base64encode("placeholder") - } -} - -resource "helm_release" "grafana_agent_flow" { - name = "grafana-agent-flow" - repository = "https://grafana.github.io/helm-charts" - chart = "grafana-agent" - version = "0.37.0" - namespace = "grafana" - - values = [ - yamlencode({ - agent = { - mode = "flow" - configMap = { - create = true - content = <<-EOT - remote.kubernetes.secret "credentials" { - namespace = "grafana" - name = "credentials" - } - discovery.kubernetes "local_pods" { - selectors { - field = "spec.nodeName=" + env("HOSTNAME") - role = "pod" - } - role = "pod" - } - discovery.relabel "specific_pods" { - targets = discovery.kubernetes.local_pods.targets - rule { - action = "drop" - regex = "Succeeded|Failed|Completed" - source_labels = ["__meta_kubernetes_pod_phase"] - } - rule { - action = "replace" - source_labels = ["__meta_kubernetes_namespace"] - target_label = "namespace" - } - rule { - action = "replace" - source_labels = ["__meta_kubernetes_pod_name"] - target_label = "pod" - } - rule { - action = "replace" - source_labels = ["__meta_kubernetes_pod_node_name"] - target_label = "node" - } - rule { - action = "replace" - source_labels = ["__meta_kubernetes_pod_container_name"] - target_label = "container" - } - rule { - action = "replace" - regex = "(.*)@(.*)" - replacement = "ebpf/$${1}/$${2}" - separator = "@" - source_labels = ["__meta_kubernetes_namespace", "__meta_kubernetes_pod_container_name"] - target_label = "service_name" - } - } - pyroscope.ebpf "instance" { - forward_to = [pyroscope.write.endpoint.receiver] - targets = discovery.relabel.specific_pods.output - demangle = "full" - } - pyroscope.write "endpoint" { - endpoint { - url = "https://profiles-prod-003.grafana.net" - basic_auth { - username = "340750" - password = remote.kubernetes.secret.credentials.data["password"] - } - } - } - EOT - } - securityContext = { - privileged = true - runAsGroup = 0 - runAsUser = 0 - } - } - controller = { - hostPID = true - } - }) - ] -} diff --git a/terraform/aptos-node-testnet/gcp/main.tf b/terraform/aptos-node-testnet/gcp/main.tf index 7551f898a78e6..c15119e09b236 100644 --- a/terraform/aptos-node-testnet/gcp/main.tf +++ b/terraform/aptos-node-testnet/gcp/main.tf @@ -44,8 +44,10 @@ module "validator" { validator_name = "aptos-node" # K8s config - k8s_api_sources = var.k8s_api_sources - cluster_ipv4_cidr_block = var.cluster_ipv4_cidr_block + k8s_api_sources = var.k8s_api_sources + cluster_ipv4_cidr_block = var.cluster_ipv4_cidr_block + router_nat_ip_allocate_option = var.router_nat_ip_allocate_option + enable_endpoint_independent_mapping = var.enable_endpoint_independent_mapping # autoscaling gke_enable_node_autoprovisioning = var.gke_enable_node_autoprovisioning diff --git a/terraform/aptos-node-testnet/gcp/variables.tf b/terraform/aptos-node-testnet/gcp/variables.tf index b56355fae99b7..880be3978563d 100644 --- a/terraform/aptos-node-testnet/gcp/variables.tf +++ b/terraform/aptos-node-testnet/gcp/variables.tf @@ -271,6 +271,18 @@ variable "cluster_ipv4_cidr_block" { default = "" } +variable "router_nat_ip_allocate_option" { + description = "The method of NAT IP allocation for the cluster. See https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#router_nat_ip_allocate_option" + type = string + default = "MANUAL_ONLY" +} + +variable "enable_endpoint_independent_mapping" { + description = "Enable endpoint independent mapping for the NAT router" + type = bool + default = true +} + variable "enable_clouddns" { description = "Enable CloudDNS (Google-managed cluster DNS)" type = bool diff --git a/terraform/aptos-node/gcp/network.tf b/terraform/aptos-node/gcp/network.tf index 97d92c23869f9..f7453ef0cb42a 100644 --- a/terraform/aptos-node/gcp/network.tf +++ b/terraform/aptos-node/gcp/network.tf @@ -26,9 +26,13 @@ resource "google_compute_address" "nat" { } resource "google_compute_router_nat" "nat" { - name = "aptos-${local.workspace_name}-nat" - router = google_compute_router.nat.name - nat_ip_allocate_option = "MANUAL_ONLY" - nat_ips = [google_compute_address.nat.self_link] - source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES" + name = "aptos-${local.workspace_name}-nat" + router = google_compute_router.nat.name + nat_ip_allocate_option = var.router_nat_ip_allocate_option + nat_ips = var.router_nat_ip_allocate_option == "MANUAL_ONLY" ? [google_compute_address.nat.self_link] : null + source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES" + min_ports_per_vm = var.router_nat_ip_allocate_option == "MANUAL_ONLY" ? null : 32 + enable_endpoint_independent_mapping = var.enable_endpoint_independent_mapping + # EndpointIndependentMapping and DynamicPortAllocation are mutually exclusive. + enable_dynamic_port_allocation = !var.enable_endpoint_independent_mapping } diff --git a/terraform/aptos-node/gcp/variables.tf b/terraform/aptos-node/gcp/variables.tf index 822cdda6681ec..fbea82b870eb8 100644 --- a/terraform/aptos-node/gcp/variables.tf +++ b/terraform/aptos-node/gcp/variables.tf @@ -263,6 +263,18 @@ variable "cluster_ipv4_cidr_block" { default = "" } +variable "router_nat_ip_allocate_option" { + description = "The method of NAT IP allocation for the cluster. See https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#router_nat_ip_allocate_option" + type = string + default = "MANUAL_ONLY" +} + +variable "enable_endpoint_independent_mapping" { + description = "Enable endpoint independent mapping for the NAT router" + type = bool + default = true +} + variable "enable_clouddns" { description = "Enable CloudDNS (Google-managed cluster DNS)" type = bool diff --git a/terraform/fullnode/gcp/network.tf b/terraform/fullnode/gcp/network.tf index 839e4a01474e0..0373648b950c1 100644 --- a/terraform/fullnode/gcp/network.tf +++ b/terraform/fullnode/gcp/network.tf @@ -26,9 +26,13 @@ resource "google_compute_address" "nat" { } resource "google_compute_router_nat" "nat" { - name = "aptos-${terraform.workspace}-nat" - router = google_compute_router.nat.name - nat_ip_allocate_option = "MANUAL_ONLY" - nat_ips = [google_compute_address.nat.self_link] - source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES" + name = "aptos-${terraform.workspace}-nat" + router = google_compute_router.nat.name + nat_ip_allocate_option = var.router_nat_ip_allocate_option + nat_ips = var.router_nat_ip_allocate_option == "MANUAL_ONLY" ? [google_compute_address.nat.self_link] : null + source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES" + min_ports_per_vm = var.router_nat_ip_allocate_option == "MANUAL_ONLY" ? null : 32 + enable_endpoint_independent_mapping = var.enable_endpoint_independent_mapping + # EndpointIndependentMapping and DynamicPortAllocation are mutually exclusive. + enable_dynamic_port_allocation = !var.enable_endpoint_independent_mapping } diff --git a/terraform/fullnode/gcp/variables.tf b/terraform/fullnode/gcp/variables.tf index d55669ac233a4..5954c15a7bd7c 100644 --- a/terraform/fullnode/gcp/variables.tf +++ b/terraform/fullnode/gcp/variables.tf @@ -180,6 +180,18 @@ variable "workspace_name_override" { ### GKE cluster config +variable "router_nat_ip_allocate_option" { + description = "The method of NAT IP allocation for the cluster. See https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#router_nat_ip_allocate_option" + type = string + default = "MANUAL_ONLY" +} + +variable "enable_endpoint_independent_mapping" { + description = "Enable endpoint independent mapping for the NAT router" + type = bool + default = false +} + variable "enable_clouddns" { description = "Enable CloudDNS (Google-managed cluster DNS)" type = bool diff --git a/terraform/helm/aptos-node/templates/fullnode.yaml b/terraform/helm/aptos-node/templates/fullnode.yaml index e942f6d3555a2..5a9f65a65f3db 100644 --- a/terraform/helm/aptos-node/templates/fullnode.yaml +++ b/terraform/helm/aptos-node/templates/fullnode.yaml @@ -96,11 +96,17 @@ spec: {{- include "aptos-validator.selectorLabels" $ | nindent 8 }} app.kubernetes.io/name: fullnode app.kubernetes.io/instance: fullnode-{{$i}} + {{- if $.Values.chain.name }} + chain_name: {{ $.Values.chain.name }} + {{- end}} group: {{ .name }} annotations: checksum/fullnode.yaml: {{ tpl ($.Files.Get "files/configs/fullnode.yaml") $ | sha256sum }} prometheus.io/scrape: "true" prometheus.io/port: "9101" + {{- if $.Values.metrics.destination }} + aptos.dev/metrics-destination: {{ $.Values.metrics.destination }} + {{- end}} spec: terminationGracePeriodSeconds: 0 securityContext: diff --git a/terraform/helm/aptos-node/templates/validator.yaml b/terraform/helm/aptos-node/templates/validator.yaml index 866ca05affa57..c356f96358387 100644 --- a/terraform/helm/aptos-node/templates/validator.yaml +++ b/terraform/helm/aptos-node/templates/validator.yaml @@ -77,10 +77,16 @@ spec: {{- include "aptos-validator.selectorLabels" $ | nindent 8 }} app.kubernetes.io/name: validator app.kubernetes.io/instance: validator-{{$i}} + {{- if $.Values.chain.name }} + chain_name: {{ $.Values.chain.name }} + {{- end}} annotations: checksum/validator.yaml: {{ tpl ($.Files.Get "files/configs/validator.yaml") $ | sha256sum }} prometheus.io/scrape: "true" prometheus.io/port: "9101" + {{- if $.Values.metrics.destination }} + aptos.dev/metrics-destination: {{ $.Values.metrics.destination }} + {{- end}} spec: terminationGracePeriodSeconds: 0 securityContext: diff --git a/terraform/helm/aptos-node/values.yaml b/terraform/helm/aptos-node/values.yaml index b09390607f35a..ef2ee7b73fc1f 100644 --- a/terraform/helm/aptos-node/values.yaml +++ b/terraform/helm/aptos-node/values.yaml @@ -178,6 +178,10 @@ serviceAccount: # -- The name of the service account to use. If not set and create is true, a name is generated using the fullname template name: +metrics: + # -- The upstream sink for metrics. Supported values are "dev" and "prod" + destination: dev + # -- Load test-data for starting a test network loadTestGenesis: false diff --git a/terraform/helm/fullnode/files/backup/gcs.yaml b/terraform/helm/fullnode/files/backup/gcs.yaml index 561a00384c64a..ddb5c93278ddf 100644 --- a/terraform/helm/fullnode/files/backup/gcs.yaml +++ b/terraform/helm/fullnode/files/backup/gcs.yaml @@ -5,12 +5,27 @@ commands: FILE_HANDLE="$BACKUP_HANDLE/$FILE_NAME" echo "$FILE_HANDLE" exec 1>&- # close stdout - gzip -c | gsutil -q cp - "gs://$BUCKET/$SUB_DIR/$FILE_HANDLE" > /dev/null - open_for_read: 'gsutil -q cp "gs://$BUCKET/$SUB_DIR/$FILE_HANDLE" - | gzip -cd' - save_metadata_line: | + gzip -c | gcloud storage cp - "gs://$BUCKET/$SUB_DIR/$FILE_HANDLE" > /dev/null + open_for_read: | + TEMP=$(mktemp) + trap "rm -f $TEMP" EXIT + for try in {0..4} + do + if [ $try -gt 0 ]; then + SLEEP=$((10 * $try)) + echo "sleeping for $SLEEP seconds before retry #$try" >&2 + sleep $SLEEP + fi + gcloud storage cp "gs://$BUCKET/$SUB_DIR/$FILE_HANDLE" $TEMP 1>&2 || continue + cat $TEMP | gzip -cd + exit + done + echo "Failed after 5 tries" >&2 + exit 1 + save_metadata_line: | FILE_HANDLE="metadata/$FILE_NAME" echo "$FILE_HANDLE" exec 1>&- - gzip -c | gsutil -q cp - "gs://$BUCKET/$SUB_DIR/$FILE_HANDLE" > /dev/null - list_metadata_files: '(gsutil -q ls gs://$BUCKET/$SUB_DIR/metadata/ ||:) | sed -ne "s#gs://.*/metadata/#metadata/#p"' - backup_metadata_file: 'gsutil mv gs://$BUCKET/$SUB_DIR/metadata/$FILE_NAME gs://$BUCKET/$SUB_DIR/metadata_backup/$FILE_NAME' + gzip -c | gcloud storage cp - "gs://$BUCKET/$SUB_DIR/$FILE_HANDLE" > /dev/null + list_metadata_files: '(gcloud storage ls gs://$BUCKET/$SUB_DIR/metadata/ ||:) | sed -ne "s#gs://.*/metadata/#metadata/#p"' + backup_metadata_file: "gcloud storage mv gs://$BUCKET/$SUB_DIR/metadata/$FILE_NAME gs://$BUCKET/$SUB_DIR/metadata_backup/$FILE_NAME" diff --git a/terraform/helm/fullnode/templates/backup-compaction.yaml b/terraform/helm/fullnode/templates/backup-compaction.yaml index 3bbc177035b00..e4d18fee51336 100644 --- a/terraform/helm/fullnode/templates/backup-compaction.yaml +++ b/terraform/helm/fullnode/templates/backup-compaction.yaml @@ -17,6 +17,10 @@ spec: labels: {{- include "backup.selectorLabels" . | nindent 12 }} app.kubernetes.io/name: backup-compaction + annotations: + {{- if $.Values.metrics.destination }} + aptos.dev/metrics-destination: {{ $.Values.metrics.destination }} + {{- end}} spec: restartPolicy: Never terminationGracePeriodSeconds: 0 diff --git a/terraform/helm/fullnode/templates/backup-verify.yaml b/terraform/helm/fullnode/templates/backup-verify.yaml index a4834aefa78b9..9a2108dd033c5 100644 --- a/terraform/helm/fullnode/templates/backup-verify.yaml +++ b/terraform/helm/fullnode/templates/backup-verify.yaml @@ -17,6 +17,10 @@ spec: labels: {{- include "backup.selectorLabels" . | nindent 12 }} app.kubernetes.io/name: backup-verify + annotations: + {{- if $.Values.metrics.destination }} + aptos.dev/metrics-destination: {{ $.Values.metrics.destination }} + {{- end}} spec: restartPolicy: Never terminationGracePeriodSeconds: 0 diff --git a/terraform/helm/fullnode/templates/backup.yaml b/terraform/helm/fullnode/templates/backup.yaml index 627193cefbd0b..f5813ef39bd42 100644 --- a/terraform/helm/fullnode/templates/backup.yaml +++ b/terraform/helm/fullnode/templates/backup.yaml @@ -1,3 +1,4 @@ +{{- if $.Values.backup.enable }} apiVersion: v1 kind: ConfigMap metadata: @@ -18,7 +19,7 @@ metadata: app.kubernetes.io/name: backup spec: serviceName: {{ include "backup.fullname" . }}-backup - replicas: {{ int .Values.backup.enable }} + replicas: 1 podManagementPolicy: Parallel selector: matchLabels: @@ -29,6 +30,10 @@ spec: labels: {{- include "backup.selectorLabels" . | nindent 8 }} app.kubernetes.io/name: backup + annotations: + {{- if $.Values.metrics.destination }} + aptos.dev/metrics-destination: {{ $.Values.metrics.destination }} + {{- end}} spec: terminationGracePeriodSeconds: 0 containers: @@ -113,3 +118,4 @@ spec: imagePullSecrets: - name: {{.Values.imagePullSecret}} {{- end }} +{{- end }} \ No newline at end of file diff --git a/terraform/helm/fullnode/templates/fullnode.yaml b/terraform/helm/fullnode/templates/fullnode.yaml index 895a3f640d06b..4c9ee6294a5ed 100644 --- a/terraform/helm/fullnode/templates/fullnode.yaml +++ b/terraform/helm/fullnode/templates/fullnode.yaml @@ -19,9 +19,15 @@ spec: labels: {{- include "aptos-fullnode.selectorLabels" . | nindent 8 }} app.kubernetes.io/name: fullnode + {{- if $.Values.chain.name }} + chain_name: {{ $.Values.chain.name }} + {{- end}} annotations: prometheus.io/scrape: "true" prometheus.io/port: "9101" + {{- if $.Values.metrics.destination }} + aptos.dev/metrics-destination: {{ $.Values.metrics.destination }} + {{- end}} spec: terminationGracePeriodSeconds: 0 initContainers: diff --git a/terraform/helm/fullnode/values.yaml b/terraform/helm/fullnode/values.yaml index 86f0f487f1d7e..1cf5e3346747b 100644 --- a/terraform/helm/fullnode/values.yaml +++ b/terraform/helm/fullnode/values.yaml @@ -101,6 +101,10 @@ logging: # -- Address for remote logging address: +metrics: + # -- The upstream sink for metrics. Supported values are "dev" and "prod" + destination: dev + backup: image: # -- Image repo to use for backup images From 7c41327f8f5f6c4b17982aaa29499ad30ac88f72 Mon Sep 17 00:00:00 2001 From: Oliver He Date: Wed, 5 Jun 2024 15:34:01 -0400 Subject: [PATCH 16/26] Lower the gas cost of Keyless txns to 32_000_000 (#13574) --- aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs b/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs index a5c4d00ed41dd..d7954dec3108d 100644 --- a/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs +++ b/aptos-move/aptos-gas-schedule/src/gas_schedule/transaction.rs @@ -257,7 +257,7 @@ crate::gas_schedule::macros::define_gas_parameters!( [ keyless_base_cost: InternalGas, { RELEASE_V1_12.. => "keyless.base" }, - 138_000_000, + 32_000_000, ] ] ); From c0749d55fff8d79fb76592dc392e70790b035a42 Mon Sep 17 00:00:00 2001 From: Satya Vusirikala Date: Wed, 5 Jun 2024 14:54:06 -0500 Subject: [PATCH 17/26] Enable order votes at genesis (#13563) --- types/src/on_chain_config/consensus_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/src/on_chain_config/consensus_config.rs b/types/src/on_chain_config/consensus_config.rs index 5d5c06f903eb6..a6b7387948f53 100644 --- a/types/src/on_chain_config/consensus_config.rs +++ b/types/src/on_chain_config/consensus_config.rs @@ -27,7 +27,7 @@ impl ConsensusAlgorithmConfig { Self::JolteonV2 { main: ConsensusConfigV1::default(), quorum_store_enabled: true, - order_vote_enabled: false, + order_vote_enabled: true, } } From 10729ad45af7163650b87911769db48aa4b8a771 Mon Sep 17 00:00:00 2001 From: igor-aptos <110557261+igor-aptos@users.noreply.github.com> Date: Wed, 5 Jun 2024 16:52:40 -0700 Subject: [PATCH 18/26] Fix forge stable and some recalibration (#13582) --- testsuite/forge-cli/src/main.rs | 24 +++++++++++-------- testsuite/forge/src/backend/k8s/prometheus.rs | 11 +++++++-- testsuite/forge/src/success_criteria.rs | 19 +++++++++++++++ 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/testsuite/forge-cli/src/main.rs b/testsuite/forge-cli/src/main.rs index 89d134fd69f4b..f0706af80692f 100644 --- a/testsuite/forge-cli/src/main.rs +++ b/testsuite/forge-cli/src/main.rs @@ -1112,10 +1112,10 @@ fn realistic_env_workload_sweep_test() -> ForgeConfig { ]), // Investigate/improve to make latency more predictable on different workloads criteria: [ - (5500, 100, 0.3, 0.3, 0.8, 0.65), - (4500, 100, 0.3, 0.4, 1.0, 2.0), - (2000, 300, 0.3, 0.8, 0.8, 2.0), - (600, 500, 0.3, 0.6, 0.8, 2.0), + (7700, 100, 0.3, 0.3, 0.5, 0.5), + (7000, 100, 0.3, 0.3, 0.5, 0.5), + (2000, 300, 0.3, 0.8, 0.6, 0.7), + (3200, 500, 0.3, 0.4, 0.7, 1.0), // (150, 0.5, 1.0, 1.5, 0.65), ] .into_iter() @@ -1425,10 +1425,7 @@ fn realistic_env_graceful_overload() -> ForgeConfig { inner_traffic: EmitJobRequest::default() .mode(EmitJobMode::ConstTps { tps: 15000 }) .init_gas_price_multiplier(20), - // Additionally - we are not really gracefully handling overlaods, - // setting limits based on current reality, to make sure they - // don't regress, but something to investigate - inner_success_criteria: SuccessCriteria::new(3400), + inner_success_criteria: SuccessCriteria::new(7500), })) // First start higher gas-fee traffic, to not cause issues with TxnEmitter setup - account creation .with_emit_job( @@ -1450,8 +1447,8 @@ fn realistic_env_graceful_overload() -> ForgeConfig { // overload test uses more CPUs than others, so increase the limit // Check that we don't use more than 18 CPU cores for 30% of the time. MetricsThreshold::new(18.0, 40), - // Check that we don't use more than 5 GB of memory for 30% of the time. - MetricsThreshold::new_gb(5.0, 30), + // Check that we don't use more than 6 GB of memory for more than 10% of the time. + MetricsThreshold::new_gb(6.0, 10), )) .add_latency_threshold(10.0, LatencyType::P50) .add_latency_threshold(30.0, LatencyType::P90) @@ -1950,6 +1947,13 @@ fn realistic_env_max_load_test( let long_running = duration_secs >= 2400; let mut success_criteria = SuccessCriteria::new(95) + .add_system_metrics_threshold(SystemMetricsThreshold::new( + // Check that we don't use more than 18 CPU cores for 10% of the time. + MetricsThreshold::new(18.0, 10), + // Memory starts around 3GB, and grows around 1.2GB/hr in this test. + // Check that we don't use more than final expected memory for more than 10% of the time. + MetricsThreshold::new_gb(3.3 + 1.4 * (duration_secs as f64 / 3600.0), 10), + )) .add_no_restarts() .add_wait_for_catchup_s( // Give at least 60s for catchup, give 10% of the run for longer durations. diff --git a/testsuite/forge/src/backend/k8s/prometheus.rs b/testsuite/forge/src/backend/k8s/prometheus.rs index d0d28b7f729df..93367f590fbe0 100644 --- a/testsuite/forge/src/backend/k8s/prometheus.rs +++ b/testsuite/forge/src/backend/k8s/prometheus.rs @@ -4,7 +4,7 @@ use crate::{create_k8s_client, K8sApi, ReadWrite, Result}; use again::RetryPolicy; use anyhow::{anyhow, bail}; -use aptos_logger::info; +use aptos_logger::{info, warn}; use k8s_openapi::api::core::v1::Secret; use once_cell::sync::Lazy; use prometheus_http_query::{ @@ -185,7 +185,14 @@ pub async fn query_range_with_metadata( new_query ) })?; - if range.len() != 1 { + if range.is_empty() { + warn!( + "Missing data for start={}, end={}, query={}", + start_time, end_time, new_query + ); + return Ok(Vec::new()); + } + if range.len() > 1 { bail!( "Expected only one range vector from prometheus, recieved {} ({:?}). start={}, end={}, query={}", range.len(), diff --git a/testsuite/forge/src/success_criteria.rs b/testsuite/forge/src/success_criteria.rs index 1bd1896d298c4..6283fbf8a57dc 100644 --- a/testsuite/forge/src/success_criteria.rs +++ b/testsuite/forge/src/success_criteria.rs @@ -34,6 +34,8 @@ pub struct MetricsThreshold { max: f64, // % of the data point that can breach the max threshold max_breach_pct: usize, + + expect_empty: bool, } impl MetricsThreshold { @@ -41,6 +43,15 @@ impl MetricsThreshold { Self { max, max_breach_pct, + expect_empty: false, + } + } + + pub fn new_expect_empty() -> Self { + Self { + max: 0.0, + max_breach_pct: 0, + expect_empty: true, } } @@ -48,6 +59,7 @@ impl MetricsThreshold { Self { max: max * 1024.0 * 1024.0 * 1024.0, max_breach_pct, + expect_empty: false, } } @@ -56,6 +68,13 @@ impl MetricsThreshold { metrics_name: &str, metrics: &Vec, ) -> anyhow::Result<()> { + if self.expect_empty { + if !metrics.is_empty() { + bail!("Data found for metrics expected to be empty"); + } + return Ok(()); + } + if metrics.is_empty() { bail!("Empty metrics provided"); } From 44d8f8561c7a50f2aca994d2dc81a7c2a47a151e Mon Sep 17 00:00:00 2001 From: aldenhu Date: Wed, 5 Jun 2024 00:48:29 +0000 Subject: [PATCH 19/26] backup service send in batch --- crates/aptos-metrics-core/src/lib.rs | 10 ++ .../backup-service/src/handlers/utils.rs | 136 ++++++++++++++---- 2 files changed, 122 insertions(+), 24 deletions(-) diff --git a/crates/aptos-metrics-core/src/lib.rs b/crates/aptos-metrics-core/src/lib.rs index 3b7c7b63b2d61..dc83c3147b3b8 100644 --- a/crates/aptos-metrics-core/src/lib.rs +++ b/crates/aptos-metrics-core/src/lib.rs @@ -37,11 +37,21 @@ impl IntGaugeHelper for IntGaugeVec { } pub trait IntCounterHelper { + type IntType; + fn inc_with(&self, labels: &[&str]); + + fn inc_with_by(&self, labels: &[&str], by: Self::IntType); } impl IntCounterHelper for IntCounterVec { + type IntType = u64; + fn inc_with(&self, labels: &[&str]) { self.with_label_values(labels).inc() } + + fn inc_with_by(&self, labels: &[&str], v: Self::IntType) { + self.with_label_values(labels).inc_by(v) + } } diff --git a/storage/backup/backup-service/src/handlers/utils.rs b/storage/backup/backup-service/src/handlers/utils.rs index f834a241a93d7..b4d68839bd36d 100644 --- a/storage/backup/backup-service/src/handlers/utils.rs +++ b/storage/backup/backup-service/src/handlers/utils.rs @@ -5,10 +5,10 @@ use aptos_db::backup::backup_handler::BackupHandler; use aptos_logger::prelude::*; use aptos_metrics_core::{ - register_histogram_vec, register_int_counter_vec, HistogramVec, IntCounterVec, + register_histogram_vec, register_int_counter_vec, HistogramVec, IntCounterHelper, IntCounterVec, }; use aptos_storage_interface::{AptosDbError, Result}; -use bytes::Bytes; +use bytes::{BufMut, Bytes, BytesMut}; use hyper::Body; use once_cell::sync::Lazy; use serde::Serialize; @@ -44,30 +44,101 @@ pub(super) fn reply_with_bcs_bytes( Ok(Box::new(bytes)) } +#[must_use] pub(super) struct BytesSender { - endpoint: &'static str, - inner: hyper::body::Sender, + buffer: BytesMut, + batch_tx: Option>, + sender_task: Option)>>, } impl BytesSender { - fn new(endpoint: &'static str, inner: hyper::body::Sender) -> Self { - Self { endpoint, inner } + fn new(endpoint: &'static str, mut inner: hyper::body::Sender) -> Self { + let (batch_tx, mut batch_rx) = tokio::sync::mpsc::channel::(2); + + let sender_task = tokio::spawn(async move { + let res = async { + while let Some(batch) = batch_rx.recv().await { + let n_bytes = batch.len(); + + inner.send_data(batch).await.into_db_res()?; + + THROUGHPUT_COUNTER.inc_with_by(&[endpoint], n_bytes as u64); + } + Ok(()) + } + .await; + + (inner, res) + }); + + Self { + buffer: BytesMut::new(), + batch_tx: Some(batch_tx), + sender_task: Some(sender_task), + } } - async fn send_data(&mut self, chunk: Bytes) -> Result<()> { - let n_bytes = chunk.len(); - self.inner - .send_data(chunk) + async fn flush_buffer(&mut self) -> Result<()> { + self.batch_tx + .as_mut() + .expect("Batch sender gone.") + .send(self.buffer.split().freeze()) .await - .map_err(|e| AptosDbError::Other(e.to_string()))?; - THROUGHPUT_COUNTER - .with_label_values(&[self.endpoint]) - .inc_by(n_bytes as u64); + .into_db_res() + } + + async fn send_data(&mut self, bytes: &[u8]) -> Result<()> { + let sender_task = self.sender_task.as_ref().expect("Sender task gone."); + + if sender_task.is_finished() { + return Err(AptosDbError::Other( + "Sender task finished unexpectedly.".to_string(), + )); + } + + self.buffer.put_slice(bytes); + + const TARGET_BATCH_SIZE: usize = if cfg!(test) { 10 } else { 1024 * 1024 * 10 }; + if self.buffer.len() >= TARGET_BATCH_SIZE { + self.flush_buffer().await? + } + Ok(()) } - fn abort(self) { - self.inner.abort() + async fn finish_impl(&mut self, abort: bool) -> Result<()> { + let mut ret = Ok(()); + + if !abort { + ret = self.flush_buffer().await + } + + // drop sender to inform the sending task to quit + self.batch_tx.take().unwrap(); + + let (inner, res) = self + .sender_task + .take() + .unwrap() + .await + .expect("Sender task panicked."); + + ret = ret.and(res); + + if abort || ret.is_err() { + inner.abort(); + } + + ret + } + + async fn finish(mut self) -> Result<()> { + self.finish_impl(false).await + } + + async fn abort(mut self) { + // ignore error + let _ = self.finish_impl(true).await; } } @@ -93,12 +164,17 @@ where I: Iterator>, R: Serialize, { - send_size_prefixed_bcs_bytes_impl(iter_res, &mut sender) - .await - .unwrap_or_else(|e| { - warn!("Failed writing to output http body: {:?}", e); - sender.abort() - }); + match send_size_prefixed_bcs_bytes_impl(iter_res, &mut sender).await { + Ok(()) => { + if let Err(e) = sender.finish().await { + warn!("Failed to finish http body: {:?}", e); + } + }, + Err(e) => { + warn!("Failed writing http body: {:?}", e); + sender.abort().await; + }, + } } async fn send_size_prefixed_bcs_bytes_impl( @@ -113,9 +189,11 @@ where let record = record_res?; let record_bytes = bcs::to_bytes(&record)?; let size_bytes = (record_bytes.len() as u32).to_be_bytes(); - sender.send_data(Bytes::from(size_bytes.to_vec())).await?; - sender.send_data(Bytes::from(record_bytes)).await?; + + sender.send_data(&size_bytes).await?; + sender.send_data(&record_bytes).await?; } + Ok(()) } @@ -135,3 +213,13 @@ pub(super) async fn handle_rejection(err: Rejection) -> Result { + fn into_db_res(self) -> Result; +} + +impl IntoDbResult for std::result::Result { + fn into_db_res(self) -> Result { + self.map_err(|e| AptosDbError::Other(e.to_string())) + } +} From 77050b027b04f73b5a04a8326d13080d7ab3c507 Mon Sep 17 00:00:00 2001 From: aldenhu Date: Wed, 5 Jun 2024 02:23:39 +0000 Subject: [PATCH 20/26] recv_some --- .../backup-service/src/handlers/utils.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/storage/backup/backup-service/src/handlers/utils.rs b/storage/backup/backup-service/src/handlers/utils.rs index b4d68839bd36d..c2deea1b60566 100644 --- a/storage/backup/backup-service/src/handlers/utils.rs +++ b/storage/backup/backup-service/src/handlers/utils.rs @@ -53,11 +53,11 @@ pub(super) struct BytesSender { impl BytesSender { fn new(endpoint: &'static str, mut inner: hyper::body::Sender) -> Self { - let (batch_tx, mut batch_rx) = tokio::sync::mpsc::channel::(2); + let (batch_tx, mut batch_rx) = tokio::sync::mpsc::channel::(100); let sender_task = tokio::spawn(async move { let res = async { - while let Some(batch) = batch_rx.recv().await { + while let Some(batch) = Self::recv_some(&mut batch_rx).await { let n_bytes = batch.len(); inner.send_data(batch).await.into_db_res()?; @@ -78,6 +78,20 @@ impl BytesSender { } } + async fn recv_some(rx: &mut tokio::sync::mpsc::Receiver) -> Option { + let mut buf = BytesMut::new(); + + while let Ok(bytes) = rx.try_recv() { + buf.put(bytes); + } + + if buf.is_empty() { + rx.recv().await + } else { + Some(buf.freeze()) + } + } + async fn flush_buffer(&mut self) -> Result<()> { self.batch_tx .as_mut() @@ -98,7 +112,7 @@ impl BytesSender { self.buffer.put_slice(bytes); - const TARGET_BATCH_SIZE: usize = if cfg!(test) { 10 } else { 1024 * 1024 * 10 }; + const TARGET_BATCH_SIZE: usize = if cfg!(test) { 10 } else { 1024 * 1024 }; if self.buffer.len() >= TARGET_BATCH_SIZE { self.flush_buffer().await? } From d0a46ebf8ced62138406c975722f49dc7401a42a Mon Sep 17 00:00:00 2001 From: igor-aptos <110557261+igor-aptos@users.noreply.github.com> Date: Wed, 5 Jun 2024 22:32:56 -0700 Subject: [PATCH 21/26] Propagate resolver errors, instead of transforming into invariant error (#13560) --- aptos-move/aptos-vm/src/aptos_vm.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 312eb1e98434d..c1be9bc885567 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -2786,11 +2786,7 @@ pub(crate) fn is_account_init_for_sponsored_transaction( && resolver .get_resource(&txn_data.sender(), &AccountResource::struct_tag()) .map(|data| data.is_none()) - .map_err(|e| { - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message(format!("{}", e)) - .finish(Location::Undefined) - })?, + .map_err(|e| e.finish(Location::Undefined))?, ) } From 1156201fcdfe8f7e82bca36c37470ce046d6735d Mon Sep 17 00:00:00 2001 From: igor-aptos <110557261+igor-aptos@users.noreply.github.com> Date: Wed, 5 Jun 2024 23:19:47 -0700 Subject: [PATCH 22/26] Examples of using is_at_least and providing such handling for TokenV2 collections (#13511) --- .../aptos-framework/doc/aggregator_v2.md | 36 ++++++++ .../sources/aggregator_v2/aggregator_v2.move | 50 +++++++++- .../aggregator_examples/Move.toml | 10 ++ .../sources/counter_with_milestone.move | 64 +++++++++++++ crates/transaction-generator-lib/src/args.rs | 8 ++ .../src/publishing/module_simple.rs | 76 +++++++++------ .../src/publishing/raw_module_data.rs | 92 +++++++++++++++++++ execution/executor-benchmark/src/lib.rs | 2 +- testsuite/module-publish/src/main.rs | 4 + testsuite/single_node_performance.py | 1 + 10 files changed, 309 insertions(+), 34 deletions(-) create mode 100644 aptos-move/move-examples/aggregator_examples/Move.toml create mode 100644 aptos-move/move-examples/aggregator_examples/sources/counter_with_milestone.move diff --git a/aptos-move/framework/aptos-framework/doc/aggregator_v2.md b/aptos-move/framework/aptos-framework/doc/aggregator_v2.md index 5c6546de16e71..842ad22ecdab3 100644 --- a/aptos-move/framework/aptos-framework/doc/aggregator_v2.md +++ b/aptos-move/framework/aptos-framework/doc/aggregator_v2.md @@ -16,6 +16,14 @@ operation that also reduced parallelism, and should be avoided as much as possib If you need to capture the value, without revealing it, use snapshot function instead, which has no parallelism impact. +From parallelism considerations, there are three different levels of effects: +* enable full parallelism (cannot create conflicts): +max_value, create_*, snapshot, derive_string_concat +* enable speculative parallelism (generally parallel via branch prediction) +try_add, add, try_sub, sub, is_at_least +* create read/write conflicts, as if you were using a regular field +read, read_snapshot, read_derived_string + - [Struct `Aggregator`](#0x1_aggregator_v2_Aggregator) - [Struct `AggregatorSnapshot`](#0x1_aggregator_v2_AggregatorSnapshot) @@ -374,6 +382,8 @@ EAGGREGATOR_ELEMENT_TYPE_NOT_SUPPORTED raised if called with a different type. Adds value to aggregator. If addition would exceed the max_value, false is returned, and aggregator value is left unchanged. +Parallelism info: This operation enables speculative parallelism. +
public fun try_add<IntElement>(aggregator: &mut aggregator_v2::Aggregator<IntElement>, value: IntElement): bool
 
@@ -395,6 +405,10 @@ If addition would exceed the max_value, false is returned, a ## Function `add` +Adds value to aggregator, unconditionally. +If addition would exceed the max_value, EAGGREGATOR_OVERFLOW exception will be thrown. + +Parallelism info: This operation enables speculative parallelism.
public fun add<IntElement>(aggregator: &mut aggregator_v2::Aggregator<IntElement>, value: IntElement)
@@ -422,6 +436,8 @@ If addition would exceed the max_value, false is returned, a
 Subtracts value from aggregator.
 If subtraction would result in a negative value, false is returned, and aggregator value is left unchanged.
 
+Parallelism info: This operation enables speculative parallelism.
+
 
 
public fun try_sub<IntElement>(aggregator: &mut aggregator_v2::Aggregator<IntElement>, value: IntElement): bool
 
@@ -444,6 +460,8 @@ If subtraction would result in a negative value, false is re ## Function `sub` +Parallelism info: This operation enables speculative parallelism. +
public fun sub<IntElement>(aggregator: &mut aggregator_v2::Aggregator<IntElement>, value: IntElement)
 
@@ -489,6 +507,14 @@ If subtraction would result in a negative value, false is re ## Function `is_at_least` +Returns true if aggregator value is larger than or equal to the given min_amount, false otherwise. + +This operation is more efficient and much more parallelization friendly than calling read(agg) > min_amount. +Until traits are deployed, is_at_most/is_equal utility methods can be derived from this one (assuming +1 doesn't overflow): +- for is_at_most(agg, max_amount), you can do !is_at_least(max_amount + 1) +- for is_equal(agg, value), you can do is_at_least(value) && !is_at_least(value + 1) + +Parallelism info: This operation enables speculative parallelism.
public fun is_at_least<IntElement>(aggregator: &aggregator_v2::Aggregator<IntElement>, min_amount: IntElement): bool
@@ -523,6 +549,8 @@ it will sequentialize that transaction. (i.e. up to concurrency_level times slow
 If called in a separate transaction (i.e. after transaction that modifies aggregator), it might be
 up to two times slower.
 
+Parallelism info: This operation *prevents* speculative parallelism.
+
 
 
public fun read<IntElement>(aggregator: &aggregator_v2::Aggregator<IntElement>): IntElement
 
@@ -547,6 +575,8 @@ up to two times slower. Returns a wrapper of a current value of an aggregator Unlike read(), it is fast and avoids sequential dependencies. +Parallelism info: This operation enables parallelism. +
public fun snapshot<IntElement>(aggregator: &aggregator_v2::Aggregator<IntElement>): aggregator_v2::AggregatorSnapshot<IntElement>
 
@@ -597,6 +627,8 @@ Note: This operation is resource-intensive, and reduces parallelism. (Especially if called in a transaction that also modifies the aggregator, or has other read/write conflicts) +Parallelism info: This operation *prevents* speculative parallelism. +
public fun read_snapshot<IntElement>(snapshot: &aggregator_v2::AggregatorSnapshot<IntElement>): IntElement
 
@@ -623,6 +655,8 @@ Note: This operation is resource-intensive, and reduces parallelism. (Especially if called in a transaction that also modifies the aggregator, or has other read/write conflicts) +Parallelism info: This operation *prevents* speculative parallelism. +
public fun read_derived_string(snapshot: &aggregator_v2::DerivedStringSnapshot): string::String
 
@@ -673,6 +707,8 @@ snapshot passed needs to have integer type - currently supported types are u64 a Raises EUNSUPPORTED_AGGREGATOR_SNAPSHOT_TYPE if called with another type. If length of prefix and suffix together exceed 256 bytes, ECONCAT_STRING_LENGTH_TOO_LARGE is raised. +Parallelism info: This operation enables parallelism. +
public fun derive_string_concat<IntElement>(before: string::String, snapshot: &aggregator_v2::AggregatorSnapshot<IntElement>, after: string::String): aggregator_v2::DerivedStringSnapshot
 
diff --git a/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move b/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move index 8bbbf9e7373b6..7e26548bc0abd 100644 --- a/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move +++ b/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move @@ -10,6 +10,14 @@ /// operation that also reduced parallelism, and should be avoided as much as possible. /// If you need to capture the value, without revealing it, use snapshot function instead, /// which has no parallelism impact. +/// +/// From parallelism considerations, there are three different levels of effects: +/// * enable full parallelism (cannot create conflicts): +/// max_value, create_*, snapshot, derive_string_concat +/// * enable speculative parallelism (generally parallel via branch prediction) +/// try_add, add, try_sub, sub, is_at_least +/// * create read/write conflicts, as if you were using a regular field +/// read, read_snapshot, read_derived_string module aptos_framework::aggregator_v2 { use std::error; use std::features; @@ -90,31 +98,57 @@ module aptos_framework::aggregator_v2 { /// Adds `value` to aggregator. /// If addition would exceed the max_value, `false` is returned, and aggregator value is left unchanged. + /// + /// Parallelism info: This operation enables speculative parallelism. public native fun try_add(aggregator: &mut Aggregator, value: IntElement): bool; - // Adds `value` to aggregator, uncoditionally. - // If addition would exceed the max_value, EAGGREGATOR_OVERFLOW exception will be thrown. + /// Adds `value` to aggregator, unconditionally. + /// If addition would exceed the max_value, EAGGREGATOR_OVERFLOW exception will be thrown. + /// + /// Parallelism info: This operation enables speculative parallelism. public fun add(aggregator: &mut Aggregator, value: IntElement) { assert!(try_add(aggregator, value), error::out_of_range(EAGGREGATOR_OVERFLOW)); } /// Subtracts `value` from aggregator. /// If subtraction would result in a negative value, `false` is returned, and aggregator value is left unchanged. + /// + /// Parallelism info: This operation enables speculative parallelism. public native fun try_sub(aggregator: &mut Aggregator, value: IntElement): bool; - // Subtracts `value` to aggregator, uncoditionally. + // Subtracts `value` to aggregator, unconditionally. // If subtraction would result in a negative value, EAGGREGATOR_UNDERFLOW exception will be thrown. + /// + /// Parallelism info: This operation enables speculative parallelism. public fun sub(aggregator: &mut Aggregator, value: IntElement) { assert!(try_sub(aggregator, value), error::out_of_range(EAGGREGATOR_UNDERFLOW)); } native fun is_at_least_impl(aggregator: &Aggregator, min_amount: IntElement): bool; + /// Returns true if aggregator value is larger than or equal to the given `min_amount`, false otherwise. + /// + /// This operation is more efficient and much more parallelization friendly than calling `read(agg) > min_amount`. + /// Until traits are deployed, `is_at_most`/`is_equal` utility methods can be derived from this one (assuming +1 doesn't overflow): + /// - for `is_at_most(agg, max_amount)`, you can do `!is_at_least(max_amount + 1)` + /// - for `is_equal(agg, value)`, you can do `is_at_least(value) && !is_at_least(value + 1)` + /// + /// Parallelism info: This operation enables speculative parallelism. public fun is_at_least(aggregator: &Aggregator, min_amount: IntElement): bool { assert!(features::aggregator_v2_is_at_least_api_enabled(), EAGGREGATOR_API_V2_NOT_ENABLED); is_at_least_impl(aggregator, min_amount) } + // TODO waiting for integer traits + // public fun is_at_most(aggregator: &Aggregator, max_amount: IntElement): bool { + // !is_at_least(max_amount + 1) + // } + + // TODO waiting for integer traits + // public fun is_equal(aggregator: &Aggregator, value: IntElement): bool { + // is_at_least(value) && !is_at_least(value + 1) + // } + /// Returns a value stored in this aggregator. /// Note: This operation is resource-intensive, and reduces parallelism. /// If you need to capture the value, without revealing it, use snapshot function instead, @@ -123,10 +157,14 @@ module aptos_framework::aggregator_v2 { /// it will sequentialize that transaction. (i.e. up to concurrency_level times slower) /// If called in a separate transaction (i.e. after transaction that modifies aggregator), it might be /// up to two times slower. + /// + /// Parallelism info: This operation *prevents* speculative parallelism. public native fun read(aggregator: &Aggregator): IntElement; /// Returns a wrapper of a current value of an aggregator /// Unlike read(), it is fast and avoids sequential dependencies. + /// + /// Parallelism info: This operation enables parallelism. public native fun snapshot(aggregator: &Aggregator): AggregatorSnapshot; /// Creates a snapshot of a given value. @@ -137,12 +175,16 @@ module aptos_framework::aggregator_v2 { /// Note: This operation is resource-intensive, and reduces parallelism. /// (Especially if called in a transaction that also modifies the aggregator, /// or has other read/write conflicts) + /// + /// Parallelism info: This operation *prevents* speculative parallelism. public native fun read_snapshot(snapshot: &AggregatorSnapshot): IntElement; /// Returns a value stored in this DerivedStringSnapshot. /// Note: This operation is resource-intensive, and reduces parallelism. /// (Especially if called in a transaction that also modifies the aggregator, /// or has other read/write conflicts) + /// + /// Parallelism info: This operation *prevents* speculative parallelism. public native fun read_derived_string(snapshot: &DerivedStringSnapshot): String; /// Creates a DerivedStringSnapshot of a given value. @@ -153,6 +195,8 @@ module aptos_framework::aggregator_v2 { /// snapshot passed needs to have integer type - currently supported types are u64 and u128. /// Raises EUNSUPPORTED_AGGREGATOR_SNAPSHOT_TYPE if called with another type. /// If length of prefix and suffix together exceed 256 bytes, ECONCAT_STRING_LENGTH_TOO_LARGE is raised. + /// + /// Parallelism info: This operation enables parallelism. public native fun derive_string_concat(before: String, snapshot: &AggregatorSnapshot, after: String): DerivedStringSnapshot; // ===== DEPRECATE/NOT YET IMPLEMENTED ==== diff --git a/aptos-move/move-examples/aggregator_examples/Move.toml b/aptos-move/move-examples/aggregator_examples/Move.toml new file mode 100644 index 0000000000000..0f74a21d75a76 --- /dev/null +++ b/aptos-move/move-examples/aggregator_examples/Move.toml @@ -0,0 +1,10 @@ +[package] +name = "Aggregator examples" +version = "0.0.0" + +[addresses] +aggregator_examples = "0xCAFE" + +[dependencies] +AptosStdlib = { local = "../../framework/aptos-stdlib" } +AptosFramework = { local = "../../framework/aptos-framework" } diff --git a/aptos-move/move-examples/aggregator_examples/sources/counter_with_milestone.move b/aptos-move/move-examples/aggregator_examples/sources/counter_with_milestone.move new file mode 100644 index 0000000000000..812b2c00df81f --- /dev/null +++ b/aptos-move/move-examples/aggregator_examples/sources/counter_with_milestone.move @@ -0,0 +1,64 @@ +/// This is a module that showcases how to use new AggregatorV2 parallelism and branch prediction features +/// to create a global counter, which is checked for specific milestones, +/// and transactions that cross the milestone have special logic to celebrate the milestone. +/// Checking the milestone only creates a reduced parallelism right around the point where +/// milestone is reached, and so if milestones are spread out (like a thousand or more apart), +/// there should be no impact on throughput / parallelism overall. +/// +/// In a very similar way, milestones for token (Digital Asset) minting can be processed, +/// by using collection::is_total_minted_at_least call, as Digital Asset collections track their +/// supply and total minted thus far. +module aggregator_examples::counter_with_milestone { + use std::error; + use std::signer; + use aptos_framework::aggregator_v2::{Self, Aggregator}; + use aptos_framework::event; + + // Resource being modified doesn't exist + const ERESOURCE_NOT_PRESENT: u64 = 2; + + // Incrementing a counter failed + const ECOUNTER_INCREMENT_FAIL: u64 = 4; + + const ENOT_AUTHORIZED: u64 = 5; + + struct MilestoneCounter has key { + next_milestone: u64, + milestone_every: u64, + count: Aggregator, + } + + #[event] + struct MilestoneReached has drop, store { + milestone: u64, + } + + // Create the global `MilestoneCounter`. + // Stored under the module publisher address. + public entry fun create(publisher: &signer, milestone_every: u64) { + assert!( + signer::address_of(publisher) == @aggregator_examples, + ENOT_AUTHORIZED, + ); + + move_to( + publisher, + MilestoneCounter { + next_milestone: milestone_every, + milestone_every, + count: aggregator_v2::create_unbounded_aggregator(), + } + ); + } + + public entry fun increment_milestone() acquires MilestoneCounter { + assert!(exists(@aggregator_examples), error::invalid_argument(ERESOURCE_NOT_PRESENT)); + let milestone_counter = borrow_global_mut(@aggregator_examples); + assert!(aggregator_v2::try_add(&mut milestone_counter.count, 1), ECOUNTER_INCREMENT_FAIL); + + if (aggregator_v2::is_at_least(&milestone_counter.count, milestone_counter.next_milestone) && !aggregator_v2::is_at_least(&milestone_counter.count, milestone_counter.next_milestone + 1)) { + event::emit(MilestoneReached { milestone: milestone_counter.next_milestone}); + milestone_counter.next_milestone = milestone_counter.next_milestone + milestone_counter.milestone_every; + } + } +} diff --git a/crates/transaction-generator-lib/src/args.rs b/crates/transaction-generator-lib/src/args.rs index 80064e5a90107..2e58fa5880a57 100644 --- a/crates/transaction-generator-lib/src/args.rs +++ b/crates/transaction-generator-lib/src/args.rs @@ -35,6 +35,7 @@ pub enum TransactionTypeArg { ModifyGlobalResourceAggV2, ModifyGlobalFlagAggV2, ModifyGlobalBoundedAggV2, + ModifyGlobalMilestoneAggV2, // Complex EntryPoints CreateObjects10, CreateObjects10WithPayload10k, @@ -156,6 +157,13 @@ impl TransactionTypeArg { num_modules: module_working_set_size, use_account_pool: sender_use_account_pool, }, + TransactionTypeArg::ModifyGlobalMilestoneAggV2 => TransactionType::CallCustomModules { + entry_point: EntryPoints::IncGlobalMilestoneAggV2 { + milestone_every: 1000, + }, + num_modules: module_working_set_size, + use_account_pool: sender_use_account_pool, + }, TransactionTypeArg::NoOp => TransactionType::CallCustomModules { entry_point: EntryPoints::Nop, num_modules: module_working_set_size, diff --git a/crates/transaction-generator-lib/src/publishing/module_simple.rs b/crates/transaction-generator-lib/src/publishing/module_simple.rs index 2399a1919037d..3f2e234d220d1 100644 --- a/crates/transaction-generator-lib/src/publishing/module_simple.rs +++ b/crates/transaction-generator-lib/src/publishing/module_simple.rs @@ -181,14 +181,21 @@ pub enum EntryPoints { max_offset: u64, max_count: u64, }, - /// Increment global (publisher) resource - COUNTER_STEP + /// Increment global (publisher) resource IncGlobal, - /// Increment global (publisher) AggregatorV2 resource - COUNTER_STEP + /// Increment global (publisher) AggregatorV2 resource IncGlobalAggV2, /// Modify (try_add(step) or try_sub(step)) AggregatorV2 bounded counter (counter with max_value=100) ModifyGlobalBoundedAggV2 { step: u64, }, + /// Increment global (publisher) AggregatorV2 resource with Milestone (and conflict) every `milestone_every` increments. + IncGlobalMilestoneAggV2 { + milestone_every: u64, + }, + CreateGlobalMilestoneAggV2 { + milestone_every: u64, + }, /// Modifying a single random tag in a resource group (which contains 8 tags), /// from a global resource (at module publishers' address) @@ -313,6 +320,8 @@ impl EntryPoints { | EntryPoints::VectorPictureRead { .. } | EntryPoints::InitializeSmartTablePicture | EntryPoints::SmartTablePicture { .. } => "complex", + EntryPoints::IncGlobalMilestoneAggV2 { .. } + | EntryPoints::CreateGlobalMilestoneAggV2 { .. } => "aggregator_examples", EntryPoints::DeserializeU256 => "bcs_stream", } } @@ -370,6 +379,8 @@ impl EntryPoints { EntryPoints::InitializeSmartTablePicture | EntryPoints::SmartTablePicture { .. } => { "smart_table_picture" }, + EntryPoints::IncGlobalMilestoneAggV2 { .. } + | EntryPoints::CreateGlobalMilestoneAggV2 { .. } => "counter_with_milestone", EntryPoints::DeserializeU256 => "bcs_stream", } } @@ -476,10 +487,32 @@ impl EntryPoints { ], ) }, - EntryPoints::IncGlobal => inc_global(module_id), - EntryPoints::IncGlobalAggV2 => inc_global_agg_v2(module_id), + EntryPoints::IncGlobal => { + get_payload(module_id, ident_str!("increment").to_owned(), vec![]) + }, + EntryPoints::IncGlobalAggV2 => { + get_payload(module_id, ident_str!("increment_agg_v2").to_owned(), vec![]) + }, EntryPoints::ModifyGlobalBoundedAggV2 { step } => { - modify_bounded_agg_v2(module_id, rng.expect("Must provide RNG"), *step) + let rng = rng.expect("Must provide RNG"); + get_payload( + module_id, + ident_str!("modify_bounded_agg_v2").to_owned(), + vec![ + bcs::to_bytes(&rng.gen::()).unwrap(), + bcs::to_bytes(&step).unwrap(), + ], + ) + }, + EntryPoints::IncGlobalMilestoneAggV2 { .. } => get_payload( + module_id, + ident_str!("increment_milestone").to_owned(), + vec![], + ), + EntryPoints::CreateGlobalMilestoneAggV2 { milestone_every } => { + get_payload(module_id, ident_str!("create").to_owned(), vec![ + bcs::to_bytes(&milestone_every).unwrap(), + ]) }, EntryPoints::CreateObjects { num_objects, @@ -711,6 +744,11 @@ impl EntryPoints { Some(EntryPoints::InitializeVectorPicture { length: *length }) }, EntryPoints::SmartTablePicture { .. } => Some(EntryPoints::InitializeSmartTablePicture), + EntryPoints::IncGlobalMilestoneAggV2 { milestone_every } => { + Some(EntryPoints::CreateGlobalMilestoneAggV2 { + milestone_every: *milestone_every, + }) + }, _ => None, } } @@ -729,6 +767,7 @@ impl EntryPoints { MultiSigConfig::Publisher }, EntryPoints::LiquidityPoolSwap { .. } => MultiSigConfig::Publisher, + EntryPoints::CreateGlobalMilestoneAggV2 { .. } => MultiSigConfig::Publisher, _ => MultiSigConfig::None, } } @@ -788,6 +827,8 @@ impl EntryPoints { EntryPoints::InitializeSmartTablePicture => AutomaticArgs::Signer, EntryPoints::SmartTablePicture { .. } => AutomaticArgs::None, EntryPoints::DeserializeU256 => AutomaticArgs::None, + EntryPoints::IncGlobalMilestoneAggV2 { .. } => AutomaticArgs::None, + EntryPoints::CreateGlobalMilestoneAggV2 { .. } => AutomaticArgs::Signer, } } } @@ -880,31 +921,6 @@ fn minimize(module_id: ModuleId, other: &AccountAddress) -> TransactionPayload { ]) } -fn inc_global(module_id: ModuleId) -> TransactionPayload { - get_payload(module_id, ident_str!("increment").to_owned(), vec![]) -} - -fn inc_global_agg_v2(module_id: ModuleId) -> TransactionPayload { - get_payload(module_id, ident_str!("increment_agg_v2").to_owned(), vec![]) -} - -fn modify_bounded_agg_v2(module_id: ModuleId, rng: &mut StdRng, step: u64) -> TransactionPayload { - get_payload( - module_id, - ident_str!("modify_bounded_agg_v2").to_owned(), - vec![ - bcs::to_bytes(&rng.gen::()).unwrap(), - bcs::to_bytes(&step).unwrap(), - ], - ) -} - -fn mint_new_token(module_id: ModuleId, other: AccountAddress) -> TransactionPayload { - get_payload(module_id, ident_str!("mint_new_token").to_owned(), vec![ - bcs::to_bytes(&other).unwrap(), - ]) -} - fn rand_string(rng: &mut StdRng, len: usize) -> String { let res = rng .sample_iter(&Alphanumeric) diff --git a/crates/transaction-generator-lib/src/publishing/raw_module_data.rs b/crates/transaction-generator-lib/src/publishing/raw_module_data.rs index 3e1685098ee7f..c46f5cacc01b3 100644 --- a/crates/transaction-generator-lib/src/publishing/raw_module_data.rs +++ b/crates/transaction-generator-lib/src/publishing/raw_module_data.rs @@ -1777,6 +1777,96 @@ pub static MODULES_AMBASSADOR_TOKEN: Lazy>> = Lazy::new(|| { vec![ MODULE_AMBASSADOR_TOKEN_AMBASSADOR.to_vec(), ]}); #[rustfmt::skip] +pub static PACKAGE_AGGREGATOR_EXAMPLES_METADATA: Lazy> = Lazy::new(|| { + vec![ + 19, 65, 103, 103, 114, 101, 103, 97, 116, 111, 114, 32, 101, 120, 97, 109, 112, 108, + 101, 115, 1, 0, 0, 0, 0, 0, 0, 0, 0, 64, 69, 55, 68, 49, 68, 70, + 69, 51, 70, 65, 54, 67, 56, 55, 55, 68, 48, 69, 68, 68, 50, 57, 51, 52, + 54, 55, 67, 49, 51, 49, 53, 53, 51, 50, 52, 57, 56, 49, 53, 48, 67, 68, + 69, 51, 52, 50, 68, 50, 65, 66, 50, 66, 50, 53, 57, 52, 53, 50, 52, 68, + 69, 51, 70, 48, 162, 1, 31, 139, 8, 0, 0, 0, 0, 0, 2, 255, 141, 142, + 65, 10, 195, 32, 16, 69, 247, 158, 66, 220, 215, 244, 2, 93, 72, 105, 46, 208, + 101, 8, 101, 170, 83, 145, 24, 21, 71, 218, 64, 233, 221, 171, 41, 201, 186, 204, + 108, 62, 255, 61, 248, 67, 2, 61, 129, 197, 145, 5, 152, 145, 159, 184, 80, 214, + 102, 180, 80, 98, 230, 184, 192, 156, 60, 146, 96, 79, 204, 228, 98, 104, 253, 81, + 214, 19, 140, 13, 96, 76, 70, 34, 164, 145, 193, 238, 220, 54, 103, 69, 151, 179, + 234, 47, 141, 53, 152, 48, 24, 12, 218, 53, 92, 165, 18, 233, 90, 140, 119, 247, + 138, 189, 185, 143, 26, 124, 19, 164, 236, 234, 63, 114, 157, 242, 138, 121, 234, 160, + 129, 7, 90, 73, 193, 63, 63, 177, 223, 234, 127, 220, 61, 55, 253, 11, 194, 232, + 76, 11, 237, 0, 0, 0, 1, 22, 99, 111, 117, 110, 116, 101, 114, 95, 119, 105, + 116, 104, 95, 109, 105, 108, 101, 115, 116, 111, 110, 101, 0, 0, 0, 3, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 14, 65, 112, 116, 111, 115, + 70, 114, 97, 109, 101, 119, 111, 114, 107, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 1, 11, 65, 112, 116, 111, 115, 83, 116, 100, 108, 105, 98, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 10, 77, 111, 118, 101, + 83, 116, 100, 108, 105, 98, 0, + ] +}); + +#[rustfmt::skip] +pub static MODULE_AGGREGATOR_EXAMPLES_COUNTER_WITH_MILESTONE: Lazy> = Lazy::new(|| { + vec![ + 161, 28, 235, 11, 6, 0, 0, 0, 12, 1, 0, 10, 2, 10, 14, 3, 24, 44, + 4, 68, 8, 5, 76, 49, 7, 125, 128, 2, 8, 253, 2, 64, 6, 189, 3, 64, + 16, 253, 3, 132, 1, 10, 129, 5, 17, 12, 146, 5, 164, 1, 13, 182, 6, 6, + 0, 0, 1, 1, 1, 2, 1, 3, 1, 4, 0, 5, 8, 0, 0, 6, 6, 0, + 1, 12, 6, 1, 0, 0, 0, 7, 0, 1, 0, 0, 8, 1, 1, 0, 4, 14, + 2, 3, 0, 1, 15, 1, 5, 1, 3, 2, 16, 4, 4, 0, 1, 17, 7, 8, + 1, 0, 1, 18, 9, 8, 1, 0, 3, 19, 11, 1, 1, 6, 3, 4, 5, 4, + 6, 4, 7, 10, 2, 6, 12, 3, 0, 1, 6, 12, 1, 5, 1, 3, 1, 11, + 2, 1, 9, 0, 2, 1, 7, 8, 0, 2, 7, 11, 2, 1, 9, 0, 9, 0, + 1, 1, 2, 6, 11, 2, 1, 9, 0, 9, 0, 1, 8, 1, 1, 9, 0, 22, + 99, 111, 117, 110, 116, 101, 114, 95, 119, 105, 116, 104, 95, 109, 105, 108, 101, 115, + 116, 111, 110, 101, 13, 97, 103, 103, 114, 101, 103, 97, 116, 111, 114, 95, 118, 50, + 5, 101, 114, 114, 111, 114, 5, 101, 118, 101, 110, 116, 6, 115, 105, 103, 110, 101, + 114, 16, 77, 105, 108, 101, 115, 116, 111, 110, 101, 67, 111, 117, 110, 116, 101, 114, + 16, 77, 105, 108, 101, 115, 116, 111, 110, 101, 82, 101, 97, 99, 104, 101, 100, 6, + 99, 114, 101, 97, 116, 101, 19, 105, 110, 99, 114, 101, 109, 101, 110, 116, 95, 109, + 105, 108, 101, 115, 116, 111, 110, 101, 14, 110, 101, 120, 116, 95, 109, 105, 108, 101, + 115, 116, 111, 110, 101, 15, 109, 105, 108, 101, 115, 116, 111, 110, 101, 95, 101, 118, + 101, 114, 121, 5, 99, 111, 117, 110, 116, 10, 65, 103, 103, 114, 101, 103, 97, 116, + 111, 114, 9, 109, 105, 108, 101, 115, 116, 111, 110, 101, 10, 97, 100, 100, 114, 101, + 115, 115, 95, 111, 102, 27, 99, 114, 101, 97, 116, 101, 95, 117, 110, 98, 111, 117, + 110, 100, 101, 100, 95, 97, 103, 103, 114, 101, 103, 97, 116, 111, 114, 16, 105, 110, + 118, 97, 108, 105, 100, 95, 97, 114, 103, 117, 109, 101, 110, 116, 7, 116, 114, 121, + 95, 97, 100, 100, 11, 105, 115, 95, 97, 116, 95, 108, 101, 97, 115, 116, 4, 101, + 109, 105, 116, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 202, 254, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 3, 8, 4, 0, 0, + 0, 0, 0, 0, 0, 3, 8, 5, 0, 0, 0, 0, 0, 0, 0, 3, 8, 2, + 0, 0, 0, 0, 0, 0, 0, 5, 32, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 202, 254, 18, 97, 112, 116, 111, 115, 58, 58, 109, 101, 116, 97, 100, + 97, 116, 97, 95, 118, 49, 112, 3, 2, 0, 0, 0, 0, 0, 0, 0, 21, 69, + 82, 69, 83, 79, 85, 82, 67, 69, 95, 78, 79, 84, 95, 80, 82, 69, 83, 69, + 78, 84, 0, 4, 0, 0, 0, 0, 0, 0, 0, 23, 69, 67, 79, 85, 78, 84, + 69, 82, 95, 73, 78, 67, 82, 69, 77, 69, 78, 84, 95, 70, 65, 73, 76, 0, + 5, 0, 0, 0, 0, 0, 0, 0, 15, 69, 78, 79, 84, 95, 65, 85, 84, 72, + 79, 82, 73, 90, 69, 68, 0, 1, 16, 77, 105, 108, 101, 115, 116, 111, 110, 101, + 82, 101, 97, 99, 104, 101, 100, 1, 4, 0, 0, 0, 2, 3, 9, 3, 10, 3, + 11, 11, 2, 1, 3, 1, 2, 1, 13, 3, 0, 1, 4, 0, 1, 17, 10, 0, + 17, 2, 7, 3, 33, 4, 6, 5, 10, 11, 0, 1, 7, 1, 39, 11, 0, 10, + 1, 11, 1, 56, 0, 18, 0, 45, 0, 2, 1, 1, 4, 1, 0, 6, 61, 7, + 3, 41, 0, 4, 4, 5, 7, 7, 2, 17, 4, 39, 7, 3, 42, 0, 12, 1, + 10, 1, 15, 0, 6, 1, 0, 0, 0, 0, 0, 0, 0, 56, 1, 4, 16, 5, + 20, 11, 1, 1, 7, 0, 39, 10, 1, 16, 0, 10, 1, 16, 1, 20, 56, 2, + 4, 38, 10, 1, 16, 0, 10, 1, 16, 1, 20, 6, 1, 0, 0, 0, 0, 0, + 0, 0, 22, 56, 2, 32, 12, 0, 5, 40, 9, 12, 0, 11, 0, 4, 58, 10, + 1, 16, 1, 20, 18, 1, 56, 3, 10, 1, 16, 1, 20, 10, 1, 16, 2, 20, + 22, 11, 1, 15, 1, 21, 5, 60, 11, 1, 1, 2, 0, 2, 0, 0, 0, 1, + 0, + ] +}); + +#[rustfmt::skip] +pub static MODULES_AGGREGATOR_EXAMPLES: Lazy>> = Lazy::new(|| { vec![ + MODULE_AGGREGATOR_EXAMPLES_COUNTER_WITH_MILESTONE.to_vec(), +]}); +#[rustfmt::skip] pub static PACKAGE_BCS_STREAM_METADATA: Lazy> = Lazy::new(|| { vec![ 9, 66, 67, 83, 83, 116, 114, 101, 97, 109, 1, 0, 0, 0, 0, 0, 0, 0, @@ -1991,6 +2081,7 @@ pub static PACKAGE_TO_METADATA: Lazy>> = Lazy::new(|| { ("simple".to_string(), PACKAGE_SIMPLE_METADATA.to_vec()), ("framework_usecases".to_string(), PACKAGE_FRAMEWORK_USECASES_METADATA.to_vec()), ("ambassador_token".to_string(), PACKAGE_AMBASSADOR_TOKEN_METADATA.to_vec()), + ("aggregator_examples".to_string(), PACKAGE_AGGREGATOR_EXAMPLES_METADATA.to_vec()), ("bcs_stream".to_string(), PACKAGE_BCS_STREAM_METADATA.to_vec()), ])}); @@ -2000,5 +2091,6 @@ pub static PACKAGE_TO_MODULES: Lazy>>> = Lazy::new(| ("simple".to_string(), MODULES_SIMPLE.to_vec()), ("framework_usecases".to_string(), MODULES_FRAMEWORK_USECASES.to_vec()), ("ambassador_token".to_string(), MODULES_AMBASSADOR_TOKEN.to_vec()), + ("aggregator_examples".to_string(), MODULES_AGGREGATOR_EXAMPLES.to_vec()), ("bcs_stream".to_string(), MODULES_BCS_STREAM.to_vec()), ])}); diff --git a/execution/executor-benchmark/src/lib.rs b/execution/executor-benchmark/src/lib.rs index 33562386cfff7..f4db0dc0083be 100644 --- a/execution/executor-benchmark/src/lib.rs +++ b/execution/executor-benchmark/src/lib.rs @@ -768,7 +768,7 @@ mod tests { AptosVM::set_processed_transactions_detailed_counters(); NativeExecutor::set_concurrency_level_once(4); test_generic_benchmark::( - Some(TransactionTypeArg::ResourceGroupsGlobalWriteTag1KB), + Some(TransactionTypeArg::ModifyGlobalMilestoneAggV2), true, ); } diff --git a/testsuite/module-publish/src/main.rs b/testsuite/module-publish/src/main.rs index ea2060aa7bb63..9c428bbf0e1d5 100644 --- a/testsuite/module-publish/src/main.rs +++ b/testsuite/module-publish/src/main.rs @@ -26,6 +26,10 @@ fn additional_packages() -> Vec<(&'static str, &'static str)> { "ambassador_token", "../../aptos-move/move-examples/token_objects/ambassador", ), + ( + "aggregator_examples", + "../../aptos-move/move-examples/aggregator_examples", + ), ("bcs_stream", "../../aptos-move/move-examples/bcs-stream"), ] } diff --git a/testsuite/single_node_performance.py b/testsuite/single_node_performance.py index 5d2cd47964f8b..da9384564b3af 100755 --- a/testsuite/single_node_performance.py +++ b/testsuite/single_node_performance.py @@ -119,6 +119,7 @@ class RunGroupConfig: RunGroupConfig(expected_tps=12500, key=RunGroupKey("modify-global-flag-agg-v2", module_working_set_size=50), included_in=Flow.AGG_V2), RunGroupConfig(expected_tps=12070, key=RunGroupKey("modify-global-bounded-agg-v2"), included_in=Flow.AGG_V2 | Flow.CONTINUOUS), RunGroupConfig(expected_tps=12500, key=RunGroupKey("modify-global-bounded-agg-v2", module_working_set_size=50), included_in=Flow.AGG_V2), + RunGroupConfig(expected_tps=16195, key=RunGroupKey("modify-global-milestone-agg-v2"), included_in=Flow.AGG_V2 | Flow.CONTINUOUS), RunGroupConfig(expected_tps=3530, key=RunGroupKey("resource-groups-global-write-tag1-kb"), included_in=LAND_BLOCKING_AND_C | Flow.RESOURCE_GROUPS), RunGroupConfig(expected_tps=8000, key=RunGroupKey("resource-groups-global-write-tag1-kb", module_working_set_size=20), included_in=Flow.RESOURCE_GROUPS, waived=True), From bc402cecdbaaf27d207de06a38c9b4b45df4a136 Mon Sep 17 00:00:00 2001 From: "Brian (Sunghoon) Cho" Date: Thu, 6 Jun 2024 10:41:33 -0700 Subject: [PATCH 23/26] [forge] move haproxy test to unstable (#13572) --- .github/workflows/forge-stable.yaml | 39 +++++++++------------------ .github/workflows/forge-unstable.yaml | 23 +++++++++++++--- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/.github/workflows/forge-stable.yaml b/.github/workflows/forge-stable.yaml index 045696302cd44..1ce8cee790b26 100644 --- a/.github/workflows/forge-stable.yaml +++ b/.github/workflows/forge-stable.yaml @@ -136,7 +136,7 @@ jobs: run-forge-realistic-env-max-load-long: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-framework-upgrade-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-framework-upgrade-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -148,7 +148,7 @@ jobs: run-forge-realistic-env-load-sweep: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-realistic-env-max-load-long] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-realistic-env-max-load-long ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -160,7 +160,7 @@ jobs: run-forge-realistic-env-workload-sweep: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-realistic-env-load-sweep] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-realistic-env-load-sweep ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -172,7 +172,7 @@ jobs: run-forge-realistic-env-graceful-overload: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-realistic-env-workload-sweep] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-realistic-env-workload-sweep ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -184,7 +184,7 @@ jobs: run-forge-realistic-network-tuned-for-throughput: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-realistic-env-graceful-overload] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-realistic-env-graceful-overload ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -199,7 +199,7 @@ jobs: run-forge-consensus-stress-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-realistic-network-tuned-for-throughput] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-realistic-network-tuned-for-throughput ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -211,7 +211,7 @@ jobs: run-forge-workload-mix-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-consensus-stress-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-consensus-stress-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -223,7 +223,7 @@ jobs: run-forge-single-vfn-perf: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-workload-mix-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-workload-mix-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -233,22 +233,9 @@ jobs: FORGE_TEST_SUITE: single_vfn_perf POST_TO_SLACK: true - run-forge-haproxy: - if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-single-vfn-perf] # Only run after the previous job completes - uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main - secrets: inherit - with: - IMAGE_TAG: ${{ needs.determine-test-metadata.outputs.IMAGE_TAG }} - FORGE_NAMESPACE: forge-haproxy-${{ needs.determine-test-metadata.outputs.BRANCH_HASH }} - FORGE_RUNNER_DURATION_SECS: 600 # Run for 10 minutes - FORGE_ENABLE_HAPROXY: true - FORGE_TEST_SUITE: realistic_env_max_load - POST_TO_SLACK: true - run-forge-fullnode-reboot-stress-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-haproxy] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-single-vfn-perf ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -262,7 +249,7 @@ jobs: run-forge-compat: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-fullnode-reboot-stress-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-fullnode-reboot-stress-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -278,7 +265,7 @@ jobs: run-forge-changing-working-quorum-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-compat] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-compat ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -291,7 +278,7 @@ jobs: run-forge-changing-working-quorum-test-high-load: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-changing-working-quorum-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-changing-working-quorum-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -305,7 +292,7 @@ jobs: # Measures PFN latencies with a constant TPS (with a realistic environment) run-forge-pfn-const-tps-realistic-env: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-changing-working-quorum-test-high-load] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-changing-working-quorum-test-high-load ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: diff --git a/.github/workflows/forge-unstable.yaml b/.github/workflows/forge-unstable.yaml index dc0d6e5eb4d4f..bfc926007220e 100644 --- a/.github/workflows/forge-unstable.yaml +++ b/.github/workflows/forge-unstable.yaml @@ -101,7 +101,7 @@ jobs: run-forge-state-sync-slow-processing-catching-up-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, forge-continuous] # Only run after the previous job completes + needs: [ determine-test-metadata, forge-continuous ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -116,7 +116,7 @@ jobs: run-forge-twin-validator-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-state-sync-slow-processing-catching-up-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-state-sync-slow-processing-catching-up-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -130,7 +130,7 @@ jobs: run-forge-state-sync-failures-catching-up-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-twin-validator-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-twin-validator-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -145,7 +145,7 @@ jobs: run-forge-validator-reboot-stress-test: if: ${{ github.event_name != 'pull_request' && always() }} - needs: [determine-test-metadata, run-forge-state-sync-failures-catching-up-test] # Only run after the previous job completes + needs: [ determine-test-metadata, run-forge-state-sync-failures-catching-up-test ] # Only run after the previous job completes uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main secrets: inherit with: @@ -156,3 +156,18 @@ jobs: FORGE_RUNNER_DURATION_SECS: 2400 # Run for 40 minutes FORGE_TEST_SUITE: validator_reboot_stress_test POST_TO_SLACK: true + + run-forge-haproxy: + if: ${{ github.event_name != 'pull_request' && always() }} + needs: [ determine-test-metadata, run-forge-validator-reboot-stress-test ] # Only run after the previous job completes + uses: aptos-labs/aptos-core/.github/workflows/workflow-run-forge.yaml@main + secrets: inherit + with: + IMAGE_TAG: ${{ needs.determine-test-metadata.outputs.IMAGE_TAG }} + # GCP cluster + FORGE_CLUSTER_NAME: aptos-forge-1 + FORGE_NAMESPACE: forge-haproxy-${{ needs.determine-test-metadata.outputs.BRANCH_HASH }} + FORGE_RUNNER_DURATION_SECS: 600 # Run for 10 minutes + FORGE_ENABLE_HAPROXY: true + FORGE_TEST_SUITE: realistic_env_max_load + POST_TO_SLACK: true From 5d6a395ad696c10794d196b9b52b0f5b8f2b7159 Mon Sep 17 00:00:00 2001 From: Gerardo Di Giacomo Date: Thu, 6 Jun 2024 12:08:03 -0700 Subject: [PATCH 24/26] Update SECURITY.md (#13585) * Update SECURITY.md --- SECURITY.md | 140 ++-------------------------------------------------- 1 file changed, 3 insertions(+), 137 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 48c42cb940f20..b956e520c47eb 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,139 +1,5 @@ -# Aptos Core Bug Bounty +# Aptos Foundation Bounty Program -## Reporting a Security Concern +The Aptos Foundation welcomes feedback from security researchers and the general public to help improve the security of the Aptos Network, and, at its sole discretion, offers bounty rewards for security reports that identify previously unknown, in-scope security vulnerabilities. -**DO NOT CREATE AN ISSUE** to report a security problem. - -Go to https://github.com/aptos-labs/aptos-core/security/advisories and open a vulnerability report. Send an email to [security@aptosfoundation.org](mailto:security@aptosfoundation.org) and provide your GitHub username. The team will triage the issue from there. - -For security reasons, DO NOT include attachments or provide detail sufficient for exploitation regarding the security issue in this email. Instead, wait for the advisory to be created, and **provide any sensitive details in the private GitHub advisory**. - -If you haven't done so already, please **enable two-factor authentation** in your GitHub account. - -Send the email from an email domain that is less likely to get flagged for spam by gmail. - -This is an actively monitored account, the team will quickly respond. - -If you do not receive a response within 24 hours, please directly followup with the team in [Discord](https://discord.gg/aptosnetwork) by reaching out to anyone with the role “Aptos Labs”. - -As above, please DO NOT include attachments or provide detail regarding the security issue in this email. - -## Incident Response Process - -1. Establish a new draft security advisory - 1. In response to an email to [security@aptosfoundation.org](mailto:security@aptosfoundation.org), a member of the Aptos Labs will create a new draft security advisory for the incident at [https://github.com/aptos-labs/aptos-core/security/policy](https://github.com/aptos-labs/aptos-core/security/policy). - 2. Add the reporter's GitHub account and relevant individuals to the draft security advisory. - 3. Respond to the reporter by email, sharing a link to the draft security advisory. -2. Responder should add appropriate content to the draft security advisory. To be eligible for a bug bounty, this includes: - 1. A clear description of the issue and the impacted areas. - 2. The code and the methodology to reproduce the underlying issue. - 3. Discussion of potential remediations. -3. Triage - 1. Validate the issue. - 2. Determine the criticality of the issue. - 3. If this is a bug and not a security issue, recommend to the submitter to create an issue. -4. Resolve by building new docker containers and binaries that contain the fix and pushing to affected entities. - -## Bug Bounties - -Aptos Foundation offers bounties for security reports. Reports will be validated against the GitHub repository branch labeled "mainnet" and no others. Moreover, any code within mainnet but not in an actively deployed network or in a position to be deployed in such an environment, such as experimental or actively developed code, are out of scope for the bug bounty program. In addition, if a bug discovered in mainnet is already fixed in main, it is out of scope. Production environments include the Aptos testnet and mainnet. - -The scope includes code within - -- [Aptos Core — main branch](https://github.com/aptos-labs/aptos-core/tree/main) -- [Move — Aptos branch](https://github.com/move-language/move/tree/aptos) - -Aptos Foundation considers the following levels of severity: - -### Critical — Up to $1,000,000 USD in APT tokens (locked for 12 months) - -- Direct loss of funds to users or protocols with minimal preconditions, such as, Move type confusion. -- Vulnerabilities in the Proof of Stake system which directly compromise consensus. -- Unintended permanent chain split requiring hard fork (network partition requiring hardfork). -- Permanent freezing, burning, or modification of funds (fix requires hardfork). - -### High — Up to $100,000 USD in APT tokens (locked for 12 months) - -- Loss of funds with some pre-conditions, such as, halting the network. -- Interrupting blockchain progress or halting the network. - -### Medium — Up to $25,000 USD in APT tokens (locked for 12 months) - -- Denial of service issues which compromise the integrity or availability of the chain. -- Loss of funds with many preconditions. -- Ability to crash a production node with some pre-conditions. - -## Payment of Bug Bounties - -- Bounties are currently awarded on a rolling/weekly basis and paid out within 30 days upon receipt of successful KYC and payment contract. -- The APT/USD conversion rate used for payments is the market price of APT (denominated in USD) at 11:59 PM PST the day that both KYC and the payment contract are completed. -- The reference for this price is the Closing Price given by Coingecko.com on that date given here: [https://www.coingecko.com/en/coins/aptos/historical_data#panel](https://www.coingecko.com/en/coins/aptos/historical_data#panel) -- Bug bounties that are paid out in APT are paid to locked to the account provided by the reporter with a lockup expiring 12 months from the date of the delivery of APT. -- Multiple vulnerabilities of similar root cause will be paid out as one report. - -## Duplicate Reports - -Compensation for duplicate reports will be split among reporters with first to report taking priority using the following equation: - -``` -R: total reports -ri: report priority -bi: bounty share - -bi = 2 ^ (R - ri) / ((2^R) - 1) -``` - -Where report priority derives from the set of integers beginning at 1, where the first reporter has `ri = 1`, the second reporter `ri = 2`, and so forth. - -Note, reports that come in after the issue has been fully triaged and resolved will not be eligible for splitting. - -## KYC Requirements - -This bug bounty program is only open to individuals [outside the OFAC restricted countries](https://home.treasury.gov/policy-issues/financial-sanctions/sanctions-programs-and-country-information). Bug bounty hunters will be required to provide evidence that they are not a resident or citizen of these countries to be eligible for a reward. If the individual is a US person, tax information will be required, such as a W-9, in order to properly issue a 1099. Aptos requires KYC to be done for all bug bounty hunters submitting a report and wanting a reward. Form W-9 or Form W-8 is required for tax purposes. All bug bounty hunters are required to use Persona for KYC, links will be provided upon resolution of the issue. The collection of this information will be done by the Aptos Foundation. - -If an impact can be caused to any other asset managed by Aptos that isn’t on this table but for which the impact is not inscope, you are encouraged to submit it for consideration by the project. - -## Out of Scope - -The following vulnerabilities are excluded from the rewards for this bug bounty program: - -- Attacks that the reporter has already exploited themselves, leading to damage. -- Attacks requiring access to leaked keys/credentials. -- Internally known issues, duplicate issues, or issues that have already been made public; in such cases, proof of prior disclosure will be provided. -- Attacks that rely on social engineering or require physical access to the victim’s device. -- Information disclosure with minimal security impact (Ex: stack traces, path disclosure, directory listing, logs). -- Tab-nabbing. -- Vulnerabilities related to auto-fill web forms. -- Vulnerabilities only exploitable on out-of-date browsers or platforms. -- Attacks requiring physical access to the victim device. -- Incorrect data supplied by third party oracles. - -### Blockchain: - -- Basic economic governance attacks (e.g. 51% attack). -- Best practice critiques. -- Missing or incorrect data in events. -- Sybil attacks. -- Centralization risk. - -### Smart Contracts: - -- Incorrect data supplied by third-party oracles (not to exclude oracle manipulation/flash loan attacks; use of such methods to generate critical impacts remain in-scope for this program). -- Basic economic governance attacks (e.g., 51% attack). -- Lack of liquidity. -- Best practice critiques. -- Missing or incorrect data in events. -- Incorrect naming (but still correct data) in contracts. -- Minor rounding errors that don’t lead to substantial loss of funds. - -## Exclusions - -The following activities are prohibited by this bug bounty program: - -- Any testing with mainnet or public testnet contracts; all testing should be done on [private testnets](https://aptos.dev/nodes/localnet/). -- Any testing with pricing oracles or third party smart contracts. -- Attempting to phish or otherwise use social engineering attacks against contributors, employees, and/or customers. -- Any testing with third-party systems and applications (e.g., browser extensions) as well as websites (e.g., SSO providers, advertising networks). -- Any denial of service attacks. -- Automated testing of services that generates significant amounts of traffic. -- Public disclosure of an unpatched vulnerability in an embargoed bounty. +To learn more visit the [Aptos Foundation Bounty Program](https://hackenproof.com/aptos-labs/aptos-network) page. From 1fa9142d415fd5c81152d81499f57b9f73282acb Mon Sep 17 00:00:00 2001 From: Balaji Arun Date: Thu, 6 Jun 2024 13:43:48 -0700 Subject: [PATCH 25/26] [telemetry-svc] support for basic auth type in metrics (#12947) --- Cargo.lock | 8 ++-- .../src/clients/mod.rs | 46 ++++++++++++++++--- crates/aptos-telemetry-service/src/lib.rs | 7 +-- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f112f469fb186..9d3a7dfc78a31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8504,9 +8504,9 @@ checksum = "92620684d99f750bae383ecb3be3748142d6095760afd5cbcf2261e9a279d780" [[package]] name = "h2" -version = "0.3.26" +version = "0.3.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81fe527a889e1532da5c525686d96d4c2e74cdd345badf8dfef9f6b39dd5f5e8" +checksum = "bb2c4422095b67ee78da96fbb51a4cc413b3b25883c7717ff7ca1ab31022c9c9" dependencies = [ "bytes", "fnv", @@ -16571,9 +16571,9 @@ dependencies = [ [[package]] name = "whoami" -version = "1.5.0" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fec781d48b41f8163426ed18e8fc2864c12937df9ce54c88ede7bd47270893e" +checksum = "a44ab49fad634e88f55bf8f9bb3abd2f27d7204172a112c7c9987e01c1c94ea9" dependencies = [ "redox_syscall 0.4.1", "wasite", diff --git a/crates/aptos-telemetry-service/src/clients/mod.rs b/crates/aptos-telemetry-service/src/clients/mod.rs index cdfce66ce7531..e96d697f90d52 100644 --- a/crates/aptos-telemetry-service/src/clients/mod.rs +++ b/crates/aptos-telemetry-service/src/clients/mod.rs @@ -12,16 +12,43 @@ pub mod victoria_metrics_api { use url::Url; use warp::hyper::body::Bytes; + #[derive(Clone)] + pub enum AuthToken { + Bearer(String), + Basic(String, String), + } + + impl From<&String> for AuthToken { + fn from(token: &String) -> Self { + // TODO(ibalajiarun): Auth type must be read from config + if token.split(':').count() == 2 { + let mut parts = token.split(':'); + AuthToken::Basic( + parts.next().unwrap().to_string(), + parts.next().unwrap().to_string(), + ) + } else { + AuthToken::Bearer(token.to_string()) + } + } + } + + impl From<&str> for AuthToken { + fn from(token: &str) -> Self { + AuthToken::from(&token.to_string()) + } + } + /// Client implementation to export metrics to Victoria Metrics #[derive(Clone)] pub struct Client { inner: ClientWithMiddleware, base_url: Url, - auth_token: String, + auth_token: AuthToken, } impl Client { - pub fn new(base_url: Url, auth_token: String) -> Self { + pub fn new(base_url: Url, auth_token: AuthToken) -> Self { let retry_policy = ExponentialBackoff::builder().build_with_max_retries(3); let inner = ClientBuilder::new(ReqwestClient::new()) .with(RetryTransientMiddleware::new_with_policy(retry_policy)) @@ -44,10 +71,17 @@ pub mod victoria_metrics_api { .map(|label| ("extra_label".into(), label.into())) .collect(); - self.inner - .post(format!("{}api/v1/import/prometheus", self.base_url)) - .bearer_auth(self.auth_token.clone()) - .header(CONTENT_ENCODING, encoding) + let req = self + .inner + .post(format!("{}api/v1/import/prometheus", self.base_url)); + let req = match &self.auth_token { + AuthToken::Bearer(token) => req.bearer_auth(token.clone()), + AuthToken::Basic(username, password) => { + req.basic_auth(username.clone(), Some(password.clone())) + }, + }; + + req.header(CONTENT_ENCODING, encoding) .query(&labels) .body(raw_metrics_body) .send() diff --git a/crates/aptos-telemetry-service/src/lib.rs b/crates/aptos-telemetry-service/src/lib.rs index 3128e5dbd547d..6b8210fed69e0 100644 --- a/crates/aptos-telemetry-service/src/lib.rs +++ b/crates/aptos-telemetry-service/src/lib.rs @@ -197,13 +197,10 @@ impl MetricsEndpoint { panic!( "environment variable {} is missing secret for {}", self.keys_env_var.clone(), - name, + name ) }); - ( - name.clone(), - MetricsClient::new(url.clone(), secret.clone()), - ) + (name.clone(), MetricsClient::new(url.clone(), secret.into())) }) .collect() } From b3c676a4c78a56a96bc04023c5142018cfdb6496 Mon Sep 17 00:00:00 2001 From: Alden Hu Date: Thu, 6 Jun 2024 15:35:02 -0700 Subject: [PATCH 26/26] backup-cli: ignore metadata file download failure (#13599) --- .../backup/backup-cli/src/metadata/cache.rs | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/storage/backup/backup-cli/src/metadata/cache.rs b/storage/backup/backup-cli/src/metadata/cache.rs index a04ba7bd97883..f7eb9c93463e2 100644 --- a/storage/backup/backup-cli/src/metadata/cache.rs +++ b/storage/backup/backup-cli/src/metadata/cache.rs @@ -154,19 +154,30 @@ pub async fn sync_and_load( let local_file = cache_dir_ref.join(*h); let local_tmp_file = cache_dir_ref.join(format!(".{}", *h)); - download_file(storage_ref, file_handle, &local_tmp_file).await?; - // rename to target file only if successful; stale tmp file caused by failure will be - // reclaimed on next run - tokio::fs::rename(local_tmp_file.clone(), local_file) - .await - .err_notes(local_tmp_file)?; - info!( - file_handle = file_handle, - processed = i + 1, - total = num_new_files, - "Metadata file downloaded." - ); - NUM_META_DOWNLOAD.inc(); + match download_file(storage_ref, file_handle, &local_tmp_file).await { + Ok(_) => { + // rename to target file only if successful; stale tmp file caused by failure will be + // reclaimed on next run + tokio::fs::rename(local_tmp_file.clone(), local_file) + .await + .err_notes(local_tmp_file)?; + info!( + file_handle = file_handle, + processed = i + 1, + total = num_new_files, + "Metadata file downloaded." + ); + NUM_META_DOWNLOAD.inc(); + }, + Err(e) => { + warn!( + file_handle = file_handle, + error = %e, + "Ignoring metadata file download error -- can be compactor removing files." + ) + }, + } + Ok(()) } });