[framework] unify Rust and Move object ID derivation

Previously, there were two incompatible implementations of ObjectID derivation. This PR:

- Eliminates the `ObjectID` derivation inside Move and replaces it with a native function that invokes the Rust `ObjectID` derivation. This ensures that Rust and Move agree on how ID's are generated, and should also be more efficient than the previous approach.
- Introduces a harness for invoking Move unit tests and a unit tests for `TxContext` that shows the ID generation working as expected.

fastpay_core/src/unit_tests/authority_tests.rs

@@ -404,9 +404,9 @@ async fn test_handle_move_order() {
         MAX_GAS,
         &sender_key,
     );
-    // 143 is for bytecode execution, 24 is for object creation.
+    // 27 is for bytecode execution, 24 is for object creation.
     // If the number changes, we want to verify that the change is intended.
-    let gas_cost = 143 + 24;
+    let gas_cost = 27 + 24;
     let res = send_and_confirm_order(&mut authority_state, order)
         .await
         .unwrap();

fastx_programmability/adapter/src/unit_tests/adapter_tests.rs

@@ -319,7 +319,7 @@ fn test_move_call_insufficient_gas() {
         &native_functions,
         "create",
         gas_object,
-        50, // This budget is not enough to execute all bytecode.
+        25, // This budget is not enough to execute all bytecode.
         Vec::new(),
         pure_args,
     );

fastx_programmability/framework/Cargo.toml

@@ -16,8 +16,10 @@ fastx-verifier = { path = "../verifier" }
 
 move-binary-format = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 move-bytecode-verifier = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
+move-cli = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 move-core-types = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 move-package = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 move-stdlib = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
+move-unit-test = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 move-vm-runtime = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 move-vm-types = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }

fastx_programmability/framework/sources/Address.move

@@ -53,4 +53,23 @@ module FastX::Address {
     public fun is_signer(a: &Address, s: &Signer): bool {
         get(s) == a
     }
+
+    // ==== test-only functions ====
+
+
+    #[test_only]
+    /// Create a `Signer` from `bytes` for testing
+    public fun new_signer(bytes: vector<u8>): Signer {
+        assert!(
+            Vector::length(&bytes) == ADDRESS_LENGTH,
+            Errors::invalid_argument(EBAD_ADDRESS_LENGTH)
+        );
+        Signer { inner: new(bytes) }
+    }
+
+    #[test_only]
+    /// Create a dummy `Signer` for testing
+    public fun dummy_signer(): Signer {
+        new_signer(x"ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad")
+    }
 }

fastx_programmability/framework/sources/ID.move

@@ -16,13 +16,13 @@ module FastX::ID {
     }
 
     /// Create a new ID. Only callable by TxContext
-    // TODO (): bring this back
+    // TODO (): bring this back once we can support `friend`
     //public(friend) fun new(bytes: vector<u8>): ID {
