Skip to content

Commit

Permalink
replace concrete blockstore in TestVM with a generic trait (#1259)
Browse files Browse the repository at this point in the history
* replace concrete blockstore in TestVM with a generic trait

* fix test expectation
  • Loading branch information
alexytsu authored Mar 28, 2023
1 parent 1df034d commit 424669b
Show file tree
Hide file tree
Showing 26 changed files with 274 additions and 197 deletions.
8 changes: 5 additions & 3 deletions actors/market/src/testing.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{
collections::{BTreeMap, BTreeSet},
convert::TryFrom,
fmt::Debug,
};

use cid::Cid;
Expand Down Expand Up @@ -61,7 +60,7 @@ pub struct StateSummary {
}

/// Checks internal invariants of market state
pub fn check_state_invariants<BS: Blockstore + Debug>(
pub fn check_state_invariants<BS: Blockstore>(
state: &State,
store: &BS,
balance: &TokenAmount,
Expand Down Expand Up @@ -229,7 +228,10 @@ pub fn check_state_invariants<BS: Blockstore + Debug>(
let ret = pending_proposals.for_each(|key, _| {
let proposal_cid = Cid::try_from(key.0.to_owned())?;

acc.require(proposal_cids.contains(&proposal_cid), format!("pending proposal with cid {proposal_cid} not found within proposals {pending_proposals:?}"));
acc.require(
proposal_cids.contains(&proposal_cid),
format!("pending proposal with cid {proposal_cid} not found within proposals"),
);

pending_proposal_count += 1;
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion actors/market/tests/market_actor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,7 @@ fn fail_when_deal_is_activated_but_proposal_is_not_found() {
&rt,
&[
Regex::new("no deal proposal for deal state \\d+").unwrap(),
Regex::new("pending proposal with cid \\w+ not found within proposals .*").unwrap(),
Regex::new("pending proposal with cid \\w+ not found within proposals").unwrap(),
Regex::new("deal op found for deal id \\d+ with missing proposal at epoch \\d+")
.unwrap(),
],
Expand Down
2 changes: 1 addition & 1 deletion actors/miner/tests/sectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn make_sector(i: u64) -> SectorOnChainInfo {
}
}

fn setup_sectors(store: &'_ MemoryBlockstore) -> Sectors<'_, MemoryBlockstore> {
fn setup_sectors(store: &'_ MemoryBlockstore) -> Sectors<MemoryBlockstore> {
sectors_arr_mbs(store, vec![make_sector(0), make_sector(1), make_sector(5)])
}

Expand Down
4 changes: 2 additions & 2 deletions actors/miner/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2967,7 +2967,7 @@ pub fn test_sector(
pub fn sectors_arr_mbs(
store: &'_ MemoryBlockstore,
sectors_info: Vec<SectorOnChainInfo>,
) -> Sectors<'_, MemoryBlockstore> {
) -> Sectors<MemoryBlockstore> {
let empty_array =
Amt::<(), _>::new_with_bit_width(store, SECTORS_AMT_BITWIDTH).flush().unwrap();
let mut sectors = Sectors::load(store, &empty_array).unwrap();
Expand Down Expand Up @@ -3294,7 +3294,7 @@ pub fn select_sectors(sectors: &[SectorOnChainInfo], field: &BitField) -> Vec<Se
#[allow(dead_code)]
pub fn require_no_expiration_groups_before(
epoch: ChainEpoch,
queue: &mut ExpirationQueue<'_, MemoryBlockstore>,
queue: &mut ExpirationQueue<MemoryBlockstore>,
) {
queue.amt.flush().unwrap();

Expand Down
2 changes: 1 addition & 1 deletion state/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ macro_rules! get_state {
// to match the Manifest implementation in the FVM.
// It could be replaced with a custom mapping trait (while Rust doesn't support
// abstract collection traits).
pub fn check_state_invariants<'a, BS: Blockstore + Debug>(
pub fn check_state_invariants<'a, BS: Blockstore>(
manifest: &BiBTreeMap<Cid, Type>,
policy: &Policy,
tree: Tree<'a, BS>,
Expand Down
94 changes: 60 additions & 34 deletions test_vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use fil_actors_runtime::{
use fil_actors_runtime::{MessageAccumulator, DATACAP_TOKEN_ACTOR_ADDR};
use fil_builtin_actors_state::check::check_state_invariants;
use fil_builtin_actors_state::check::Tree;
use fvm_ipld_blockstore::Blockstore;
use fvm_ipld_blockstore::MemoryBlockstore;
use fvm_ipld_encoding::ipld_block::IpldBlock;
use fvm_ipld_encoding::tuple::*;
Expand Down Expand Up @@ -74,8 +75,11 @@ use std::ops::Add;

pub mod util;

pub struct TestVM<'bs> {
pub store: &'bs MemoryBlockstore,
pub struct TestVM<'bs, BS>
where
BS: Blockstore,
{
pub store: &'bs BS,
pub state_root: RefCell<Cid>,
total_fil: TokenAmount,
actors_dirty: RefCell<bool>,
Expand Down Expand Up @@ -121,8 +125,11 @@ pub const TEST_FAUCET_ADDR: Address = Address::new_id(FIRST_NON_SINGLETON_ADDR +
pub const FIRST_TEST_USER_ADDR: ActorID = FIRST_NON_SINGLETON_ADDR + 3;

// accounts for verifreg root signer and msig
impl<'bs> TestVM<'bs> {
pub fn new(store: &'bs MemoryBlockstore) -> TestVM<'bs> {
impl<'bs, BS> TestVM<'bs, BS>
where
BS: Blockstore,
{
pub fn new(store: &'bs MemoryBlockstore) -> TestVM<'bs, MemoryBlockstore> {
let mut actors = Hamt::<&'bs MemoryBlockstore, Actor, BytesKey, Sha256>::new(store);
TestVM {
store,
Expand All @@ -140,11 +147,12 @@ impl<'bs> TestVM<'bs> {
Self { total_fil, ..self }
}

pub fn new_with_singletons(store: &'bs MemoryBlockstore) -> TestVM<'bs> {
pub fn new_with_singletons(store: &'bs MemoryBlockstore) -> TestVM<'bs, MemoryBlockstore> {
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).with_total_fil(&reward_total + &faucet_total);
let v = TestVM::<'_, MemoryBlockstore>::new(store)
.with_total_fil(&reward_total + &faucet_total);

// system
let sys_st = SystemState::new(store).unwrap();
Expand Down Expand Up @@ -283,7 +291,7 @@ impl<'bs> TestVM<'bs> {
v
}

pub fn with_epoch(self, epoch: ChainEpoch) -> TestVM<'bs> {
pub fn with_epoch(self, epoch: ChainEpoch) -> TestVM<'bs, BS> {
self.checkpoint();
TestVM {
store: self.store,
Expand Down Expand Up @@ -352,11 +360,9 @@ impl<'bs> TestVM<'bs> {
return Some(act.clone());
}
// go to persisted map
let actors = Hamt::<&'bs MemoryBlockstore, Actor, BytesKey, Sha256>::load(
&self.state_root.borrow(),
self.store,
)
.unwrap();
let actors =
Hamt::<&'bs BS, Actor, BytesKey, Sha256>::load(&self.state_root.borrow(), self.store)
.unwrap();
let actor = actors.get(&addr.to_bytes()).unwrap().cloned();
actor.iter().for_each(|a| {
self.actors_cache.borrow_mut().insert(addr, a.clone());
Expand All @@ -372,11 +378,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, Actor, BytesKey, Sha256>::load(
&self.state_root.borrow(),
self.store,
)
.unwrap();
let mut actors =
Hamt::<&'bs BS, Actor, BytesKey, Sha256>::load(&self.state_root.borrow(), self.store)
.unwrap();
for (addr, act) in self.actors_cache.borrow().iter() {
actors.set(addr.to_bytes().into(), act.clone()).unwrap();
}
Expand All @@ -394,7 +398,7 @@ impl<'bs> TestVM<'bs> {

pub fn normalize_address(&self, addr: &Address) -> Option<Address> {
let st = self.get_state::<InitState>(INIT_ACTOR_ADDR).unwrap();
st.resolve_address::<MemoryBlockstore>(self.store, addr).unwrap()
st.resolve_address::<BS>(self.store, addr).unwrap()
}

pub fn get_state<T: DeserializeOwned>(&self, addr: Address) -> Option<T> {
Expand Down Expand Up @@ -497,11 +501,9 @@ impl<'bs> TestVM<'bs> {
/// Checks the state invariants and returns broken invariants.
pub fn check_state_invariants(&self) -> anyhow::Result<MessageAccumulator> {
self.checkpoint();
let actors = Hamt::<&'bs MemoryBlockstore, Actor, BytesKey, Sha256>::load(
&self.state_root.borrow(),
self.store,
)
.unwrap();
let actors =
Hamt::<&'bs BS, Actor, BytesKey, Sha256>::load(&self.state_root.borrow(), self.store)
.unwrap();

let mut manifest = BiBTreeMap::new();
actors
Expand Down Expand Up @@ -571,7 +573,10 @@ impl InternalMessage {
}
}

impl MessageInfo for InvocationCtx<'_, '_> {
impl<BS> MessageInfo for InvocationCtx<'_, '_, BS>
where
BS: Blockstore,
{
fn nonce(&self) -> u64 {
self.top.originator_call_seq
}
Expand All @@ -598,8 +603,11 @@ pub const TEST_VM_RAND_ARRAY: [u8; 32] = [
];
pub const TEST_VM_INVALID_POST: &str = "i_am_invalid_post";

pub struct InvocationCtx<'invocation, 'bs> {
v: &'invocation TestVM<'bs>,
pub struct InvocationCtx<'invocation, 'bs, BS>
where
BS: Blockstore,
{
v: &'invocation TestVM<'bs, BS>,
top: TopCtx,
msg: InternalMessage,
allow_side_effects: RefCell<bool>,
Expand All @@ -609,7 +617,10 @@ pub struct InvocationCtx<'invocation, 'bs> {
subinvocations: RefCell<Vec<InvocationTrace>>,
}

impl<'invocation, 'bs> InvocationCtx<'invocation, 'bs> {
impl<'invocation, 'bs, BS> InvocationCtx<'invocation, 'bs, BS>
where
BS: Blockstore,
{
fn resolve_target(&'invocation self, target: &Address) -> Result<(Actor, Address), ActorError> {
if let Some(a) = self.v.normalize_address(target) {
if let Some(act) = self.v.get_actor(a) {
Expand Down Expand Up @@ -779,8 +790,11 @@ impl<'invocation, 'bs> InvocationCtx<'invocation, 'bs> {
}
}

impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
type Blockstore = &'bs MemoryBlockstore;
impl<'invocation, 'bs, BS> Runtime for InvocationCtx<'invocation, 'bs, BS>
where
BS: Blockstore,
{
type Blockstore = &'bs BS;

fn create_actor(
&self,
Expand Down Expand Up @@ -822,7 +836,7 @@ impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
Ok(())
}

fn store(&self) -> &&'bs MemoryBlockstore {
fn store(&self) -> &&'bs BS {
&self.v.store
}

Expand Down Expand Up @@ -1123,7 +1137,10 @@ impl<'invocation, 'bs> Runtime for InvocationCtx<'invocation, 'bs> {
}
}

impl Primitives for TestVM<'_> {
impl<BS> Primitives for TestVM<'_, BS>
where
BS: Blockstore,
{
// A "valid" signature has its bytes equal to the plaintext.
// Anything else is considered invalid.
fn verify_signature(
Expand Down Expand Up @@ -1179,7 +1196,10 @@ impl Primitives for TestVM<'_> {
}
}

impl Primitives for InvocationCtx<'_, '_> {
impl<BS> Primitives for InvocationCtx<'_, '_, BS>
where
BS: Blockstore,
{
fn verify_signature(
&self,
signature: &Signature,
Expand Down Expand Up @@ -1218,7 +1238,10 @@ impl Primitives for InvocationCtx<'_, '_> {
}
}

impl Verifier for InvocationCtx<'_, '_> {
impl<BS> Verifier for InvocationCtx<'_, '_, BS>
where
BS: Blockstore,
{
fn verify_seal(&self, _vi: &SealVerifyInfo) -> Result<(), anyhow::Error> {
Ok(())
}
Expand Down Expand Up @@ -1258,7 +1281,10 @@ impl Verifier for InvocationCtx<'_, '_> {
}
}

impl RuntimePolicy for InvocationCtx<'_, '_> {
impl<BS> RuntimePolicy for InvocationCtx<'_, '_, BS>
where
BS: Blockstore,
{
fn policy(&self) -> &Policy {
self.policy
}
Expand Down
Loading

0 comments on commit 424669b

Please sign in to comment.