Test VM should use Rc<Blockstore> for consistency with the MockRuntime (#1454)

* test vm should use Rc<Blockstore>

* use builder for RC<blockstore>

* review changes

Author: aarshkshah1992

test_vm/src/lib.rs

@@ -38,6 +38,7 @@ use fvm_shared::{MethodNum, METHOD_SEND};
 use serde::ser;
 use std::cell::{RefCell, RefMut};
 use std::collections::{BTreeMap, HashMap};
+use std::rc::Rc;
 use vm_api::trace::InvocationTrace;
 use vm_api::{new_actor, ActorState, MessageResult, VMError, VM};
 
@@ -50,9 +51,9 @@ mod messaging;
 pub use messaging::*;
 
 /// An in-memory rust-execution VM for testing builtin-actors that yields sensible stack traces and debug info
-pub struct TestVM<'bs> {
+pub struct TestVM {
     pub primitives: FakePrimitives,
-    pub store: &'bs MemoryBlockstore,
+    pub store: Rc<MemoryBlockstore>,
     pub state_root: RefCell<Cid>,
     circulating_supply: RefCell<TokenAmount>,
     actors_dirty: RefCell<bool>,
@@ -62,13 +63,15 @@ pub struct TestVM<'bs> {
     invocations: RefCell<Vec<InvocationTrace>>,
 }
 
-impl<'bs> TestVM<'bs> {
-    pub fn new(store: &'bs MemoryBlockstore) -> TestVM<'bs> {
+impl TestVM {
+    pub fn new(store: impl Into<Rc<MemoryBlockstore>>) -> TestVM {
+        let store = store.into();
         let mut actors =
-            Hamt::<&'bs MemoryBlockstore, ActorState, BytesKey, Sha256>::new_with_config(
-                store,
+            Hamt::<Rc<MemoryBlockstore>, ActorState, BytesKey, Sha256>::new_with_config(
+                Rc::clone(&store),
                 DEFAULT_HAMT_CONFIG,
             );
+
         TestVM {
             primitives: FakePrimitives {},
             store,
@@ -82,15 +85,17 @@ impl<'bs> TestVM<'bs> {
         }
     }
 
-    pub fn new_with_singletons(store: &'bs MemoryBlockstore) -> TestVM<'bs> {
+    pub fn new_with_singletons(store: impl Into<Rc<MemoryBlockstore>>) -> TestVM {
         let reward_total = TokenAmount::from_whole(1_100_000_000i64);
         let faucet_total = TokenAmount::from_whole(1_000_000_000i64);
 
-        let v = TestVM::<'_>::new(store);
+        let store = store.into();
+
+        let v = TestVM::new(Rc::clone(&store));
         v.set_circulating_supply(&reward_total + &faucet_total);
 
         // system
-        let sys_st = SystemState::new(store).unwrap();
+        let sys_st = SystemState::new(&store).unwrap();
         let sys_head = v.put_store(&sys_st);
         let sys_value = faucet_total.clone(); // delegate faucet funds to system so we can construct faucet by sending to bls addr
         v.set_actor(
@@ -99,7 +104,7 @@ impl<'bs> TestVM<'bs> {
         );
 
         // init
-        let init_st = InitState::new(store, "integration-test".to_string()).unwrap();
+        let init_st = InitState::new(&store, "integration-test".to_string()).unwrap();
         let init_head = v.put_store(&init_st);
         v.set_actor(
             &INIT_ACTOR_ADDR,
@@ -239,9 +244,9 @@ impl<'bs> TestVM<'bs> {
     pub fn checkpoint(&self) -> Cid {
         // persist cache on top of latest checkpoint and clear
         let mut actors =
-            Hamt::<&'bs MemoryBlockstore, ActorState, BytesKey, Sha256>::load_with_config(
+            Hamt::<Rc<MemoryBlockstore>, ActorState, BytesKey, Sha256>::load_with_config(
                 &self.state_root.borrow(),
-                self.store,
+                Rc::clone(&self.store),
                 DEFAULT_HAMT_CONFIG,
             )
             .unwrap();
@@ -275,9 +280,9 @@ impl<'bs> TestVM<'bs> {
     }
 }
 
-impl<'bs> VM for TestVM<'bs> {
+impl VM for TestVM {
     fn blockstore(&self) -> &dyn Blockstore {
-        self.store
+        self.store.as_ref()
     }
 
     fn epoch(&self) -> ChainEpoch {
@@ -364,7 +369,7 @@ impl<'bs> VM for TestVM<'bs> {
     }
     fn resolve_id_address(&self, address: &Address) -> Option<Address> {
         let st: InitState = get_state(self, &INIT_ACTOR_ADDR).unwrap();
-        st.resolve_address(self.store, address).unwrap()
+        st.resolve_address(&self.store, address).unwrap()
     }
 
     fn set_epoch(&self, epoch: ChainEpoch) {
@@ -386,9 +391,9 @@ impl<'bs> VM for TestVM<'bs> {
             return Some(act.clone());
         }
         // go to persisted map
-        let actors = Hamt::<&'bs MemoryBlockstore, ActorState, BytesKey, Sha256>::load_with_config(
+        let actors = Hamt::<Rc<MemoryBlockstore>, ActorState, BytesKey, Sha256>::load_with_config(
             &self.state_root.borrow(),
-            self.store,
+            Rc::clone(&self.store),
             DEFAULT_HAMT_CONFIG,
         )
         .unwrap();

test_vm/src/messaging.rs

@@ -62,6 +62,7 @@ use vm_api::{new_actor, ActorState, VM};
 
 use fil_actors_runtime::test_blockstores::MemoryBlockstore;
 use std::ops::Add;
+use std::rc::Rc;
 
 use crate::{TestVM, TEST_VM_INVALID_POST, TEST_VM_RAND_ARRAY};
 
@@ -82,7 +83,7 @@ pub struct InternalMessage {
     pub params: Option<IpldBlock>,
 }
 
-impl MessageInfo for InvocationCtx<'_, '_> {
+impl MessageInfo for InvocationCtx<'_> {
     fn nonce(&self) -> u64 {
         self.top.originator_call_seq
     }
@@ -103,8 +104,8 @@ impl MessageInfo for InvocationCtx<'_, '_> {
     }
 }
 
-pub struct InvocationCtx<'invocation, 'bs> {
-    pub v: &'invocation TestVM<'bs>,
+pub struct InvocationCtx<'invocation> {
+    pub v: &'invocation TestVM,
     pub top: TopCtx,
     pub msg: InternalMessage,
     pub allow_side_effects: RefCell<bool>,
@@ -114,7 +115,7 @@ pub struct InvocationCtx<'invocation, 'bs> {
     pub subinvocations: RefCell<Vec<InvocationTrace>>,
 }
 
-impl<'invocation, 'bs> InvocationCtx<'invocation, 'bs> {
+impl<'invocation> InvocationCtx<'invocation> {
     fn resolve_target(
         &'invocation self,
         target: &Address,
@@ -152,7 +153,7 @@ impl<'invocation, 'bs> InvocationCtx<'invocation, 'bs> {
         }
 
         let mut st: InitState = get_state(self.v, &INIT_ACTOR_ADDR).unwrap();
-        let (target_id, existing) = st.map_addresses_to_id(self.v.store, target, None).unwrap();
+        let (target_id, existing) = st.map_addresses_to_id(&self.v.store, target, None).unwrap();
         assert!(!existing, "should never have existing actor when no f4 address is specified");
         let target_id_addr = Address::new_id(target_id);
         let mut init_actor = self.v.actor(&INIT_ACTOR_ADDR).unwrap();
@@ -298,8 +299,8 @@ impl<'invocation, 'bs> InvocationCtx<'invocation, 'bs> {
     }
 }
 
-impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
-    type Blockstore = &'bs MemoryBlockstore;
+impl<'invocation> Runtime for InvocationCtx<'invocation> {
+    type Blockstore = Rc<MemoryBlockstore>;
 
     fn create_actor(
         &self,
@@ -341,7 +342,7 @@ impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
         Ok(())
     }
 
-    fn store(&self) -> &&'bs MemoryBlockstore {
+    fn store(&self) -> &Rc<MemoryBlockstore> {
         &self.v.store
     }
 
@@ -645,7 +646,7 @@ impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
     }
 }
 
-impl Primitives for InvocationCtx<'_, '_> {
+impl Primitives for InvocationCtx<'_> {
     fn verify_signature(
         &self,
         signature: &Signature,
@@ -684,7 +685,7 @@ impl Primitives for InvocationCtx<'_, '_> {
     }
 }
 
-impl Verifier for InvocationCtx<'_, '_> {
+impl Verifier for InvocationCtx<'_> {
     fn verify_post(&self, verify_info: &WindowPoStVerifyInfo) -> Result<(), anyhow::Error> {
         for proof in &verify_info.proofs {
             if proof.proof_bytes.eq(&TEST_VM_INVALID_POST.as_bytes().to_vec()) {
@@ -720,7 +721,7 @@ impl Verifier for InvocationCtx<'_, '_> {
     }
 }
 
-impl RuntimePolicy for InvocationCtx<'_, '_> {
+impl RuntimePolicy for InvocationCtx<'_> {
     fn policy(&self) -> &Policy {
         self.policy
     }

test_vm/tests/suite/authenticate_message_test.rs

@@ -5,6 +5,6 @@ use test_vm::TestVM;
 #[test]
 fn account_authenticate_message() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     account_authenticate_message_test(&v);
 }

test_vm/tests/suite/batch_onboarding.rs

@@ -8,7 +8,7 @@ use test_vm::TestVM;
 #[test_case(true; "v2")]
 fn batch_onboarding(v2: bool) {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
 
     batch_onboarding_test(&v, v2);
 }

test_vm/tests/suite/batch_onboarding_deals_test.rs

@@ -5,6 +5,6 @@ use test_vm::TestVM;
 #[test]
 fn batch_onboarding_deals() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     batch_onboarding_deals_test(&v);
 }

test_vm/tests/suite/change_beneficiary_test.rs

@@ -8,20 +8,20 @@ use test_vm::TestVM;
 #[test]
 fn change_beneficiary_success() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     change_beneficiary_success_test(&v);
 }
 
 #[test]
 fn change_beneficiary_back_owner_success() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     change_beneficiary_back_owner_success_test(&v);
 }
 
 #[test]
 fn change_beneficiary_fail() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     change_beneficiary_fail_test(&v);
 }

test_vm/tests/suite/change_owner_test.rs

@@ -7,20 +7,20 @@ use test_vm::TestVM;
 #[test]
 fn change_owner_success() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     change_owner_success_test(&v);
 }
 
 #[test]
 fn keep_beneficiary_when_owner_changed() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     keep_beneficiary_when_owner_changed_test(&v);
 }
 
 #[test]
 fn change_owner_fail() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     change_owner_fail_test(&v);
 }

test_vm/tests/suite/commit_post_test.rs

@@ -6,59 +6,58 @@ use fil_actors_integration_tests::tests::{
 };
 use fil_actors_runtime::test_blockstores::MemoryBlockstore;
 use test_vm::TestVM;
-
 #[test]
 fn submit_post_succeeds() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     submit_post_succeeds_test(&v);
 }
 
 #[test]
 fn skip_sector() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     skip_sector_test(&v);
 }
 
 #[test]
 fn missed_first_post_deadline() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     missed_first_post_deadline_test(&v);
 }
 
 #[test]
 fn overdue_precommit() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     overdue_precommit_test(&v);
 }
 
 #[test]
 fn aggregate_bad_sector_number() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     aggregate_bad_sector_number_test(&v);
 }
 
 #[test]
 fn aggregate_size_limits() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     aggregate_size_limits_test(&v);
 }
 
 #[test]
 fn aggregate_bad_sender() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     aggregate_bad_sender_test(&v);
 }
 
 #[test]
 fn aggregate_one_precommit_expires() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     aggregate_one_precommit_expires_test(&v);
 }

test_vm/tests/suite/datacap_tests.rs

@@ -6,14 +6,14 @@ use test_vm::TestVM;
 #[test]
 fn datacap_transfer() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     datacap_transfer_test(&v);
 }
 
 /* Call name & symbol */
 #[test]
 fn call_name_symbol() {
     let store = MemoryBlockstore::new();
-    let v = TestVM::new_with_singletons(&store);
+    let v = TestVM::new_with_singletons(store);
     call_name_symbol_test(&v);
 }