diff --git a/Cargo.lock b/Cargo.lock index 1b7f0446f..d79bba1f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1806,6 +1806,7 @@ dependencies = [ "fvm_sdk", "fvm_shared", "hex", + "integer-encoding", "itertools 0.10.5", "lazy_static", "libsecp256k1", diff --git a/actors/init/src/state.rs b/actors/init/src/state.rs index 632a08797..c70dbee26 100644 --- a/actors/init/src/state.rs +++ b/actors/init/src/state.rs @@ -17,7 +17,7 @@ pub struct State { pub network_name: String, } -pub type AddressMap<'bs, BS> = Map2<'bs, BS, ActorID>; +pub type AddressMap = Map2; impl State { pub fn new(store: &BS, network_name: String) -> Result { @@ -41,13 +41,12 @@ impl State { let (id, existing) = if let Some(delegated_addr) = delegated_addr { // If there's a delegated address, either recall the already-mapped actor ID or // create and map a new one. - let key = delegated_addr.to_bytes(); - if let Some(existing_id) = map.get(&key)? { + if let Some(existing_id) = map.get(delegated_addr)? { (*existing_id, true) } else { let new_id = self.next_id; self.next_id += 1; - map.set(&key, new_id)?; + map.set(delegated_addr, new_id)?; (new_id, false) } } else { @@ -58,7 +57,7 @@ impl State { }; // Map the robust address to the ID, failing if it's already mapped to anything. - let is_new = map.set_if_absent(&robust_addr.to_bytes(), id)?; + let is_new = map.set_if_absent(robust_addr, id)?; if !is_new { return Err(actor_error!( forbidden, @@ -89,7 +88,7 @@ impl State { return Ok(Some(*addr)); } let map = AddressMap::load(store, &self.address_map, DEFAULT_CONF, "addresses")?; - let found = map.get(&addr.to_bytes())?; + let found = map.get(addr)?; Ok(found.copied().map(Address::new_id)) } } diff --git a/actors/init/src/testing.rs b/actors/init/src/testing.rs index 13ef43e65..10671b6d4 100644 --- a/actors/init/src/testing.rs +++ b/actors/init/src/testing.rs @@ -1,15 +1,13 @@ use std::collections::HashMap; -use fil_actors_runtime::{ - AsActorError, MessageAccumulator, DEFAULT_CONF, FIRST_NON_SINGLETON_ADDR, -}; use fvm_ipld_blockstore::Blockstore; -use fvm_shared::error::ExitCode; use fvm_shared::{ address::{Address, Protocol}, ActorID, }; +use fil_actors_runtime::{MessageAccumulator, DEFAULT_CONF, FIRST_NON_SINGLETON_ADDR}; + use crate::state::AddressMap; use crate::State; @@ -39,42 +37,33 @@ pub fn check_state_invariants( match AddressMap::load(store, &state.address_map, DEFAULT_CONF, "addresses") { Ok(address_map) => { let ret = address_map.for_each(|key, actor_id| { - let key_address = - Address::from_bytes(key).exit_code(ExitCode::USR_ILLEGAL_STATE)?; - - acc.require( - key_address.protocol() != Protocol::ID, - format!("key {key_address} is an ID address"), - ); + acc.require(key.protocol() != Protocol::ID, format!("key {key} is an ID address")); acc.require( actor_id >= &FIRST_NON_SINGLETON_ADDR, format!("unexpected singleton ID value {actor_id}"), ); - match key_address.protocol() { + match key.protocol() { Protocol::ID => { - acc.add(format!("key {key_address} is an ID address")); + acc.add(format!("key {key} is an ID address")); } Protocol::Delegated => { - if let Some(duplicate) = - delegated_address_by_id.insert(*actor_id, key_address) - { + if let Some(duplicate) = delegated_address_by_id.insert(*actor_id, key) { acc.add(format!( - "duplicate mapping to ID {actor_id}: {key_address} {duplicate}" + "duplicate mapping to ID {actor_id}: {key} {duplicate}" )); } } _ => { - if let Some(duplicate) = stable_address_by_id.insert(*actor_id, key_address) - { + if let Some(duplicate) = stable_address_by_id.insert(*actor_id, key) { acc.add(format!( - "duplicate mapping to ID {actor_id}: {key_address} {duplicate}" + "duplicate mapping to ID {actor_id}: {key} {duplicate}" )); } } } - init_summary.ids_by_address.insert(key_address, *actor_id); + init_summary.ids_by_address.insert(key, *actor_id); Ok(()) }); diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 673849cb7..b7426cca7 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -8,40 +8,41 @@ edition = "2021" repository = "https://github.com/filecoin-project/builtin-actors" [dependencies] -fvm_ipld_hamt = { workspace = true } -fvm_ipld_amt = { workspace = true } -fvm_shared = { workspace = true } -num = { workspace = true } -num-traits = { workspace = true } -num-derive = { workspace = true } -serde = { workspace = true } -lazy_static = { workspace = true, optional = true } -unsigned-varint = { workspace = true } +anyhow = { workspace = true } byteorder = { workspace = true } +castaway = { workspace = true } cid = { workspace = true } -log = { workspace = true } -thiserror = { workspace = true } -anyhow = { workspace = true } -fvm_sdk = { workspace = true, optional = true } +fvm_ipld_amt = { workspace = true } +fvm_ipld_bitfield = { workspace = true } fvm_ipld_blockstore = { workspace = true } fvm_ipld_encoding = { workspace = true } -fvm_ipld_bitfield = { workspace = true } -multihash = { workspace = true } -serde_repr = { workspace = true } -regex = { workspace = true } +fvm_ipld_hamt = { workspace = true } +fvm_sdk = { workspace = true, optional = true } +fvm_shared = { workspace = true } +integer-encoding = { workspace = true } itertools = { workspace = true } +lazy_static = { workspace = true, optional = true } +log = { workspace = true } +multihash = { workspace = true } +num = { workspace = true } +num-derive = { workspace = true } +num-traits = { workspace = true } paste = { workspace = true } -castaway = { workspace = true } +regex = { workspace = true } +serde = { workspace = true } +serde_repr = { workspace = true } +thiserror = { workspace = true } +unsigned-varint = { workspace = true } # A fake-proofs dependency but... we can't select on that feature here because we enable it from # build.rs. sha2 = { workspace = true } # test_util -rand = { workspace = true, optional = true } -hex = { workspace = true, optional = true } blake2b_simd = { workspace = true, optional = true } +hex = { workspace = true, optional = true } pretty_env_logger = { workspace = true, optional = true } +rand = { workspace = true, optional = true } [dependencies.libsecp256k1] workspace = true diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 959e637b6..f09899a41 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1,29 +1,29 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use builtin::HAMT_BIT_WIDTH; use cid::Cid; use fvm_ipld_amt::Amt; use fvm_ipld_blockstore::Blockstore; +#[cfg(not(feature = "fil-actor"))] +use fvm_ipld_hamt::Sha256; use fvm_ipld_hamt::{BytesKey, Error as HamtError, Hamt}; -use fvm_shared::address::Address; use fvm_shared::bigint::BigInt; pub use fvm_shared::BLOCKS_PER_EPOCH as EXPECTED_LEADERS_PER_EPOCH; use serde::de::DeserializeOwned; use serde::Serialize; use unsigned_varint::decode::Error as UVarintError; -pub use {fvm_ipld_amt, fvm_ipld_hamt}; -pub use self::actor_error::*; -pub use self::builtin::*; -pub use self::util::*; -use crate::runtime::Runtime; +use builtin::HAMT_BIT_WIDTH; +pub use dispatch::{dispatch, dispatch_default}; +pub use {fvm_ipld_amt, fvm_ipld_hamt}; #[cfg(feature = "fil-actor")] use crate::runtime::hash_algorithm::FvmHashSha256; +use crate::runtime::Runtime; -#[cfg(not(feature = "fil-actor"))] -use fvm_ipld_hamt::Sha256; +pub use self::actor_error::*; +pub use self::builtin::*; +pub use self::util::*; pub mod actor_error; pub mod builtin; @@ -31,7 +31,6 @@ pub mod runtime; pub mod util; mod dispatch; -pub use dispatch::{dispatch, dispatch_default}; #[cfg(feature = "test_utils")] pub mod test_utils; @@ -45,7 +44,6 @@ macro_rules! wasm_trampoline { }; } -/// XXX move to map #[cfg(feature = "fil-actor")] type Hasher = FvmHashSha256; @@ -113,12 +111,6 @@ pub trait Keyer { fn key(&self) -> BytesKey; } -impl Keyer for Address { - fn key(&self) -> BytesKey { - self.to_bytes().into() - } -} - impl Keyer for u64 { fn key(&self) -> BytesKey { u64_key(*self) diff --git a/runtime/src/util/map.rs b/runtime/src/util/map.rs index 6b89b4c63..079a099d7 100644 --- a/runtime/src/util/map.rs +++ b/runtime/src/util/map.rs @@ -4,24 +4,27 @@ use anyhow::anyhow; use cid::Cid; use fvm_ipld_blockstore::Blockstore; use fvm_ipld_hamt as hamt; +use fvm_shared::address::Address; use fvm_shared::error::ExitCode; use serde::de::DeserializeOwned; use serde::Serialize; +use std::marker::PhantomData; /// Wraps a HAMT to provide a convenient map API. -/// The key type is Vec, so conversion to/from interpretations must by done by the caller. /// Any errors are returned with exit code indicating illegal state. /// The name is not persisted in state, but adorns any error messages. -pub struct Map2<'bs, BS, V> +pub struct Map2 where BS: Blockstore, + K: MapKey, V: DeserializeOwned + Serialize, { - hamt: hamt::Hamt<&'bs BS, V, hamt::BytesKey, Hasher>, + hamt: hamt::Hamt, name: &'static str, + key_type: PhantomData, } -trait MapKey: Sized { +pub trait MapKey: Sized { fn from_bytes(b: &[u8]) -> Result; fn to_bytes(&self) -> Result, String>; } @@ -31,19 +34,24 @@ pub type Config = hamt::Config; pub const DEFAULT_CONF: Config = Config { bit_width: HAMT_BIT_WIDTH, min_data_depth: 0, max_array_width: 3 }; -impl<'bs, BS, V> Map2<'bs, BS, V> +impl Map2 where BS: Blockstore, + K: MapKey, V: DeserializeOwned + Serialize, { /// Creates a new, empty map. - pub fn empty(store: &'bs BS, config: Config, name: &'static str) -> Self { - Self { hamt: hamt::Hamt::new_with_config(store, config), name } + pub fn empty(store: BS, config: Config, name: &'static str) -> Self { + Self { + hamt: hamt::Hamt::new_with_config(store, config), + name, + key_type: Default::default(), + } } /// Creates a new empty map and flushes it to the store. /// Returns the CID of the empty map root. - pub fn flush_empty(store: &'bs BS, config: Config) -> Result { + pub fn flush_empty(store: BS, config: Config) -> Result { // This CID is constant regardless of the HAMT's configuration, so as an optimisation // we could hard-code it and merely check it is already stored. Self::empty(store, config, "empty").flush() @@ -54,7 +62,7 @@ where // The caller must know the configuration to interpret the HAMT correctly. // Forcing them to provide it makes it harder to accidentally use an incorrect default. pub fn load( - store: &'bs BS, + store: BS, root: &Cid, config: Config, name: &'static str, @@ -65,6 +73,7 @@ where format!("failed to load HAMT '{}'", name) })?, name, + key_type: Default::default(), }) } @@ -77,31 +86,34 @@ where } /// Returns a reference to the value associated with a key, if present. - pub fn get(&self, key: &[u8]) -> Result, ActorError> { - self.hamt.get(key).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + pub fn get(&self, key: &K) -> Result, ActorError> { + let k = key.to_bytes().context_code(ExitCode::USR_ASSERTION_FAILED, "invalid key")?; + self.hamt.get(&k).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { format!("failed to get from HAMT '{}'", self.name) }) } /// Inserts a key-value pair into the map. /// Returns any value previously associated with the key. - pub fn set(&mut self, key: &[u8], value: V) -> Result, ActorError> + pub fn set(&mut self, key: &K, value: V) -> Result, ActorError> where V: PartialEq, { - self.hamt.set(key.into(), value).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { + let k = key.to_bytes().context_code(ExitCode::USR_ASSERTION_FAILED, "invalid key")?; + self.hamt.set(k.into(), value).with_context_code(ExitCode::USR_ILLEGAL_STATE, || { format!("failed to set in HAMT '{}'", self.name) }) } /// Inserts a key-value pair only if the key does not already exist. /// Returns whether the map was modified (i.e. key was absent). - pub fn set_if_absent(&mut self, key: &[u8], value: V) -> Result + pub fn set_if_absent(&mut self, key: &K, value: V) -> Result where V: PartialEq, { + let k = key.to_bytes().context_code(ExitCode::USR_ASSERTION_FAILED, "invalid key")?; self.hamt - .set_if_absent(key.into(), value) + .set_if_absent(k.into(), value) .with_context_code(ExitCode::USR_ILLEGAL_STATE, || { format!("failed to set in HAMT '{}'", self.name) }) @@ -113,9 +125,12 @@ where // Note the result type of F uses ActorError. // The implementation will extract and propagate any ActorError // wrapped in a hamt::Error::Dynamic. - F: FnMut(&[u8], &V) -> Result<(), ActorError>, + F: FnMut(K, &V) -> Result<(), ActorError>, { - match self.hamt.for_each(|k, v| f(k, v).map_err(|e| anyhow!(e))) { + match self.hamt.for_each(|k, v| { + let key = K::from_bytes(k).context_code(ExitCode::USR_ILLEGAL_STATE, "invalid key")?; + f(key, v).map_err(|e| anyhow!(e)) + }) { Ok(_) => Ok(()), Err(hamt_err) => match hamt_err { hamt::Error::Dynamic(e) => match e.downcast::() { @@ -134,6 +149,32 @@ where } } +impl MapKey for u64 { + fn from_bytes(b: &[u8]) -> Result { + let (v, rem) = unsigned_varint::decode::u64(b).map_err(|e| e.to_string())?; + if !rem.is_empty() { + return Err(format!("trailing bytes after varint: {:?}", rem)); + } + Ok(v) + } + + fn to_bytes(&self) -> Result, String> { + let mut bz = unsigned_varint::encode::u64_buffer(); + let slice = unsigned_varint::encode::u64(*self, &mut bz); + Ok(slice.into()) + } +} + +impl MapKey for Address { + fn from_bytes(b: &[u8]) -> Result { + Address::from_bytes(b).map_err(|e| e.to_string()) + } + + fn to_bytes(&self) -> Result, String> { + Ok(Address::to_bytes(*self)) + } +} + #[cfg(test)] mod tests { use super::*; @@ -142,17 +183,17 @@ mod tests { #[test] fn basic_put_get() { let bs = MemoryBlockstore::new(); - let mut m = Map2::::empty(&bs, DEFAULT_CONF, "empty"); - m.set(&[1, 2, 3, 4], "1234".to_string()).unwrap(); - assert!(m.get(&[1, 2]).unwrap().is_none()); - assert_eq!(&"1234".to_string(), m.get(&[1, 2, 3, 4]).unwrap().unwrap()); + let mut m = Map2::<_, u64, String>::empty(bs, DEFAULT_CONF, "empty"); + m.set(&1234, "1234".to_string()).unwrap(); + assert!(m.get(&2222).unwrap().is_none()); + assert_eq!(&"1234".to_string(), m.get(&1234).unwrap().unwrap()); } #[test] fn for_each_callback_exitcode_propagates() { let bs = MemoryBlockstore::new(); - let mut m = Map2::::empty(&bs, DEFAULT_CONF, "empty"); - m.set(&[1, 2, 3, 4], "1234".to_string()).unwrap(); + let mut m = Map2::<_, u64, String>::empty(bs, DEFAULT_CONF, "empty"); + m.set(&1234, "1234".to_string()).unwrap(); let res = m.for_each(|_, _| Err(ActorError::forbidden("test".to_string()))); assert!(res.is_err()); assert_eq!(res.unwrap_err(), ActorError::forbidden("test".to_string()));