[upgrades] git commit hash included on-chain on upgrades (#80)

Co-authored-by: 0o-de-lally <[email protected]>

framework/Cargo.toml

@@ -17,7 +17,6 @@ anyhow = { workspace = true }
 bcs = { workspace = true }
 clap = { workspace = true }
 dialoguer = { workspace = true }
-
 diem = { workspace = true }
 diem-build-info = { workspace = true }
 diem-crypto = { workspace = true }

framework/libra-framework/sources/configs/version.move

@@ -12,6 +12,10 @@ module diem_framework::version {
         major: u64,
     }
 
+    struct Git has key {
+      hash: vector<u8>
+    }
+
     struct SetVersionCapability has key {}
 
     /// Specified major version number must be greater than current version number.
@@ -24,7 +28,7 @@ module diem_framework::version {
     public(friend) fun initialize(diem_framework: &signer, initial_version: u64) {
         system_addresses::assert_diem_framework(diem_framework);
 
-        move_to(diem_framework, Version { major: initial_version });
+        move_to(diem_framework, Version { major: initial_version});
         // Give diem framework account capability to call set version. This allows on chain governance to do it through
         // control of the diem framework account.
         move_to(diem_framework, SetVersionCapability {});
@@ -45,6 +49,19 @@ module diem_framework::version {
         reconfiguration::reconfigure();
     }
 
+    /// set the git commit of a current upgrade.
+    // NOTE: easier to troublshoot than the code.move manifests
+    public fun upgrade_set_git(framework: &signer, hash: vector<u8>) acquires Git {
+      if (!exists<Git>(@ol_framework)) {
+        move_to(framework, Git {
+          hash,
+        })
+      } else {
+        let state = borrow_global_mut<Git>(@ol_framework);
+        state.hash = hash;
+      }
+    }
+
     /// Only called in tests and testnets. This allows the core resources account, which only exists in tests/testnets,
     /// to update the version.
     fun initialize_for_test(core_resources: &signer) {

framework/libra-framework/sources/modified_source/diem_governance.move

@@ -520,14 +520,12 @@ module diem_framework::diem_governance {
     }
 
     #[view]
-    // what is the state of the proposal
+    /// what is the state of the proposal
     public fun get_proposal_state(proposal_id: u64):u64  {
       voting::get_proposal_state<GovernanceProposal>(@diem_framework, proposal_id)
     }
 
 
-
-
     //////// 0L ////////
     // hack for smoke testing:
     // is the proposal approved and ready for resolution?

framework/src/builder/framework_generate_upgrade_proposal.rs

@@ -1,13 +1,12 @@
 //! generate framework upgrade proposal scripts
 //! see vendor diem-move/framework/src/release_bundle.rs
 
+use crate::{builder::framework_release_bundle::libra_author_script_file, BYTECODE_VERSION};
 use anyhow::{ensure, Context, Result};
 use diem_crypto::HashValue;
 use diem_framework::{BuildOptions, BuiltPackage, ReleasePackage};
 use diem_types::account_address::AccountAddress;
 use std::path::{Path, PathBuf};
-// use serde::{Serialize, Deserialize};
-use crate::{builder::framework_release_bundle::libra_author_script_file, BYTECODE_VERSION};
 
 /// Core modules address to deploy to
 // NOTE: we are always usin 0x1 here. So if that ever changes in the future then this can't be hard coded.
@@ -35,20 +34,10 @@ pub fn make_framework_upgrade_artifacts(
     let mut next_execution_hash = vec![];
 
     let mut core_modules = core_modules.to_owned().unwrap_or_else(default_core_modules);
-    // 0L TODO: don't make this hard coded
-    // let mut core_modules = vec![
-    //     ("0x1", "move-stdlib"),
-    //     // ("0x1", "vendor-stdlib"),
-    //     // ("0x1", "libra-framework"),
-    //     // ("0x3", "diem-move/framework/diem-token"),
-    //     // ("0x4", "diem-move/framework/diem-token-objects"),
-    // ];
 
     // TODO: we are not using these formatted files now that we are saving them directly
     let mut formatted_scripts: Vec<(String, String)> = vec![];
 
-    // let commit_info = diem_build_info::get_git_hash();
-
     // For generating multi-step proposal files, we need to generate them in the reverse order since
     // we need the hash of the next script.
     // We will reverse the order back when writing the files into a directory.
@@ -63,9 +52,6 @@ pub fn make_framework_upgrade_artifacts(
         let mut core_module_dir = framework_local_dir.to_owned().canonicalize()?;
         core_module_dir.push(core_module_name);
 
-        // the governance script name
-        // let core_module_name = core_module_dir.file_name().unwrap().to_str().unwrap();
-
         // We first need to compile and build each CORE MODULE we are upgrading (e.g. MoveStdlib, LibraFramework)
 
         let options = BuildOptions {

framework/src/builder/framework_release_bundle.rs

@@ -55,14 +55,8 @@ pub fn libra_author_script_file(
     emitln!(writer, "use std::vector;");
     emitln!(writer, "use diem_framework::diem_governance;");
     emitln!(writer, "use diem_framework::code;\n");
+    emitln!(writer, "use diem_framework::version;\n");
 
-    // emitln!(writer, "fun main(proposal_id: u64){");
-    // writer.indent();
-    // emitln!(
-    //     writer,
-    //     "let framework_signer = diem_governance::resolve(proposal_id, @{});",
-    //     for_address
-    // );
     emitln!(writer, "fun main(proposal_id: u64){");
     writer.indent();
     // This is the multi step proposal, needs a next hash even if it a single step and thus an empty vec.
@@ -104,8 +98,15 @@ pub fn libra_author_script_file(
 
     emitln!(
         writer,
-        "code::publish_package_txn(&framework_signer, chunk1, code)"
+        "code::publish_package_txn(&framework_signer, chunk1, code);"
     );
+
+    emitln!(
+        writer,
+        "version::upgrade_set_git(&framework_signer, x\"{}\")",
+        diem_build_info::get_git_hash()
+    );
+
     writer.unindent();
     emitln!(writer, "}");
     writer.unindent();

smoke-tests/src/upgrade_fixtures.rs

@@ -3,6 +3,7 @@
 use std::path::PathBuf;
 use std::str::FromStr;
 
+use anyhow::Context;
 use libra_framework::builder::framework_generate_upgrade_proposal::make_framework_upgrade_artifacts;
 
 // TODO: This could be generated dynamically at the start of the test suites. using `Once`. Though if the tools aren't compiled it will take approximately forever to do so. Hence fixtures, though not ideal.
@@ -29,7 +30,7 @@ pub fn insert_test_file(core_module_name: &str, remove: bool) -> anyhow::Result<
     }
 
     let file_path = this_crate
-        .join("src")
+        // .join("src")
         .join("tests")
         .join("fixtures")
         .join("all_your_base.move");
@@ -42,10 +43,13 @@ pub fn insert_test_file(core_module_name: &str, remove: bool) -> anyhow::Result<
 pub fn generate_fixtures(output_path: PathBuf, modules: Vec<String>) -> anyhow::Result<()> {
     println!("generating files, this will take some time, go do some laundry");
     let destination_module = modules.last().unwrap().clone();
-    insert_test_file(&destination_module, false)?;
+    insert_test_file(&destination_module, false).context("could not insert test file")?;
 
-    let this_crate = PathBuf::from_str(env!("CARGO_MANIFEST_DIR")).unwrap();
-    let libra_framework_sources = this_crate.parent().unwrap().join("framework");
+    let this_crate = PathBuf::from_str(env!("CARGO_MANIFEST_DIR"))?;
+    let libra_framework_sources = this_crate
+        .parent()
+        .context("no parent dir")?
+        .join("framework");
 
     make_framework_upgrade_artifacts(&output_path, &libra_framework_sources, &Some(modules))?;
     // ok, cleanup

smoke-tests/tests/fixtures/upgrade_multi_step/1-move-stdlib/script_sha3

@@ -1 +1 @@
-b83240b68e496aaf2ff6840bf46935cd93f4460a791096ee5725a67da12eee32
+a6e7b34be7e67801f1dcd0087cfbe7fd721bbf45596dae3ee5c4439c180c2b65

smoke-tests/tests/fixtures/upgrade_multi_step/1-move-stdlib/sources/1-move-stdlib.move

@@ -1,21 +1,23 @@
 // Upgrade proposal for package `MoveStdlib`
 
-// Framework commit hash: 5b1067cb80b8bb3e5d3e3394e915815a03586cac
-// Builder commit hash: 5b1067cb80b8bb3e5d3e3394e915815a03586cac
+// Framework commit hash: bafac94d6edd39d972729db21156d47758eb8969
+// Builder commit hash: bafac94d6edd39d972729db21156d47758eb8969
 
-// Next step script hash: 7e30938cc147d4cf29adcad8110b39b76c81b81d785df911fc79da06416ec6be
+// Next step script hash: f05bdad0508955fc2044f83c307228ac589249b025430ee664bedf61ddbf179b
 
 // source digest: B7AED5C969B8CA9DF201BC95AB87937D7A060ECFDC7E715CE3A0F6450AB8AD71
 script {
     use std::vector;
     use diem_framework::diem_governance;
     use diem_framework::code;
 
+    use diem_framework::version;
+
     fun main(proposal_id: u64){
         let framework_signer = diem_governance::resolve_multi_step_proposal(
             proposal_id,
             @0000000000000000000000000000000000000000000000000000000000000001,
-            vector[126u8,48u8,147u8,140u8,193u8,71u8,212u8,207u8,41u8,173u8,202u8,216u8,17u8,11u8,57u8,183u8,108u8,129u8,184u8,29u8,120u8,93u8,249u8,17u8,252u8,121u8,218u8,6u8,65u8,110u8,198u8,190u8,],
+            vector[240u8,91u8,218u8,208u8,80u8,137u8,85u8,252u8,32u8,68u8,248u8,60u8,48u8,114u8,40u8,172u8,88u8,146u8,73u8,176u8,37u8,67u8,14u8,230u8,100u8,190u8,223u8,97u8,221u8,191u8,23u8,155u8,],
         );
         let code = vector::empty();
         let chunk0 =
@@ -632,6 +634,7 @@ script {
             110u8,101u8,114u8,0u8,0u8,0u8,8u8,102u8,101u8,97u8,116u8,117u8,114u8,101u8,115u8,0u8,0u8,0u8,6u8,111u8,
             112u8,116u8,105u8,111u8,110u8,0u8,0u8,0u8,6u8,115u8,116u8,114u8,105u8,110u8,103u8,0u8,0u8,0u8,0u8,0u8,
         ];
-        code::publish_package_txn(&framework_signer, chunk1, code)
+        code::publish_package_txn(&framework_signer, chunk1, code);
+        version::upgrade_set_git(&framework_signer, x"bafac94d6edd39d972729db21156d47758eb8969")
     }
 }

smoke-tests/tests/fixtures/upgrade_multi_step/2-vendor-stdlib/script_sha3

@@ -1 +1 @@
-015988a1f5402ce5b4489b14fd9cc6274fc9351e23c0d062325616ef2d635a5f
+f05bdad0508955fc2044f83c307228ac589249b025430ee664bedf61ddbf179b