From ca68aa279f10b9a4c21d3d94a30956202960e5b1 Mon Sep 17 00:00:00 2001 From: Alex North <445306+anorth@users.noreply.github.com> Date: Tue, 27 Feb 2024 07:25:01 +1300 Subject: [PATCH] Replace Map with Map2 in miner actor (#1523) --- actors/miner/src/deadline_state.rs | 35 ++++++++++--- actors/miner/src/lib.rs | 24 ++------- actors/miner/src/state.rs | 80 ++++++++++++----------------- actors/miner/src/testing.rs | 26 +++------- actors/miner/tests/state_harness.rs | 3 +- test_vm/src/lib.rs | 12 ++--- 6 files changed, 79 insertions(+), 101 deletions(-) diff --git a/actors/miner/src/deadline_state.rs b/actors/miner/src/deadline_state.rs index 0a18d97ea..fb2752285 100644 --- a/actors/miner/src/deadline_state.rs +++ b/actors/miner/src/deadline_state.rs @@ -7,8 +7,6 @@ use std::collections::BTreeSet; use anyhow::anyhow; use cid::multihash::Code; use cid::Cid; -use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::{actor_error, ActorDowncast, ActorError, Array, AsActorError}; use fvm_ipld_bitfield::BitField; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::tuple::*; @@ -19,11 +17,15 @@ use fvm_shared::error::ExitCode; use fvm_shared::sector::{PoStProof, SectorSize}; use num_traits::{Signed, Zero}; +use fil_actors_runtime::runtime::Policy; +use fil_actors_runtime::{actor_error, ActorDowncast, ActorError, Array, AsActorError}; + +use crate::SECTORS_AMT_BITWIDTH; + use super::{ BitFieldQueue, ExpirationSet, Partition, PartitionSectorMap, PoStPartition, PowerPair, QuantSpec, SectorOnChainInfo, Sectors, TerminationResult, }; -use crate::SECTORS_AMT_BITWIDTH; // Bitwidth of AMTs determined empirically from mutation patterns and projections of mainnet data. // Usually a small array @@ -184,24 +186,41 @@ pub struct DisputeInfo { } impl Deadline { - pub fn new(store: &BS) -> anyhow::Result { + pub fn new(store: &BS) -> Result { let empty_partitions_array = Array::<(), BS>::new_with_bit_width(store, DEADLINE_PARTITIONS_AMT_BITWIDTH) .flush() - .map_err(|e| e.downcast_wrap("Failed to create empty states array"))?; + .map_err(|e| { + e.downcast_default( + ExitCode::USR_ILLEGAL_STATE, + "Failed to create empty states array", + ) + })?; let empty_deadline_expiration_array = Array::<(), BS>::new_with_bit_width(store, DEADLINE_EXPIRATIONS_AMT_BITWIDTH) .flush() - .map_err(|e| e.downcast_wrap("Failed to create empty states array"))?; + .map_err(|e| { + e.downcast_default( + ExitCode::USR_ILLEGAL_STATE, + "Failed to create empty states array", + ) + })?; let empty_post_submissions_array = Array::<(), BS>::new_with_bit_width( store, DEADLINE_OPTIMISTIC_POST_SUBMISSIONS_AMT_BITWIDTH, ) .flush() - .map_err(|e| e.downcast_wrap("Failed to create empty states array"))?; + .map_err(|e| { + e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "Failed to create empty states array") + })?; let empty_sectors_array = Array::<(), BS>::new_with_bit_width(store, SECTORS_AMT_BITWIDTH) .flush() - .map_err(|e| e.downcast_wrap("Failed to construct empty sectors snapshot array"))?; + .map_err(|e| { + e.downcast_default( + ExitCode::USR_ILLEGAL_STATE, + "Failed to construct empty sectors snapshot array", + ) + })?; Ok(Self { partitions: empty_partitions_array, expirations_epochs: empty_deadline_expiration_array, diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index b1a0fb461..80850a44d 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -54,8 +54,6 @@ use fil_actors_runtime::{ BURNT_FUNDS_ACTOR_ADDR, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR, }; - -use crate::ext::market::NO_ALLOCATION_ID; pub use monies::*; pub use partition_state::*; pub use policy::*; @@ -67,6 +65,7 @@ pub use termination::*; pub use types::*; pub use vesting_state::*; +use crate::ext::market::NO_ALLOCATION_ID; use crate::notifications::{notify_data_consumers, ActivationNotifications}; // The following errors are particular cases of illegal state. @@ -239,12 +238,8 @@ impl Actor { e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to construct illegal state") })?; - let st = - State::new(policy, rt.store(), info_cid, period_start, deadline_idx).map_err(|e| { - e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to construct state") - })?; + let st = State::new(policy, rt.store(), info_cid, period_start, deadline_idx)?; rt.create(&st)?; - Ok(()) } @@ -1555,7 +1550,7 @@ impl Actor { None => return Err(actor_error!( illegal_argument, "unspecified CompactCommD not allowed past nv21, need explicit None value for CC or CommD")), - _ => {}, + _ => {} } // Require sector lifetime meets minimum by assuming activation happens at last epoch permitted for seal proof. @@ -1977,12 +1972,7 @@ impl Actor { // Validate pre-commit. let precommit = st .get_precommitted_sector(store, params.sector_number) - .map_err(|e| { - e.downcast_default( - ExitCode::USR_ILLEGAL_STATE, - format!("failed to load pre-committed sector {}", params.sector_number), - ) - })? + .with_context(|| format!("loading pre-commit {}", params.sector_number))? .ok_or_else(|| { actor_error!(not_found, "no pre-commited sector {}", params.sector_number) })?; @@ -5316,11 +5306,7 @@ fn activate_new_sector_infos( state.put_sectors(store, new_sectors.clone()).map_err(|e| { e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to put new sectors") })?; - - state.delete_precommitted_sectors(store, &new_sector_numbers).map_err(|e| { - e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to delete precommited sectors") - })?; - + state.delete_precommitted_sectors(store, &new_sector_numbers)?; state .assign_sectors_to_deadlines( policy, diff --git a/actors/miner/src/state.rs b/actors/miner/src/state.rs index e4babb43f..9fe58a9b2 100644 --- a/actors/miner/src/state.rs +++ b/actors/miner/src/state.rs @@ -8,20 +8,12 @@ use std::ops::Neg; use anyhow::{anyhow, Error}; use cid::multihash::Code; use cid::Cid; -use fil_actors_runtime::runtime::policy_constants::MAX_SECTOR_NUMBER; -use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::{ - actor_error, make_empty_map, make_map_with_root_and_bitwidth, u64_key, ActorDowncast, - ActorError, Array, AsActorError, -}; use fvm_ipld_amt::Error as AmtError; use fvm_ipld_bitfield::BitField; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::tuple::*; use fvm_ipld_encoding::{strict_bytes, BytesDe, CborStore}; -use fvm_ipld_hamt::Error as HamtError; use fvm_shared::address::Address; - use fvm_shared::clock::{ChainEpoch, EPOCH_UNDEFINED}; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; @@ -30,6 +22,13 @@ use fvm_shared::{ActorID, HAMT_BIT_WIDTH}; use itertools::Itertools; use num_traits::Zero; +use fil_actors_runtime::runtime::policy_constants::MAX_SECTOR_NUMBER; +use fil_actors_runtime::runtime::Policy; +use fil_actors_runtime::{ + actor_error, ActorContext, ActorDowncast, ActorError, Array, AsActorError, Config, Map2, + DEFAULT_HAMT_CONFIG, +}; + use super::beneficiary::*; use super::deadlines::new_deadline_info; use super::policy::*; @@ -40,6 +39,9 @@ use super::{ PowerPair, QuantSpec, Sectors, TerminationResult, VestingFunds, }; +pub type PreCommitMap = Map2; +pub const PRECOMMIT_CONFIG: Config = Config { bit_width: HAMT_BIT_WIDTH, ..DEFAULT_HAMT_CONFIG }; + const PRECOMMIT_EXPIRY_AMT_BITWIDTH: u32 = 6; pub const SECTORS_AMT_BITWIDTH: u32 = 5; @@ -126,14 +128,10 @@ impl State { info_cid: Cid, period_start: ChainEpoch, deadline_idx: u64, - ) -> anyhow::Result { + ) -> Result { let empty_precommit_map = - make_empty_map::<_, ()>(store, HAMT_BIT_WIDTH).flush().map_err(|e| { - e.downcast_default( - ExitCode::USR_ILLEGAL_STATE, - "failed to construct empty precommit map", - ) - })?; + PreCommitMap::empty(store, PRECOMMIT_CONFIG, "precommits").flush()?; + let empty_precommits_cleanup_array = Array::::new_with_bit_width(store, PRECOMMIT_EXPIRY_AMT_BITWIDTH) .flush() @@ -292,14 +290,12 @@ impl State { precommits: Vec, ) -> anyhow::Result<()> { let mut precommitted = - make_map_with_root_and_bitwidth(&self.pre_committed_sectors, store, HAMT_BIT_WIDTH)?; + PreCommitMap::load(store, &self.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits")?; for precommit in precommits.into_iter() { let sector_no = precommit.info.sector_number; let modified = precommitted - .set_if_absent(u64_key(precommit.info.sector_number), precommit) - .map_err(|e| { - e.downcast_wrap(format!("failed to store precommitment for {:?}", sector_no,)) - })?; + .set_if_absent(§or_no, precommit) + .with_context(|| format!("storing precommit for {}", sector_no))?; if !modified { return Err(anyhow!("sector {} already pre-commited", sector_no)); } @@ -313,10 +309,10 @@ impl State { &self, store: &BS, sector_num: SectorNumber, - ) -> Result, HamtError> { + ) -> Result, ActorError> { let precommitted = - make_map_with_root_and_bitwidth(&self.pre_committed_sectors, store, HAMT_BIT_WIDTH)?; - Ok(precommitted.get(&u64_key(sector_num))?.cloned()) + PreCommitMap::load(store, &self.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits")?; + Ok(precommitted.get(§or_num)?.cloned()) } /// Gets and returns the requested pre-committed sectors, skipping missing sectors. @@ -325,17 +321,15 @@ impl State { store: &BS, sector_numbers: &[SectorNumber], ) -> anyhow::Result> { - let precommitted = make_map_with_root_and_bitwidth::<_, SectorPreCommitOnChainInfo>( - &self.pre_committed_sectors, - store, - HAMT_BIT_WIDTH, - )?; + let precommitted = + PreCommitMap::load(store, &self.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits")?; let mut result = Vec::with_capacity(sector_numbers.len()); for §or_number in sector_numbers { - let info = match precommitted.get(&u64_key(sector_number)).map_err(|e| { - e.downcast_wrap(format!("failed to load precommitment for {}", sector_number)) - })? { + let info = match precommitted + .get(§or_number) + .with_context(|| format!("loading precommit {}", sector_number))? + { Some(info) => info.clone(), None => continue, }; @@ -350,17 +344,13 @@ impl State { &mut self, store: &BS, sector_nums: &[SectorNumber], - ) -> Result<(), HamtError> { - let mut precommitted = make_map_with_root_and_bitwidth::<_, SectorPreCommitOnChainInfo>( - &self.pre_committed_sectors, - store, - HAMT_BIT_WIDTH, - )?; - + ) -> Result<(), ActorError> { + let mut precommitted = + PreCommitMap::load(store, &self.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits")?; for §or_num in sector_nums { - let prev_entry = precommitted.delete(&u64_key(sector_num))?; + let prev_entry = precommitted.delete(§or_num)?; if prev_entry.is_none() { - return Err(format!("sector {} doesn't exist", sector_num).into()); + return Err(actor_error!(illegal_state, "sector {} not pre-committed", sector_num)); } } @@ -1052,13 +1042,12 @@ impl State { let mut precommits_to_delete = Vec::new(); let precommitted = - make_map_with_root_and_bitwidth(&self.pre_committed_sectors, store, HAMT_BIT_WIDTH)?; + PreCommitMap::load(store, &self.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits")?; for i in sectors.iter() { let sector_number = i as SectorNumber; - let sector: SectorPreCommitOnChainInfo = - match precommitted.get(&u64_key(sector_number))?.cloned() { + match precommitted.get(§or_number)?.cloned() { Some(sector) => sector, // already committed/deleted None => continue, @@ -1179,15 +1168,14 @@ impl State { ) -> Result, ActorError> { let mut precommits = Vec::new(); let precommitted = - make_map_with_root_and_bitwidth(&self.pre_committed_sectors, store, HAMT_BIT_WIDTH) - .context_code(ExitCode::USR_ILLEGAL_STATE, "failed to load precommitted sectors")?; + PreCommitMap::load(store, &self.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits")?; for sector_no in sector_nos.into_iter() { let sector_no = *sector_no.borrow(); if sector_no > MAX_SECTOR_NUMBER { return Err(actor_error!(illegal_argument; "sector number greater than maximum")); } let info: &SectorPreCommitOnChainInfo = precommitted - .get(&u64_key(sector_no)) + .get(§or_no) .exit_code(ExitCode::USR_ILLEGAL_STATE)? .ok_or_else(|| actor_error!(not_found, "sector {} not found", sector_no))?; precommits.push(info.clone()); diff --git a/actors/miner/src/testing.rs b/actors/miner/src/testing.rs index 591109352..64beb603a 100644 --- a/actors/miner/src/testing.rs +++ b/actors/miner/src/testing.rs @@ -1,12 +1,10 @@ use crate::{ power_for_sectors, BitFieldQueue, Deadline, ExpirationQueue, MinerInfo, Partition, PowerPair, - QuantSpec, SectorOnChainInfo, SectorOnChainInfoFlags, SectorPreCommitOnChainInfo, Sectors, - State, NO_QUANTIZATION, + PreCommitMap, QuantSpec, SectorOnChainInfo, SectorOnChainInfoFlags, Sectors, State, + NO_QUANTIZATION, PRECOMMIT_CONFIG, }; use fil_actors_runtime::runtime::Policy; -use fil_actors_runtime::{ - parse_uint_key, DealWeight, Map, MessageAccumulator, DEFAULT_HAMT_CONFIG, -}; +use fil_actors_runtime::{DealWeight, MessageAccumulator}; use fvm_ipld_bitfield::BitField; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_encoding::CborStore; @@ -349,23 +347,11 @@ fn check_precommits( let mut precommit_total = TokenAmount::zero(); - let precommited_sectors = Map::<_, SectorPreCommitOnChainInfo>::load_with_config( - &state.pre_committed_sectors, - store, - DEFAULT_HAMT_CONFIG, - ); - + let precommited_sectors = + PreCommitMap::load(store, &state.pre_committed_sectors, PRECOMMIT_CONFIG, "precommits"); match precommited_sectors { Ok(precommited_sectors) => { - let ret = precommited_sectors.for_each(|key, precommit| { - let sector_number = match parse_uint_key(key) { - Ok(sector_number) => sector_number, - Err(e) => { - acc.add(format!("error parsing pre-commit key as uint: {e}")); - return Ok(()); - } - }; - + let ret = precommited_sectors.for_each(|sector_number, precommit| { acc.require( allocated_sectors.contains(§or_number), format!("pre-commited sector number has not been allocated {sector_number}"), diff --git a/actors/miner/tests/state_harness.rs b/actors/miner/tests/state_harness.rs index bd79616d9..0ea6490f0 100644 --- a/actors/miner/tests/state_harness.rs +++ b/actors/miner/tests/state_harness.rs @@ -9,7 +9,6 @@ use fil_actors_runtime::{runtime::Policy, ActorError}; use fvm_ipld_bitfield::BitField; use fvm_ipld_encoding::BytesDe; use fvm_ipld_encoding::CborStore; -use fvm_ipld_hamt::Error as HamtError; use fvm_shared::econ::TokenAmount; use fvm_shared::sector::{SectorNumber, SectorSize}; use fvm_shared::{clock::ChainEpoch, sector::RegisteredPoStProof}; @@ -68,7 +67,7 @@ impl StateHarness { pub fn delete_precommitted_sectors( &mut self, sector_nums: &[SectorNumber], - ) -> Result<(), HamtError> { + ) -> Result<(), ActorError> { self.st.delete_precommitted_sectors(&self.store, sector_nums) } diff --git a/test_vm/src/lib.rs b/test_vm/src/lib.rs index 70e5fcda2..9e13ac5d5 100644 --- a/test_vm/src/lib.rs +++ b/test_vm/src/lib.rs @@ -13,8 +13,8 @@ use fil_actors_runtime::cbor::serialize; use fil_actors_runtime::runtime::builtins::Type; use fil_actors_runtime::runtime::{Policy, Primitives, EMPTY_ARR_CID}; use fil_actors_runtime::test_blockstores::MemoryBlockstore; -use fil_actors_runtime::{test_utils::*, DEFAULT_HAMT_CONFIG}; -use fil_actors_runtime::{Map, DATACAP_TOKEN_ACTOR_ADDR}; +use fil_actors_runtime::DATACAP_TOKEN_ACTOR_ADDR; +use fil_actors_runtime::{test_utils::*, Map2, DEFAULT_HAMT_CONFIG}; use fil_actors_runtime::{ BURNT_FUNDS_ACTOR_ADDR, CRON_ACTOR_ADDR, EAM_ACTOR_ADDR, INIT_ACTOR_ADDR, REWARD_ACTOR_ADDR, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR, @@ -266,8 +266,8 @@ impl TestVM { self.actors_dirty.replace(false); } - fn actor_map(&self) -> Map { - Map::load_with_config(&self.checkpoint(), self.store.as_ref(), DEFAULT_HAMT_CONFIG).unwrap() + fn actor_map(&self) -> Map2<&MemoryBlockstore, Address, ActorState> { + Map2::load(self.store.as_ref(), &self.checkpoint(), DEFAULT_HAMT_CONFIG, "actors").unwrap() } } @@ -380,7 +380,7 @@ impl VM for TestVM { } // go to persisted map let actors = self.actor_map(); - let actor = actors.get(&address.to_bytes()).unwrap().cloned(); + let actor = actors.get(address).unwrap().cloned(); actor.iter().for_each(|a| { self.actors_cache.borrow_mut().insert(*address, a.clone()); }); @@ -404,7 +404,7 @@ impl VM for TestVM { let map = self.actor_map(); let mut tree = BTreeMap::new(); map.for_each(|k, v| { - tree.insert(Address::from_bytes(k).unwrap(), v.clone()); + tree.insert(k, v.clone()); Ok(()) }) .unwrap();