[aptos-release-tooling] Add an option to compare release binary with …

…on chain configs (aptos-labs#5796)

* [aptos-release-tooling] Add an option to compare release binary with on chain configs

* fixup! [aptos-release-tooling] Add an option to compare release binary with on chain configs

aptos-move/aptos-release-builder/Cargo.toml

@@ -16,15 +16,19 @@ rust-version = { workspace = true }
 anyhow = { workspace = true }
 aptos-crypto = { workspace = true }
 aptos-gas = { workspace = true }
+aptos-rest-client = { workspace = true }
 aptos-temppath = { workspace = true }
 aptos-types = { workspace = true }
 bcs = { workspace = true }
 clap = { workspace = true }
+futures = { workspace = true }
 move-core-types = { workspace = true }
 move-model = { workspace = true }
 serde = { workspace = true }
 serde_yaml = { workspace = true }
 tempfile = { workspace = true }
+tokio = { workspace = true }
+url = { workspace = true }
 
 [[bin]]
 name = "aptos-release-builder"

aptos-move/aptos-release-builder/src/components/feature_flags.rs

@@ -3,7 +3,7 @@
 
 use crate::utils::*;
 use anyhow::Result;
-use aptos_types::on_chain_config::FeatureFlag as AFeatureFlag;
+use aptos_types::on_chain_config::{FeatureFlag as AptosFeatureFlag, Features as AptosFeatures};
 use move_model::{code_writer::CodeWriter, emit, emitln, model::Loc};
 use serde::{Deserialize, Serialize};
 
@@ -50,12 +50,12 @@ pub fn generate_feature_upgrade_proposal(
     let enabled = features
         .enabled
         .iter()
-        .map(|f| AFeatureFlag::from(f.clone()) as u64)
+        .map(|f| AptosFeatureFlag::from(f.clone()) as u64)
         .collect::<Vec<_>>();
     let disabled = features
         .disabled
         .iter()
-        .map(|f| AFeatureFlag::from(f.clone()) as u64)
+        .map(|f| AptosFeatureFlag::from(f.clone()) as u64)
         .collect::<Vec<_>>();
 
     assert!(enabled.len() < u16::MAX as usize);
@@ -88,23 +88,36 @@ pub fn generate_feature_upgrade_proposal(
     Ok(result)
 }
 
-impl From<FeatureFlag> for AFeatureFlag {
+impl From<FeatureFlag> for AptosFeatureFlag {
     fn from(f: FeatureFlag) -> Self {
         match f {
-            FeatureFlag::CodeDependencyCheck => AFeatureFlag::CODE_DEPENDENCY_CHECK,
-            FeatureFlag::TreatFriendAsPrivate => AFeatureFlag::TREAT_FRIEND_AS_PRIVATE,
-            FeatureFlag::VMBinaryFormatV6 => AFeatureFlag::VM_BINARY_FORMAT_V6,
+            FeatureFlag::CodeDependencyCheck => AptosFeatureFlag::CODE_DEPENDENCY_CHECK,
+            FeatureFlag::TreatFriendAsPrivate => AptosFeatureFlag::TREAT_FRIEND_AS_PRIVATE,
+            FeatureFlag::VMBinaryFormatV6 => AptosFeatureFlag::VM_BINARY_FORMAT_V6,
         }
     }
 }
 
 // We don't need this implementation. Just to make sure we have an exhaustive 1-1 mapping between the two structs.
-impl From<AFeatureFlag> for FeatureFlag {
-    fn from(f: AFeatureFlag) -> Self {
+impl From<AptosFeatureFlag> for FeatureFlag {
+    fn from(f: AptosFeatureFlag) -> Self {
         match f {
-            AFeatureFlag::CODE_DEPENDENCY_CHECK => FeatureFlag::CodeDependencyCheck,
-            AFeatureFlag::TREAT_FRIEND_AS_PRIVATE => FeatureFlag::TreatFriendAsPrivate,
-            AFeatureFlag::VM_BINARY_FORMAT_V6 => FeatureFlag::VMBinaryFormatV6,
+            AptosFeatureFlag::CODE_DEPENDENCY_CHECK => FeatureFlag::CodeDependencyCheck,
+            AptosFeatureFlag::TREAT_FRIEND_AS_PRIVATE => FeatureFlag::TreatFriendAsPrivate,
+            AptosFeatureFlag::VM_BINARY_FORMAT_V6 => FeatureFlag::VMBinaryFormatV6,
         }
     }
 }
+
+impl Features {
+    // Compare if the current feature set is different from features that has been enabled on chain.
+    pub(crate) fn has_modified(&self, on_chain_features: &AptosFeatures) -> bool {
+        self.enabled
+            .iter()
+            .any(|f| !on_chain_features.is_enabled(AptosFeatureFlag::from(f.clone())))
+            || self
+                .disabled
+                .iter()
+                .any(|f| on_chain_features.is_enabled(AptosFeatureFlag::from(f.clone())))
+    }
+}

aptos-move/aptos-release-builder/src/components/mod.rs

@@ -4,13 +4,19 @@
 use crate::components::feature_flags::Features;
 use anyhow::{anyhow, Result};
 use aptos_crypto::HashValue;
-use aptos_types::on_chain_config::{GasScheduleV2, OnChainConsensusConfig, Version};
+use aptos_rest_client::Client;
+use aptos_types::{
+    account_config::CORE_CODE_ADDRESS,
+    on_chain_config::{GasScheduleV2, OnChainConfig, OnChainConsensusConfig, Version},
+};
+use futures::executor::block_on;
 use serde::{Deserialize, Serialize};
 use std::{
     fs::File,
     io::{Read, Write},
     path::Path,
 };
+use url::Url;
 
 pub mod consensus_config;
 pub mod feature_flags;
@@ -21,6 +27,7 @@ pub mod version;
 #[derive(Serialize, Deserialize, Clone, Eq, PartialEq)]
 pub struct ReleaseConfig {
     pub testnet: bool,
+    pub remote_endpoint: Option<Url>,
     pub framework_release: bool,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub gas_schedule: Option<GasScheduleV2>,
@@ -34,16 +41,53 @@ pub struct ReleaseConfig {
     pub is_multi_step: bool,
 }
 
+// Compare the current on chain config with the value recorded on chain. Return false if there's a difference.
+fn fetch_and_equals<T: OnChainConfig + PartialEq>(
+    client: &Option<Client>,
+    expected: &T,
+) -> Result<bool> {
+    match client {
+        Some(client) => {
+            let config = T::deserialize_into_config(
+                block_on(async {
+                    client
+                        .get_account_resource_bytes(
+                            CORE_CODE_ADDRESS,
+                            format!(
+                                "{}::{}::{}",
+                                T::ADDRESS,
+                                T::MODULE_IDENTIFIER,
+                                T::TYPE_IDENTIFIER
+                            )
+                            .as_str(),
+                        )
+                        .await
+                })?
+                .inner(),
+            )?;
+
+            Ok(&config == expected)
+        }
+        None => Ok(false),
+    }
+}
+
 impl ReleaseConfig {
     pub fn generate_release_proposal_scripts(&self, base_path: &Path) -> Result<()> {
         let mut result: Vec<(String, String)> = vec![];
-        let mut release_generation_functions: Vec<&dyn Fn(&Self, &mut Vec<(String, String)>)> = vec![
+        let mut release_generation_functions: Vec<
+            &dyn Fn(&Self, &Option<Client>, &mut Vec<(String, String)>) -> Result<()>,
+        > = vec![
             &Self::generate_framework_release,
             &Self::generate_gas_schedule,
             &Self::generate_version_file,
             &Self::generate_feature_flag_file,
             &Self::generate_consensus_file,
         ];
+        let client = self
+            .remote_endpoint
+            .as_ref()
+            .map(|url| Client::new(url.clone()));
 
         // If we are generating multi-step proposal files, we generate the files in reverse order,
         // since we need to pass in the hash of the next file to the previous file.
@@ -52,7 +96,7 @@ impl ReleaseConfig {
         }
 
         for f in &release_generation_functions {
-            (f)(self, &mut result);
+            (f)(self, &client, &mut result)?;
         }
 
         // Here we are reversing the results back, so the result would be in order.
@@ -73,88 +117,119 @@ impl ReleaseConfig {
         Ok(())
     }
 
-    fn generate_framework_release(&self, result: &mut Vec<(String, String)>) {
+    fn generate_framework_release(
+        &self,
+        _client: &Option<Client>,
+        result: &mut Vec<(String, String)>,
+    ) -> Result<()> {
         if self.framework_release {
-            result.append(
-                &mut framework::generate_upgrade_proposals(
-                    self.testnet,
-                    if self.is_multi_step {
-                        Self::get_execution_hash(result)
-                    } else {
-                        "".to_owned()
-                    },
-                )
-                .unwrap(),
-            );
+            result.append(&mut framework::generate_upgrade_proposals(
+                self.testnet,
+                if self.is_multi_step {
+                    Self::get_execution_hash(result)
+                } else {
+                    "".to_owned()
+                },
+            )?);
         }
+        Ok(())
     }
 
-    fn generate_gas_schedule(&self, result: &mut Vec<(String, String)>) {
+    fn generate_gas_schedule(
+        &self,
+        client: &Option<Client>,
+        result: &mut Vec<(String, String)>,
+    ) -> Result<()> {
         if let Some(gas_schedule) = &self.gas_schedule {
-            result.append(
-                &mut gas::generate_gas_upgrade_proposal(
+            if !fetch_and_equals::<GasScheduleV2>(client, gas_schedule)? {
+                result.append(&mut gas::generate_gas_upgrade_proposal(
                     gas_schedule,
                     self.testnet,
                     if self.is_multi_step {
                         Self::get_execution_hash(result)
                     } else {
                         "".to_owned()
                     },
-                )
-                .unwrap(),
-            );
+                )?);
+            }
         }
+        Ok(())
     }
 
-    fn generate_version_file(&self, result: &mut Vec<(String, String)>) {
+    fn generate_version_file(
+        &self,
+        client: &Option<Client>,
+        result: &mut Vec<(String, String)>,
+    ) -> Result<()> {
         if let Some(version) = &self.version {
-            result.append(
-                &mut version::generate_version_upgrade_proposal(
+            if !fetch_and_equals::<Version>(client, version)? {
+                result.append(&mut version::generate_version_upgrade_proposal(
                     version,
                     self.testnet,
                     if self.is_multi_step {
                         Self::get_execution_hash(result)
                     } else {
                         "".to_owned()
                     },
-                )
-                .unwrap(),
-            );
+                )?);
+            }
         }
+        Ok(())
     }
 
-    fn generate_feature_flag_file(&self, result: &mut Vec<(String, String)>) {
+    fn generate_feature_flag_file(
+        &self,
+        client: &Option<Client>,
+        result: &mut Vec<(String, String)>,
+    ) -> Result<()> {
         if let Some(feature_flags) = &self.feature_flags {
-            result.append(
-                &mut feature_flags::generate_feature_upgrade_proposal(
+            let mut needs_update = false;
+            if let Some(client) = client {
+                let features = block_on(async {
+                    client
+                        .get_account_resource_bcs::<aptos_types::on_chain_config::Features>(
+                            CORE_CODE_ADDRESS,
+                            "0x1::features::Features",
+                        )
+                        .await
+                })?;
+                // Only update the feature flags section when there's a divergence between the local configs and on chain configs.
+                needs_update = feature_flags.has_modified(features.inner());
+            }
+            if needs_update {
+                result.append(&mut feature_flags::generate_feature_upgrade_proposal(
                     feature_flags,
                     self.testnet,
                     if self.is_multi_step {
                         Self::get_execution_hash(result)
                     } else {
                         "".to_owned()
                     },
-                )
-                .unwrap(),
-            );
+                )?);
+            }
         }
+        Ok(())
     }
 
-    fn generate_consensus_file(&self, result: &mut Vec<(String, String)>) {
+    fn generate_consensus_file(
+        &self,
+        client: &Option<Client>,
+        result: &mut Vec<(String, String)>,
+    ) -> Result<()> {
         if let Some(consensus_config) = &self.consensus_config {
-            result.append(
-                &mut consensus_config::generate_consensus_upgrade_proposal(
+            if fetch_and_equals(client, consensus_config)? {
+                result.append(&mut consensus_config::generate_consensus_upgrade_proposal(
                     consensus_config,
                     self.testnet,
                     if self.is_multi_step {
                         Self::get_execution_hash(result)
                     } else {
                         "".to_owned()
                     },
-                )
-                .unwrap(),
-            );
+                )?);
+            }
         }
+        Ok(())
     }
 
     pub fn load_config<P: AsRef<Path>>(path: P) -> Result<Self> {
@@ -213,6 +288,7 @@ impl Default for ReleaseConfig {
             feature_flags: None,
             consensus_config: Some(OnChainConsensusConfig::default()),
             is_multi_step: false,
+            remote_endpoint: None,
         }
     }
 }

aptos-move/aptos-release-builder/src/main.rs

@@ -25,7 +25,8 @@ pub enum Commands {
     },
 }
 
-fn main() -> Result<()> {
+#[tokio::main]
+async fn main() -> Result<()> {
     let args = Argument::parse();
 
     // TODO: Being able to parse the release config from a TOML file to generate the proposals.

crates/aptos-rest-client/src/lib.rs

@@ -1026,6 +1026,17 @@ impl Client {
         Ok(response.map(|inner| inner.to_vec()))
     }
 
+    pub async fn get_account_resource_bytes(
+        &self,
+        address: AccountAddress,
+        resource_type: &str,
+    ) -> AptosResult<Response<Vec<u8>>> {
+        let url = self.build_path(&format!("accounts/{}/resource/{}", address, resource_type))?;
+
+        let response = self.get_bcs(url).await?;
+        Ok(response.map(|inner| inner.to_vec()))
+    }
+
     pub async fn get_account_resource_at_version(
         &self,
         address: AccountAddress,