[read/write set analysis] fix bug in access path trie

This bug breaks `get_keys_read` and `get_keys_written` in the dynamic analysis, so it's important to fix. I also expanded the RW-set API exposed by the Move CLI to allow calling these functions + added tests to confirm the fix.

Closes: aptos-labs#8751

language/diem-tools/df-cli/tests/testsuite/concretize_read_write_set_smoke/args.exp

@@ -4,7 +4,7 @@ Command `sandbox run storage/0x00000000000000000000000000000001/modules/AccountC
 Command `sandbox run  storage/0x00000000000000000000000000000001/modules/AccountCreationScripts.mv create_parent_vasp_account --mode diem --type-args 0x1::XDX::XDX --signers 0xB1E55ED --args 0 0xB x"00000000000000000000000000000000" b"VASP_B" true`:
 Command `sandbox run storage/0x00000000000000000000000000000001/modules/TreasuryComplianceScripts.mv tiered_mint --mode diem --type-args 0x1::XUS::XUS --signers 0xB1E55ED --args 0 0xDD 1000 0`:
 Command `sandbox run storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --type-args 0x1::XUS::XUS --signers 0xDD --args 0xA 700 x"" x""`:
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize paths --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""`:
 0xa/0x1::AccountFreezing::FreezingBit: Read
 0xa/0x1::AccountFreezing::FreezingBit/is_frozen: Read
 0xa/0x1::VASP::ParentVASP: Read
