Replace Map with Map2 in miner actor (#1523)

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<BS: Blockstore>(store: &BS) -> anyhow::Result<Self> {
+    pub fn new<BS: Blockstore>(store: &BS) -> Result<Self, ActorError> {
         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,

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,

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<BS> = Map2<BS, SectorNumber, SectorPreCommitOnChainInfo>;
+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<Self> {
+    ) -> Result<Self, ActorError> {
         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::<BitField, BS>::new_with_bit_width(store, PRECOMMIT_EXPIRY_AMT_BITWIDTH)
                 .flush()
@@ -292,14 +290,12 @@ impl State {
         precommits: Vec<SectorPreCommitOnChainInfo>,
     ) -> 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(&sector_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<Option<SectorPreCommitOnChainInfo>, HamtError> {
+    ) -> Result<Option<SectorPreCommitOnChainInfo>, 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(&sector_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<Vec<SectorPreCommitOnChainInfo>> {
-        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 &sector_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(&sector_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 &sector_num in sector_nums {
-            let prev_entry = precommitted.delete(&u64_key(sector_num))?;
+            let prev_entry = precommitted.delete(&sector_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(&sector_number)?.cloned() {
                     Some(sector) => sector,
                     // already committed/deleted
                     None => continue,
@@ -1179,15 +1168,14 @@ impl State {
     ) -> Result<Vec<SectorPreCommitOnChainInfo>, 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(&sector_no)
                 .exit_code(ExitCode::USR_ILLEGAL_STATE)?
                 .ok_or_else(|| actor_error!(not_found, "sector {} not found", sector_no))?;
             precommits.push(info.clone());

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<BS: Blockstore>(
 
     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(&sector_number),
                     format!("pre-commited sector number has not been allocated {sector_number}"),

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)
     }