-    public fun new(bytes: vector<u8>): ID {
-        ID { id: IDBytes { bytes: bytes_to_address(bytes) } }
+    public fun new(bytes: address): ID {
+        ID { id: IDBytes { bytes } }
     }
 
-    /// Create a new ID bytes for comparison with existing ID's
+    /// Create a new ID bytes for comparison with existing ID's.
     public fun new_bytes(bytes: vector<u8>): IDBytes {
         IDBytes { bytes: bytes_to_address(bytes) }
     }

fastx_programmability/framework/sources/TxContext.move

@@ -1,9 +1,6 @@
 module FastX::TxContext {
     use FastX::ID::{Self, ID};
     use FastX::Address::{Self, Address, Signer};
-    use Std::BCS;
-    use Std::Hash;
-    use Std::Vector;
 
     /// Information about the transaction currently being executed.
     /// This is a privileged object created by the VM and passed into `main`
@@ -13,20 +10,9 @@ module FastX::TxContext {
         signer: Signer,
         /// Hash of all the input objects to this transaction
         inputs_hash: vector<u8>,
-        /// Counter recording the number of objects created while executing
+        /// Counter recording the number of fresh id's created while executing
         /// this transaction
-        objects_created: u64
-    }
-
-    /// Generate a new primary key
-    // TODO: can make this native for better perf
-    public fun new_id(ctx: &mut TxContext): ID {
-        let msg = *&ctx.inputs_hash;
-        let next_object_num = ctx.objects_created;
-        ctx.objects_created = next_object_num + 1;
-
-        Vector::append(&mut msg, BCS::to_bytes(&next_object_num));
-        ID::new(Hash::sha3_256(msg))
+        ids_created: u64
     }
 
     /// Return the signer of the current transaction
@@ -39,4 +25,34 @@ module FastX::TxContext {
     public fun get_signer_address(self: &TxContext): Address {
         *Address::get(&self.signer)
     }
+
+    /// Return the number of id's created by the current transaction
+    public fun get_ids_created(self: &TxContext): u64 {
+        self.ids_created
+    }
+
+    /// Generate a new object ID
+    public fun new_id(ctx: &mut TxContext): ID {
+        let ids_created = ctx.ids_created;
+        let id = ID::new(fresh_id(*&ctx.inputs_hash, ids_created));
+        ctx.ids_created = ids_created + 1;
+        id
+    }
+
+    native fun fresh_id(inputs_hash: vector<u8>, ids_created: u64): address;
+
+    // ==== test-only functions ====
+
+    #[test_only]
+    /// Create a `TxContext` for testing
+    public fun new(signer: Signer, inputs_hash: vector<u8>, ids_created: u64): TxContext {
+        TxContext { signer, inputs_hash, ids_created }
+    }
+
+    #[test_only]
+    /// Create a dummy `TxContext` for testing
+    public fun dummy(): TxContext {
+        let inputs_hash = x"3a985da74fe225b2045c172d6bd390bd855f086e3e9d525b46bfe24511431532";
+        new(Address::dummy_signer(), inputs_hash, 0)
+    }
 }

fastx_programmability/framework/src/lib.rs

@@ -7,10 +7,15 @@ use fastx_verifier::verifier as fastx_bytecode_verifier;
 use move_binary_format::CompiledModule;
 use move_core_types::{ident_str, language_storage::ModuleId};
 use move_package::{compilation::compiled_package::CompiledPackage, BuildConfig};
+
 use std::path::PathBuf;
 
 pub mod natives;
 
+// Move unit tests will halt after executing this many steps. This is a protection to avoid divergence
+#[cfg(test)]
+const MAX_UNIT_TEST_INSTRUCTIONS: u64 = 100_000;
+
 /// Return all the modules of the fastX framework and its dependencies in topologically
 /// sorted dependency order (leaves first)
 pub fn get_framework_modules() -> Result<Vec<CompiledModule>> {
@@ -55,8 +60,33 @@ fn build(include_examples: bool) -> Result<CompiledPackage> {
 }
 
 #[test]
-fn check_that_move_code_can_be_built_and_verified() {
+fn check_that_move_code_can_be_built_verified_testsd() {
     let include_examples = true;
     let verify = true;
     get_framework_modules_(include_examples, verify).unwrap();
+    // ideally this would be a separate test, but doing so introduces
+    // races because of https://github.com/diem/diem/issues/10102
+    run_move_unit_tests();
+}
+
+#[cfg(test)]
+fn run_move_unit_tests() {
+    use fastx_types::FASTX_FRAMEWORK_ADDRESS;
+    use move_cli::package::cli;
+    use move_unit_test::UnitTestingConfig;
+    use std::path::Path;
+
+    let framework_dir = Path::new(env!("CARGO_MANIFEST_DIR"));
+    let include_examples = true;
+    cli::run_move_unit_tests(
+        framework_dir,
+        BuildConfig {
+            dev_mode: include_examples,
+            ..Default::default()
+        },
+        UnitTestingConfig::default_with_bound(Some(MAX_UNIT_TEST_INSTRUCTIONS)),
+        natives::all_natives(MOVE_STDLIB_ADDRESS, FASTX_FRAMEWORK_ADDRESS),
+        /* compute_coverage */ false,
+    )
+    .unwrap();
 }

fastx_programmability/framework/src/natives/mod.rs

@@ -3,6 +3,7 @@
 
 mod id;
 mod transfer;
+mod tx_context;
 
 use move_core_types::{account_address::AccountAddress, identifier::Identifier};
 use move_vm_runtime::native_functions::{NativeFunction, NativeFunctionTable};
@@ -14,6 +15,7 @@ pub fn all_natives(
     const FASTX_NATIVES: &[(&str, &str, NativeFunction)] = &[
         ("ID", "bytes_to_address", id::bytes_to_address),
         ("Transfer", "transfer_internal", transfer::transfer_internal),
+        ("TxContext", "fresh_id", tx_context::fresh_id),
     ];
     FASTX_NATIVES
         .iter()

fastx_programmability/framework/src/natives/tx_context.rs

@@ -0,0 +1,37 @@
+// Copyright (c) Mysten Labs
+// SPDX-License-Identifier: Apache-2.0
+
+use fastx_types::base_types::TransactionDigest;
+use move_binary_format::errors::PartialVMResult;
+use move_vm_runtime::native_functions::NativeContext;
+use move_vm_types::{
+    gas_schedule::NativeCostIndex,
+    loaded_data::runtime_types::Type,
+    natives::function::{native_gas, NativeResult},
+    pop_arg,
+    values::Value,
+};
+use smallvec::smallvec;
+use std::{collections::VecDeque, convert::TryFrom};
+
+pub fn fresh_id(
+    context: &mut NativeContext,
+    ty_args: Vec<Type>,
+    mut args: VecDeque<Value>,
+) -> PartialVMResult<NativeResult> {
+    debug_assert!(ty_args.is_empty());
+    debug_assert!(args.len() == 2);
+
+    let ids_created = pop_arg!(args, u64);
+    let inputs_hash = pop_arg!(args, Vec<u8>);
+
+    // TODO(https://github.com/MystenLabs/fastnft/issues/58): finalize digest format
+    // unwrap safe because all digests in Move are serialized from the Rust `TransactionDigest`
+    let digest = TransactionDigest::try_from(inputs_hash.as_slice()).unwrap();
+    let id = Value::address(digest.derive_id(ids_created));
+
+    // TODO: choose cost
+    let cost = native_gas(context.cost_table(), NativeCostIndex::CREATE_SIGNER, 0);
+
+    Ok(NativeResult::ok(cost, smallvec![id]))
+}

fastx_programmability/framework/tests/TxContextTests.move

@@ -0,0 +1,18 @@
+#[test_only]
+module FastX::TxContextTests {
+    use FastX::TxContext;
+
+    #[test]
+    fun test_id_generation() {
+        let ctx = TxContext::dummy();
+        assert!(TxContext::get_ids_created(&ctx) == 0, 0);
+
+        let id1 = TxContext::new_id(&mut ctx);
+        let id2 = TxContext::new_id(&mut ctx);
+
+        // new_id should always produce fresh ID's
+        assert!(id1 != id2, 1);
+        assert!(TxContext::get_ids_created(&ctx) == 2, 2);
+    }
+
+}

fastx_types/src/base_types.rs

@@ -74,10 +74,12 @@ pub type AuthorityName = PublicKeyBytes;
 pub type ObjectID = AccountAddress;
 pub type ObjectRef = (ObjectID, SequenceNumber, ObjectDigest);
 
-// A transaction will have a (unique) digest.
-#[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Debug, Serialize, Deserialize)]
-pub struct TransactionDigest([u8; 32]); // We use SHA3-256 hence 32 bytes here
+// We use SHA3-256 hence 32 bytes here
+const TRANSACTION_DIGEST_LENGTH: usize = 32;
 
+/// A transaction will have a (unique) digest.
+#[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Debug, Serialize, Deserialize)]
+pub struct TransactionDigest([u8; TRANSACTION_DIGEST_LENGTH]);
 // Each object has a unique digest
 #[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone, Hash, Debug, Serialize, Deserialize)]
 pub struct ObjectDigest([u8; 32]); // We use SHA3-256 hence 32 bytes here
@@ -108,16 +110,8 @@ impl TxContext {
 
     /// Derive a globally unique object ID by hashing self.digest | self.ids_created
     pub fn fresh_id(&mut self) -> ObjectID {
-        // TODO(https://github.com/MystenLabs/fastnft/issues/58):
-        // audit ID derivation: do we want/need domain separation, different hash function, truncation ...
-
-        let mut hasher = Sha3_256::default();
-        hasher.update(self.digest.0);
-        hasher.update(self.ids_created.to_le_bytes());
-        let hash = hasher.finalize();
+        let id = self.digest.derive_id(self.ids_created);
 
-        // truncate into an ObjectID.
-        let id = AccountAddress::try_from(&hash[0..AccountAddress::LENGTH]).unwrap();
         self.ids_created += 1;
         id
     }
@@ -163,6 +157,20 @@ impl TransactionDigest {
         Self::new([0; 32])
     }
 
+    /// Create an ObjectID from `self` and `creation_num`.
+    /// Caller is responsible for ensuring that `creation_num` is fresh
+    pub fn derive_id(&self, creation_num: u64) -> ObjectID {
+        // TODO(https://github.com/MystenLabs/fastnft/issues/58):audit ID derivation
+
+        let mut hasher = Sha3_256::default();
+        hasher.update(self.0);
+        hasher.update(creation_num.to_le_bytes());
+        let hash = hasher.finalize();
+
+        // truncate into an ObjectID.
+        AccountAddress::try_from(&hash[0..AccountAddress::LENGTH]).unwrap()
+    }
+
     // for testing
     pub fn random() -> Self {
         use rand::Rng;
@@ -399,3 +407,14 @@ pub fn sha3_hash<S: Signable<Sha3_256>>(signable: &S) -> [u8; 32] {
     let hash = digest.finalize();
     hash.into()
 }
+
+impl TryFrom<&[u8]> for TransactionDigest {
+    type Error = FastPayError;
+
+    fn try_from(bytes: &[u8]) -> Result<Self, FastPayError> {
+        let arr: [u8; TRANSACTION_DIGEST_LENGTH] = bytes
+            .try_into()
+            .map_err(|_| FastPayError::InvalidTransactionDigest)?;
+        Ok(Self(arr))
+    }
+}

fastx_types/src/error.rs

@@ -84,6 +84,8 @@ pub enum FastPayError {
     InvalidCrossShardUpdate,
     #[error("Invalid authenticator")]
     InvalidAuthenticator,
+    #[error("Invalid transaction digest.")]
+    InvalidTransactionDigest,
     #[error("Cannot deserialize.")]
     InvalidDecoding,
     #[error("Unexpected message.")]