@@ -41,3 +41,21 @@ Command `experimental read-write-set storage/0x00000000000000000000000000000001/
 0xa550c18/0x1::DualAttestation::Limit: Read
 0xa550c18/0x1::DualAttestation::Limit/micro_xdx_limit: Read
 
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize reads --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""`:
+0xa/0x1::AccountFreezing::FreezingBit
+0xa/0x1::VASP::ParentVASP
+0xa/0x1::DiemAccount::DiemAccount
+0xa/0x1::DiemAccount::Balance<0x1::XUS::XUS>
+0xb/0x1::AccountFreezing::FreezingBit
+0xb/0x1::VASP::ParentVASP
+0xb/0x1::DualAttestation::Credential
+0xb/0x1::DiemAccount::DiemAccount
+0xb/0x1::DiemAccount::Balance<0x1::XUS::XUS>
+0xa550c18/0x1::DiemTimestamp::CurrentTimeMicroseconds
+0xa550c18/0x1::Diem::CurrencyInfo<0x1::XUS::XUS>
+0xa550c18/0x1::DualAttestation::Limit
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize writes --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""`:
+0xa/0x1::DiemAccount::DiemAccount
+0xa/0x1::DiemAccount::Balance<0x1::XUS::XUS>
+0xb/0x1::DiemAccount::DiemAccount
+0xb/0x1::DiemAccount::Balance<0x1::XUS::XUS>

language/diem-tools/df-cli/tests/testsuite/concretize_read_write_set_smoke/args.txt

@@ -18,5 +18,11 @@ sandbox run storage/0x00000000000000000000000000000001/modules/TreasuryComplianc
 # transfer 700 XUS from 0xDD to 0xA
 sandbox run storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --type-args 0x1::XUS::XUS --signers 0xDD --args 0xA 700 x"" x""
 
-# transfer 500 XUS from 0xA to 0xB
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""
+# get concretized read/write set for a transfer
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize paths --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""
+
+# same thing, but get resources read only
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize reads --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""
+
+# same thing, but get resources written only
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/PaymentScripts.mv peer_to_peer_with_metadata --mode diem --concretize writes --type-args 0x1::XUS::XUS --signers 0xA --args 0xB 500 x"" x""

language/move-core/types/src/language_storage.rs

@@ -151,3 +151,9 @@ impl Display for TypeTag {
         }
     }
 }
+
+impl Display for ResourceKey {
+    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
+        write!(f, "0x{}/{}", self.address.short_str_lossless(), self.type_)
+    }
+}

language/move-prover/bytecode/src/access_path_trie.rs

@@ -69,23 +69,21 @@ impl<T: FootprintDomain> TrieNode<T> {
         }
     }
 
-    /// Like join, but gracefully handles `Non` data fields by treating None as Bottom
+    /// Like join, but gracefully handles `None` data fields by treating None as Bottom
     pub fn join_data_opt(&mut self, other: &Option<T>) -> JoinResult {
         Self::join_data_opt_(&mut self.data, other)
     }
 
     pub fn join_child_data(&self, mut acc: Option<T>) -> Option<T> {
         Self::join_data_opt_(&mut acc, &self.data);
         for v in self.children.values() {
-            Self::join_data_opt_(&mut acc, &v.data);
+            acc = v.join_child_data(acc)
         }
         acc
     }
 
     pub fn get_child_data(&self) -> Option<T> {
-        let mut acc = None;
-        Self::join_data_opt_(&mut acc, &self.data);
-        acc
+        self.join_child_data(None)
     }
 
     pub fn data(&self) -> &Option<T> {

language/tools/move-cli/src/experimental/commands/read_writeset_analysis.rs

@@ -11,6 +11,7 @@ use move_core_types::{
     transaction_argument::{convert_txn_args, TransactionArgument},
 };
 
+use crate::ConcretizeMode;
 use anyhow::{anyhow, Result};
 use std::fs;
 
@@ -21,7 +22,7 @@ pub fn analyze_read_write_set(
     signers: &[String],
     txn_args: &[TransactionArgument],
     type_args: &[TypeTag],
-    concretize: bool,
+    concretize: ConcretizeMode,
     verbose: bool,
 ) -> Result<()> {
     let module_id = CompiledModule::deserialize(&fs::read(module_file)?)
@@ -40,31 +41,59 @@ pub fn analyze_read_write_set(
     let modules = dep_graph.compute_topological_order()?;
     let rw = read_write_set::analyze(modules)?;
     if let Some(fenv) = rw.get_function_env(&module_id, &fun_id) {
-        if concretize {
-            let signer_addresses = signers
-                .iter()
-                .map(|s| AccountAddress::from_hex_literal(&s))
-                .collect::<Result<Vec<AccountAddress>, _>>()?;
-            // TODO: parse Value's directly instead of going through the indirection of TransactionArgument?
-            let script_args: Vec<Vec<u8>> = convert_txn_args(&txn_args);
-            // substitute given script arguments + blockchain state into abstract r/w set
-            // safe to unwrap here because every function must be analyzed
-            let results = rw.get_concretized_summary(
-                &module_id,
-                &fun_id,
-                &signer_addresses,
-                &script_args,
-                type_args,
-                state,
-            )?;
-            println!("{}", results.display(&fenv))
-        } else {
-            // don't try try to concretize; just print the R/W set
-            // safe to unwrap here because every function must be analyzed
-            let results = rw.get_summary(&module_id, &fun_id).expect(
-                "Invariant violation: couldn't resolve R/W set summary for defined function",
-            );
-            println!("{}", results.display(&fenv))
+        let signer_addresses = signers
+            .iter()
+            .map(|s| AccountAddress::from_hex_literal(&s))
+            .collect::<Result<Vec<AccountAddress>, _>>()?;
+        // TODO: parse Value's directly instead of going through the indirection of TransactionArgument?
+        let script_args: Vec<Vec<u8>> = convert_txn_args(&txn_args);
+        // substitute given script arguments + blockchain state into abstract r/w set
+        match concretize {
+            ConcretizeMode::Paths => {
+                let results = rw.get_concretized_summary(
+                    &module_id,
+                    &fun_id,
+                    &signer_addresses,
+                    &script_args,
+                    type_args,
+                    state,
+                )?;
+                println!("{}", results.display(&fenv))
+            }
+            ConcretizeMode::Reads => {
+                let results = rw.get_keys_read(
+                    &module_id,
+                    &fun_id,
+                    &signer_addresses,
+                    &script_args,
+                    &type_args,
+                    state,
+                )?;
+                for key in results {
+                    println!("{}", key)
+                }
+            }
+            ConcretizeMode::Writes => {
+                let results = rw.get_keys_written(
+                    &module_id,
+                    &fun_id,
+                    &signer_addresses,
+                    &script_args,
+                    &type_args,
+                    state,
+                )?;
+                for key in results {
+                    println!("{}", key)
+                }
+            }
+            ConcretizeMode::Dont => {
+                // don't try try to concretize; just print the R/W set
+                // safe to unwrap here because every function must be analyzed
+                let results = rw.get_summary(&module_id, &fun_id).expect(
+                    "Invariant violation: couldn't resolve R/W set summary for defined function",
+                );
+                println!("{}", results.display(&fenv))
+            }
         }
     } else {
         println!("Function {} not found in {}", function, module_file)

language/tools/move-cli/src/lib.rs

@@ -31,7 +31,7 @@ use move_core_types::{
 use move_vm_runtime::native_functions::NativeFunction;
 use sandbox::utils::mode::{Mode, ModeType};
 use std::{fs, path::Path};
-use structopt::StructOpt;
+use structopt::{clap::arg_enum, StructOpt};
 
 type NativeFunctionRecord = (AccountAddress, Identifier, Identifier, NativeFunction);
 
@@ -223,11 +223,27 @@ pub enum ExperimentalCommand {
         args: Vec<TransactionArgument>,
         #[structopt(long = "type-args", parse(try_from_str = parser::parse_type_tag))]
         type_args: Vec<TypeTag>,
-        #[structopt(long = "concretize")]
-        concretize: bool,
+        #[structopt(long = "concretize", possible_values = &ConcretizeMode::variants(), case_insensitive = true, default_value = "dont")]
+        concretize: ConcretizeMode,
     },
 }
 
+// Specify if/how the analysis should concretize and filter the static analysis summary
+arg_enum! {
+    // Specify if/how the analysis should concretize and filter the static analysis summary
+    #[derive(Debug, Clone, Copy)]
+    pub enum ConcretizeMode {
+        // Show the full concretized access paths read or written (e.g. 0xA/0x1::M::S/f/g)
+        Paths,
+        // Show only the concrete resource keys that are read (e.g. 0xA/0x1::M::S)
+        Reads,
+        // Show only the concrete resource keys that are written (e.g. 0xA/0x1::M::S)
+        Writes,
+        // Do not concretize; show the results from the static analysis
+        Dont,
+    }
+}
+
 fn handle_experimental_commands(
     move_args: &Move,
     mode: &Mode,

language/tools/move-cli/tests/testsuite/concretize_read_write_set/args.exp

@@ -8,18 +8,18 @@ Formal(0)/0x1::ConcretizeSecondaryIndexes::Addr/a/0x1::ConcretizeSecondaryIndexe
 Locals:
 Ret(0): Formal(0)/0x1::ConcretizeSecondaryIndexes::Addr/a/0x1::ConcretizeSecondaryIndexes::S/f
 
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize --args 0xA`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize paths --args 0xA`:
 
 Command `sandbox run storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv publish_addr --signers 0xA --args 0xB`:
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize --args 0xA`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize paths --args 0xA`:
 0xa/0x1::ConcretizeSecondaryIndexes::Addr/a: Read
 
 Command `sandbox run storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv publish --signers 0xB`:
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize --args 0xA`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize paths --args 0xA`:
 0xa/0x1::ConcretizeSecondaryIndexes::Addr/a: Read
 0xb/0x1::ConcretizeSecondaryIndexes::S/f: Read
 
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv multi_arg --concretize --signers 0x1 --args 0xA 2`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv multi_arg --concretize paths --signers 0x1 --args 0xA 2`:
 0xa/0x1::ConcretizeSecondaryIndexes::Addr/a: Read
 0xb/0x1::ConcretizeSecondaryIndexes::S/f: Read
 
@@ -33,16 +33,16 @@ Formal(0)/0x1::ConcretizeVector::S/v/[_]/0x1::ConcretizeVector::T/f: Read
 Locals:
 Ret(0): Formal(0)/0x1::ConcretizeVector::S/v/[_]/0x1::ConcretizeVector::T/f
 
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize --args 0x1`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize paths --args 0x1`:
 
 Command `sandbox run storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv publish --signers 0x1 0x2`:
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize --args 0x1`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize paths --args 0x1`:
 0x1/0x1::ConcretizeVector::S/v: Read
 0x1/0x1::ConcretizeVector::S/v/[_]: Read
 0x1/0x1::ConcretizeVector::T/f: Read
 0x2/0x1::ConcretizeVector::T/f: Read
 
-Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv write_vec --concretize --args 0x1 2`:
+Command `experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv write_vec --concretize paths --args 0x1 2`:
 0x1/0x1::ConcretizeVector::S/v: Read
 0x1/0x1::ConcretizeVector::S/v/[_]: Read
 0x1/0x1::ConcretizeVector::T/f: Write

language/tools/move-cli/tests/testsuite/concretize_read_write_set/args.txt

@@ -4,24 +4,24 @@ sandbox publish
 # print the abstract read/write set state
 experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect
 # try to concretize, should print nothing. publish first resource needed
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize --args 0xA
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize paths --args 0xA
 sandbox run storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv publish_addr --signers 0xA --args 0xB
 # try to concretize, should print one read. publish second resource needed
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize --args 0xA
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize paths --args 0xA
 sandbox run storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv publish --signers 0xB
 # try to concretize, should now print both resources
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize --args 0xA
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv read_indirect --concretize paths --args 0xA
 
 # check that concretizing with both signers and address args works
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv multi_arg --concretize --signers 0x1 --args 0xA 2
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeSecondaryIndexes.mv multi_arg --concretize paths --signers 0x1 --args 0xA 2
 
 ## vector + secondary index test
 # print the abstract read/write set state
 experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec
 # try to concretize, should print nothing. publish resources needed
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize --args 0x1
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize paths --args 0x1
 sandbox run storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv publish --signers 0x1 0x2
 # try to concretize, should now print one S resource and two T resources
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize --args 0x1
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv read_vec --concretize paths --args 0x1
 # same thing, but with write function
-experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv write_vec --concretize --args 0x1 2
+experimental read-write-set storage/0x00000000000000000000000000000001/modules/ConcretizeVector.mv write_vec --concretize paths --args 0x1 2