From d874f3c0d185b61ab7f331dbfe436afdc4b5f175 Mon Sep 17 00:00:00 2001 From: Sander Pick Date: Sun, 16 Jun 2024 08:08:20 -0700 Subject: [PATCH 1/3] os: refactor state (include metadata, remove internal objects) Signed-off-by: Sander Pick --- Cargo.lock | 3 + fendermint/actors/objectstore/Cargo.toml | 3 + fendermint/actors/objectstore/src/actor.rs | 24 +- fendermint/actors/objectstore/src/shared.rs | 21 +- fendermint/actors/objectstore/src/state.rs | 329 ++++++++------------ fendermint/app/src/cmd/objects.rs | 4 +- fendermint/vm/interpreter/src/chain.rs | 6 +- 7 files changed, 164 insertions(+), 226 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a4de097b..e196b45d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2927,6 +2927,7 @@ dependencies = [ "anyhow", "cid", "fendermint_actor_machine", + "fendermint_testing", "fil_actors_runtime", "frc42_dispatch", "fvm_ipld_blockstore", @@ -2935,6 +2936,8 @@ dependencies = [ "fvm_shared", "num-derive 0.3.3", "num-traits", + "quickcheck", + "quickcheck_macros", "serde", "serde_tuple", ] diff --git a/fendermint/actors/objectstore/Cargo.toml b/fendermint/actors/objectstore/Cargo.toml index 8bfcabc2..7818fa63 100644 --- a/fendermint/actors/objectstore/Cargo.toml +++ b/fendermint/actors/objectstore/Cargo.toml @@ -29,10 +29,13 @@ fvm_ipld_hamt = { workspace = true } fendermint_actor_machine = { path = "../machine" } [dev-dependencies] +fendermint_testing = { path = "../../testing", features = ["arb"] } fil_actors_runtime = { workspace = true, features = [ "test_utils", "fil-actor", ] } +quickcheck = { workspace = true } +quickcheck_macros = { workspace = true } [features] default = [] diff --git a/fendermint/actors/objectstore/src/actor.rs b/fendermint/actors/objectstore/src/actor.rs index 7a8623d0..83239018 100644 --- a/fendermint/actors/objectstore/src/actor.rs +++ b/fendermint/actors/objectstore/src/actor.rs @@ -14,8 +14,8 @@ use fvm_ipld_hamt::BytesKey; use fvm_shared::{error::ExitCode, MethodNum}; use crate::{ - DeleteParams, GetParams, ListParams, Method, Object, ObjectList, PutParams, - ResolveExternalParams, State, OBJECTSTORE_ACTOR_NAME, + AddParams, DeleteParams, GetParams, ListParams, Method, Object, ObjectList, ResolveParams, + State, OBJECTSTORE_ACTOR_NAME, }; #[cfg(feature = "fil-actor")] @@ -36,29 +36,27 @@ impl Actor { rt.create(&state) } - fn put_object(rt: &impl Runtime, params: PutParams) -> Result { + fn add_object(rt: &impl Runtime, params: AddParams) -> Result { Self::ensure_write_allowed(rt)?; let root = rt.transaction(|st: &mut State, rt| { - st.put( + st.add( rt.store(), BytesKey(params.key), - params.kind, + params.cid, + params.metadata, params.overwrite, ) - .map_err(|e| e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to put object")) + .map_err(|e| e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to add object")) })?; Ok(root) } - fn resolve_external_object( - rt: &impl Runtime, - params: ResolveExternalParams, - ) -> Result<(), ActorError> { + fn resolve_object(rt: &impl Runtime, params: ResolveParams) -> Result<(), ActorError> { rt.validate_immediate_caller_is(std::iter::once(&SYSTEM_ACTOR_ADDR))?; rt.transaction(|st: &mut State, rt| { - st.resolve_external(rt.store(), BytesKey(params.key), params.value) + st.resolve(rt.store(), BytesKey(params.key), params.value) .map_err(|e| { e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to resolve object") }) @@ -135,8 +133,8 @@ impl ActorCode for Actor { actor_dispatch! { Constructor => constructor, GetMetadata => get_metadata, - PutObject => put_object, - ResolveExternalObject => resolve_external_object, + AddObject => add_object, + ResolveObject => resolve_object, DeleteObject => delete_object, GetObject => get_object, ListObjects => list_objects, diff --git a/fendermint/actors/objectstore/src/shared.rs b/fendermint/actors/objectstore/src/shared.rs index 8092e273..eac86334 100644 --- a/fendermint/actors/objectstore/src/shared.rs +++ b/fendermint/actors/objectstore/src/shared.rs @@ -7,30 +7,33 @@ use fendermint_actor_machine::GET_METADATA_METHOD; use fvm_ipld_encoding::{strict_bytes, tuple::*}; use fvm_shared::METHOD_CONSTRUCTOR; use num_derive::FromPrimitive; +use std::collections::HashMap; -pub use crate::state::{Object, ObjectKind, ObjectList, ObjectListItem, State}; +pub use crate::state::{Object, ObjectList, State}; pub const OBJECTSTORE_ACTOR_NAME: &str = "objectstore"; /// Params for putting an object. #[derive(Clone, Debug, Serialize_tuple, Deserialize_tuple)] -pub struct PutParams { +pub struct AddParams { /// Object key. #[serde(with = "strict_bytes")] pub key: Vec, - /// Kind of object. - pub kind: ObjectKind, + /// Object value. + pub cid: Cid, + /// Object metadata. + pub metadata: HashMap, /// Whether to overwrite a key if it already exists. pub overwrite: bool, } -/// Params for resolving an external object. +/// Params for resolving an object. #[derive(Clone, Debug, Serialize_tuple, Deserialize_tuple)] -pub struct ResolveExternalParams { +pub struct ResolveParams { /// Object key. #[serde(with = "strict_bytes")] pub key: Vec, - /// External object value. + /// Object value. pub value: Cid, } @@ -70,8 +73,8 @@ pub struct ListParams { pub enum Method { Constructor = METHOD_CONSTRUCTOR, GetMetadata = GET_METADATA_METHOD, - PutObject = frc42_dispatch::method_hash!("PutObject"), - ResolveExternalObject = frc42_dispatch::method_hash!("ResolveExternalObject"), + AddObject = frc42_dispatch::method_hash!("AddObject"), + ResolveObject = frc42_dispatch::method_hash!("ResolveObject"), DeleteObject = frc42_dispatch::method_hash!("DeleteObject"), GetObject = frc42_dispatch::method_hash!("GetObject"), ListObjects = frc42_dispatch::method_hash!("ListObjects"), diff --git a/fendermint/actors/objectstore/src/state.rs b/fendermint/actors/objectstore/src/state.rs index 89e95830..b72e8f1e 100644 --- a/fendermint/actors/objectstore/src/state.rs +++ b/fendermint/actors/objectstore/src/state.rs @@ -2,16 +2,16 @@ // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT -use cid::multihash::{Code, MultihashDigest}; use cid::Cid; use fendermint_actor_machine::{Kind, MachineState, WriteAccess}; use fvm_ipld_blockstore::Blockstore; -use fvm_ipld_encoding::{strict_bytes::ByteBuf, tuple::*, DAG_CBOR}; +use fvm_ipld_encoding::{strict_bytes::ByteBuf, tuple::*}; use fvm_ipld_hamt::{BytesKey, Hamt}; use fvm_shared::address::Address; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; -pub const BIT_WIDTH: u32 = 8; +const BIT_WIDTH: u32 = 8; const MAX_LIST_LIMIT: usize = 10000; @@ -42,43 +42,24 @@ impl MachineState for State { /// The stored representation of an object in the object store. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -pub enum Object { - /// Internal objects are stored on-chain. - Internal(ByteBuf), - /// External objects reference an off-chain object by Cid. - /// The bool indicates whether the object has been resolved. - External((ByteBuf, bool)), -} - -/// The kind of object. This is used during object creation. -#[derive(Clone, Debug, Serialize, Deserialize)] -pub enum ObjectKind { - /// Internal objects are stored on-chain. - Internal(ByteBuf), - /// External objects reference an off-chain object by Cid. - External(Cid), +pub struct Object { + /// The object content identifier. + pub cid: ByteBuf, + /// Whether the object has been resolved. + pub resolved: bool, + /// User-defined object metadata (e.g., size, last modified timestamp, etc.). + pub metadata: HashMap, } /// A list of objects and their common prefixes. #[derive(Default, Debug, Serialize_tuple, Deserialize_tuple)] pub struct ObjectList { /// List of key-values matching the list query. - pub objects: Vec<(Vec, ObjectListItem)>, + pub objects: Vec<(Vec, Object)>, /// When a delimiter is used in the list query, this contains common key prefixes. pub common_prefixes: Vec>, } -/// The kind of object. This is used during object creation. -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -pub enum ObjectListItem { - /// Internal objects are stored on-chain. - /// The u64 is the size of the object. - Internal((Cid, u64)), - /// External objects reference an off-chain object by Cid. - /// The bool indicates whether the object has been resolved. - External((Cid, bool)), -} - impl State { pub fn new( store: &BS, @@ -101,17 +82,19 @@ impl State { }) } - pub fn put( + pub fn add( &mut self, store: &BS, key: BytesKey, - kind: ObjectKind, + cid: Cid, + metadata: HashMap, overwrite: bool, ) -> anyhow::Result { let mut hamt = Hamt::<_, Object>::load_with_bit_width(&self.root, store, BIT_WIDTH)?; - let object = match kind { - ObjectKind::Internal(buf) => Object::Internal(buf), - ObjectKind::External(cid) => Object::External((ByteBuf(cid.to_bytes()), false)), + let object = Object { + cid: ByteBuf(cid.to_bytes()), + resolved: false, + metadata, }; if overwrite { hamt.set(key, object)?; @@ -122,7 +105,7 @@ impl State { Ok(self.root) } - pub fn resolve_external( + pub fn resolve( &mut self, store: &BS, key: BytesKey, @@ -130,18 +113,14 @@ impl State { ) -> anyhow::Result<()> { let mut hamt = Hamt::<_, Object>::load_with_bit_width(&self.root, store, BIT_WIDTH)?; match hamt.get(&key).map(|v| v.cloned())? { - Some(object) => { - match object { - Object::Internal(_) => Ok(()), - Object::External((v, _)) => { - // Ignore if value changed before it was resolved. - if v.0 == value.to_bytes() { - hamt.set(key, Object::External((v, true)))?; - self.root = hamt.flush()?; - } - Ok(()) - } + Some(mut object) => { + // Ignore if value changed before it was resolved. + if object.cid.0 == value.to_bytes() { + object.resolved = true; + hamt.set(key, object)?; + self.root = hamt.flush()?; } + Ok(()) } // Don't error here in case the key was deleted before the value was resolved. None => Ok(()), @@ -210,18 +189,7 @@ impl State { if count <= offset { continue; } - let item = match v { - Object::Internal(b) => { - let mh_code = Code::Blake2b256; - let mh = mh_code.digest(&b.0); - let cid = Cid::new_v1(DAG_CBOR, mh); - ObjectListItem::Internal((cid, b.0.len() as u64)) - } - Object::External((b, resolved)) => { - ObjectListItem::External((Cid::try_from(b.0.as_slice())?, *resolved)) - } - }; - objects.push((key, item)); + objects.push((key, v.to_owned())); if limit > 0 && objects.len() >= limit { break; } @@ -237,11 +205,51 @@ impl State { #[cfg(test)] mod tests { - use fvm_ipld_blockstore::MemoryBlockstore; - use super::*; + use cid::multihash::{Code, MultihashDigest}; + use fendermint_testing::arb::ArbCid; + use fil_actors_runtime::MapKey; + use fvm_ipld_blockstore::MemoryBlockstore; + use fvm_ipld_encoding::DAG_CBOR; + use quickcheck::Arbitrary; + use quickcheck_macros::quickcheck; use std::str::FromStr; + impl Arbitrary for Object { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + Object { + cid: ByteBuf(ArbCid::<64>::arbitrary(g).0.to_bytes()), + metadata: HashMap::arbitrary(g), + resolved: false, + } + } + } + + fn default_object() -> Object { + Object { + cid: ByteBuf(Cid::default().to_bytes()), + metadata: HashMap::::new(), + resolved: false, + } + } + + fn golden_object() -> Object { + let mh_code = Code::Blake2b256; + let mh = mh_code.digest(&[1, 2, 3, 4, 5]); + let cid = Cid::new_v1(DAG_CBOR, mh); + let mut metadata = HashMap::::new(); + metadata.insert("_size".to_string(), String::from("5")); + metadata.insert("_created".to_string(), String::from("1718464344")); + metadata.insert("_modified".to_string(), String::from("1718464345")); + Object { + cid: ByteBuf(cid.to_bytes()), + metadata, + resolved: false, + } + } + + const GOLDEN_CID: &str = "bafy2bzacebi2bfkcucl3r2oih4du4hd7jrlvm7cqgggofnkzzz3m4asycsbmq"; + #[test] fn test_constructor() { let store = MemoryBlockstore::default(); @@ -255,100 +263,50 @@ mod tests { } #[test] - fn test_put_internal() { + fn test_add() { let store = MemoryBlockstore::default(); let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); + let object = golden_object(); assert!(state - .put( + .add( &store, BytesKey(vec![1, 2, 3]), - ObjectKind::Internal(ByteBuf(vec![4, 5, 6])), + Cid::from_bytes(&object.cid.0).unwrap(), + object.metadata, true ) .is_ok()); - assert_eq!( - state.root, - Cid::from_str("bafy2bzacecyfqn52y34p5fu2yxne263thvwc2pitaec36ul3l7ghocmwjli5k") - .unwrap() - ); - } - - #[test] - fn test_put_external() { - let store = MemoryBlockstore::default(); - let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); - assert!(state - .put( - &store, - BytesKey(vec![1, 2, 3]), - ObjectKind::External(Cid::default()), - true - ) - .is_ok()); - - assert_eq!( - state.root, - Cid::from_str("bafy2bzaceaq7b4t24dmwbnkztaib3jlv7bcajecsqobshg6juoitave2rxws2") - .unwrap() - ); - } - - #[test] - fn test_resolve_external() { - let store = MemoryBlockstore::default(); - let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); - let key = BytesKey(vec![1, 2, 3]); - state - .put( - &store, - key.clone(), - ObjectKind::External(Cid::default()), - true, - ) - .unwrap(); - assert!(state - .resolve_external(&store, key.clone(), Cid::default()) - .is_ok()); - - let result = state.get(&store, &key); - assert!(result.is_ok()); - assert_eq!( - result.unwrap(), - Some(Object::External((ByteBuf(Cid::default().to_bytes()), true))) - ); + assert_eq!(state.root, Cid::from_str(GOLDEN_CID).unwrap()); } - #[test] - fn test_delete_internal() { + #[quickcheck] + fn test_resolve(mut object: Object) { let store = MemoryBlockstore::default(); let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); let key = BytesKey(vec![1, 2, 3]); - state - .put( - &store, - key.clone(), - ObjectKind::Internal(ByteBuf(vec![4, 5, 6])), - true, - ) - .unwrap(); - assert!(state.delete(&store, &key).is_ok()); + let cid = Cid::from_bytes(&object.cid.0).unwrap(); + let md = object.metadata.clone(); + state.add(&store, key.clone(), cid, md, true).unwrap(); + assert!(state.resolve(&store, key.clone(), cid).is_ok()); + object.resolved = true; let result = state.get(&store, &key); assert!(result.is_ok()); - assert_eq!(result.unwrap(), None); + assert_eq!(result.unwrap().unwrap(), object); } - #[test] - fn test_delete_external() { + #[quickcheck] + fn test_delete(object: Object) { let store = MemoryBlockstore::default(); let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); let key = BytesKey(vec![1, 2, 3]); state - .put( + .add( &store, key.clone(), - ObjectKind::External(Cid::default()), + Cid::from_bytes(&object.cid.0).unwrap(), + object.metadata, true, ) .unwrap(); @@ -359,51 +317,18 @@ mod tests { assert_eq!(result.unwrap(), None); } - #[test] - fn test_get_internal() { + #[quickcheck] + fn test_get(object: Object) { let store = MemoryBlockstore::default(); let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); let key = BytesKey(vec![1, 2, 3]); - state - .put( - &store, - key.clone(), - ObjectKind::Internal(ByteBuf(vec![4, 5, 6])), - true, - ) - .unwrap(); - let result = state.get(&store, &key); - - assert!(result.is_ok()); - assert_eq!( - result.unwrap(), - Some(Object::Internal(ByteBuf(vec![4, 5, 6]))), - ); - } - - #[test] - fn test_get_external() { - let store = MemoryBlockstore::default(); - let mut state = State::new(&store, Address::new_id(100), WriteAccess::OnlyOwner).unwrap(); - let key = BytesKey(vec![1, 2, 3]); - state - .put( - &store, - key.clone(), - ObjectKind::External(Cid::default()), - true, - ) - .unwrap(); + let cid = Cid::from_bytes(&object.cid.0).unwrap(); + let md = object.metadata.clone(); + state.add(&store, key.clone(), cid, md, true).unwrap(); let result = state.get(&store, &key); assert!(result.is_ok()); - assert_eq!( - result.unwrap(), - Some(Object::External(( - ByteBuf(Cid::default().to_bytes()), - false - ))) - ); + assert_eq!(result.unwrap().unwrap(), object); } fn create_and_put_objects( @@ -411,33 +336,37 @@ mod tests { store: &MemoryBlockstore, ) -> anyhow::Result<(BytesKey, BytesKey, BytesKey)> { let jpeg_key = BytesKey("foo.jpeg".as_bytes().to_vec()); - state.put( + state.add( store, jpeg_key.clone(), - ObjectKind::Internal(ByteBuf(vec![4, 5, 6])), + Cid::default(), + HashMap::::new(), false, )?; let bar_key = BytesKey("foo/bar.png".as_bytes().to_vec()); - state.put( + state.add( store, bar_key.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, )?; let baz_key = BytesKey("foo/baz.png".as_bytes().to_vec()); - state.put( + state.add( store, baz_key.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, )?; // We'll mostly ignore this one let other_key = BytesKey("zzzz/image.png".as_bytes().to_vec()); - state.put( + state.add( &store, other_key.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, )?; Ok((jpeg_key, bar_key, baz_key)) @@ -450,14 +379,14 @@ mod tests { let (_, _, baz_key) = create_and_put_objects(&mut state, &store).unwrap(); - let default_item = ObjectListItem::External((Cid::default(), false)); + let default_obj = default_object(); // List all keys with a limit let result = state.list(&store, vec![], vec![], 0, 0); assert!(result.is_ok()); let result = result.unwrap(); assert_eq!(result.objects.len(), 4); - assert_eq!(result.objects.first(), Some(&(baz_key.0, default_item))); + assert_eq!(result.objects.first(), Some(&(baz_key.0, default_obj))); } #[test] @@ -467,15 +396,15 @@ mod tests { let (_, bar_key, baz_key) = create_and_put_objects(&mut state, &store).unwrap(); - let default_item = ObjectListItem::External((Cid::default(), false)); + let default_obj = default_object(); let foo_key = BytesKey("foo".as_bytes().to_vec()); let result = state.list(&store, foo_key.0.clone(), vec![], 0, 0); assert!(result.is_ok()); let result = result.unwrap(); assert_eq!(result.objects.len(), 3); - assert_eq!(result.objects[0], (baz_key.0, default_item.clone())); - assert_eq!(result.objects[1], (bar_key.0, default_item.clone())); + assert_eq!(result.objects[0], (baz_key.0, default_obj.clone())); + assert_eq!(result.objects[1], (bar_key.0, default_obj.clone())); } #[test] @@ -485,10 +414,7 @@ mod tests { let (jpeg_key, _, _) = create_and_put_objects(&mut state, &store).unwrap(); - let mh_code = Code::Blake2b256; - let mh = mh_code.digest(&[4, 5, 6]); - let cid = Cid::new_v1(DAG_CBOR, mh); - let default_item = ObjectListItem::Internal((cid, 3)); + let default_obj = default_object(); let foo_key = BytesKey("foo".as_bytes().to_vec()); let delimiter_key = BytesKey("/".as_bytes().to_vec()); @@ -497,7 +423,7 @@ mod tests { assert!(result.is_ok()); let result = result.unwrap(); assert_eq!(result.objects.len(), 1); - assert_eq!(result.objects[0], (jpeg_key.0, default_item)); + assert_eq!(result.objects[0], (jpeg_key.0, default_obj)); assert_eq!(result.common_prefixes[0], full_key); } @@ -508,28 +434,31 @@ mod tests { let jpeg_key = BytesKey("foo.jpeg".as_bytes().to_vec()); state - .put( + .add( &store, jpeg_key.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, ) .unwrap(); let bar_key = BytesKey("bin/foo/bar.png".as_bytes().to_vec()); state - .put( + .add( &store, bar_key.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, ) .unwrap(); let baz_key = BytesKey("bin/foo/baz.png".as_bytes().to_vec()); state - .put( + .add( &store, baz_key.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, ) .unwrap(); @@ -552,7 +481,7 @@ mod tests { let (_, bar_key, _) = create_and_put_objects(&mut state, &store).unwrap(); - let default_item = ObjectListItem::External((Cid::default(), false)); + let default_obj = default_object(); // List all keys with a limit and offset let result = state.list(&store, vec![], vec![], 1, 1); @@ -560,7 +489,7 @@ mod tests { let result = result.unwrap(); assert_eq!(result.objects.len(), 1); // Note that baz is listed first in order, so an offset of 1 will return bar - assert_eq!(result.objects.first(), Some(&(bar_key.0, default_item))); + assert_eq!(result.objects.first(), Some(&(bar_key.0, default_obj))); } #[test] @@ -570,19 +499,21 @@ mod tests { let one = BytesKey("hello/world".as_bytes().to_vec()); state - .put( + .add( &store, one.clone(), - ObjectKind::Internal(ByteBuf(vec![4, 5, 6])), + Cid::default(), + HashMap::::new(), false, ) .unwrap(); let two = BytesKey("hello/again".as_bytes().to_vec()); state - .put( + .add( &store, two.clone(), - ObjectKind::External(Cid::default()), + Cid::default(), + HashMap::::new(), false, ) .unwrap(); diff --git a/fendermint/app/src/cmd/objects.rs b/fendermint/app/src/cmd/objects.rs index 1c831e18..abe80c66 100644 --- a/fendermint/app/src/cmd/objects.rs +++ b/fendermint/app/src/cmd/objects.rs @@ -586,7 +586,7 @@ mod tests { use cid::multihash::{Code, MultihashDigest}; use ethers::core::k256::ecdsa::SigningKey; use ethers::core::rand::{rngs::StdRng, SeedableRng}; - use fendermint_actor_objectstore::{ObjectKind, PutParams}; + use fendermint_actor_objectstore::{AddParams, ObjectKind}; use fendermint_rpc::FendermintClient; use fendermint_vm_message::conv::from_eth::to_fvm_address; use fvm_ipld_encoding::RawBytes; @@ -741,7 +741,7 @@ mod tests { let external_object = b"hello world".as_ref(); let digest = Code::Blake2b256.digest(external_object); let object_cid = Cid::new_v1(fvm_ipld_encoding::IPLD_RAW, digest); - let params = PutParams { + let params = AddParams { key: key.to_vec(), kind: ObjectKind::External(object_cid), overwrite: true, diff --git a/fendermint/vm/interpreter/src/chain.rs b/fendermint/vm/interpreter/src/chain.rs index 72fbeccd..4a53f1c6 100644 --- a/fendermint/vm/interpreter/src/chain.rs +++ b/fendermint/vm/interpreter/src/chain.rs @@ -6,7 +6,7 @@ use async_stm::atomically; use async_trait::async_trait; use fendermint_actor_objectstore::{ GetParams, - Method::{GetObject, ResolveExternalObject}, + Method::{GetObject, ResolveObject}, }; use fendermint_tracing::emit; use fendermint_vm_actor_interface::{ipc, system}; @@ -513,10 +513,10 @@ where IpcMessage::ObjectResolved(obj) => { let from = system::SYSTEM_ACTOR_ADDR; let to = obj.address; - let method_num = ResolveExternalObject as u64; + let method_num = ResolveObject as u64; let gas_limit = fvm_shared::BLOCK_GAS_LIMIT; - let params = fendermint_actor_objectstore::ResolveExternalParams { + let params = fendermint_actor_objectstore::ResolveParams { key: obj.key, value: obj.value, }; From 2a2427bfdbefe93e1ca3d0728cd682920de05a21 Mon Sep 17 00:00:00 2001 From: Bruno Calza Date: Wed, 3 Jul 2024 14:18:47 -0300 Subject: [PATCH 2/3] changes to the objects api Signed-off-by: Bruno Calza --- fendermint/app/src/cmd/objects.rs | 50 ++++++++++++-------------- fendermint/vm/interpreter/src/chain.rs | 5 +-- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/fendermint/app/src/cmd/objects.rs b/fendermint/app/src/cmd/objects.rs index abe80c66..cdbd2d62 100644 --- a/fendermint/app/src/cmd/objects.rs +++ b/fendermint/app/src/cmd/objects.rs @@ -454,30 +454,21 @@ async fn handle_object_download match maybe_object { Some(object) => { - let object_range = match object { - Object::Internal(_) => { - return Err(Rejection::from(BadRequest { - message: "internal objects are not supported".to_string(), - })) - } - Object::External((buf, resolved)) => { - let cid = Cid::try_from(buf.0).map_err(|e| { - Rejection::from(BadRequest { - message: format!("failed to decode cid: {}", e), - }) - })?; - if !resolved { - return Err(Rejection::from(BadRequest { - message: "object is not resolved".to_string(), - })); - } - ipfs.get_object(range, cid).await.map_err(|e| { - Rejection::from(BadRequest { - message: format!("failed to fetch detached object {}", e), - }) - })? - } - }; + let cid = Cid::try_from(object.cid.0).map_err(|e| { + Rejection::from(BadRequest { + message: format!("failed to decode cid: {}", e), + }) + })?; + if !object.resolved { + return Err(Rejection::from(BadRequest { + message: "object is not resolved".to_string(), + })); + } + let object_range = ipfs.get_object(range, cid).await.map_err(|e| { + Rejection::from(BadRequest { + message: format!("failed to fetch object {}", e), + }) + })?; // If it is a HEAD request, we don't need to send the body // but we still need to send the Content-Length header @@ -582,11 +573,13 @@ async fn os_get( #[cfg(test)] mod tests { + use std::collections::HashMap; + use super::*; use cid::multihash::{Code, MultihashDigest}; use ethers::core::k256::ecdsa::SigningKey; use ethers::core::rand::{rngs::StdRng, SeedableRng}; - use fendermint_actor_objectstore::{AddParams, ObjectKind}; + use fendermint_actor_objectstore::AddParams; use fendermint_rpc::FendermintClient; use fendermint_vm_message::conv::from_eth::to_fvm_address; use fvm_ipld_encoding::RawBytes; @@ -667,7 +660,7 @@ mod tests { "info": "", "index": "0", "key": "", - "value": "mIUSGDIYoRhoGEUYeBh0GGUYchhuGGEYbBiCGFgYJAEYcBIYIA0YmQ4AGGsYaRhIGEIYvxjzDxhbGKUYQxjTGCAYVxiHGL0Y6xg0GHIY8hgwGM8Y3RgYGF0YVRi2GPIYphj1GDAYyxiSGGQYOhhLCgcYbRhlGHMYcxhhGGcYZRINCgQYZhhyGG8YbRIDGHQYMBgwGBgBEhgxCgIYdBhvEhgpGHQYMhhtGG4YZBg1GGoYaxh1GHYYbRhzGGEYZhg0GDUYNxh5GG0YcBhuGGYYMxhtGG8YbhhhGGwYaBgzGHYYbxh0GGgYZBhkGDUYbhhqGG8YeRgYAQ==", + "value": "mJoSGEcYoxhjGGMYaRhkGFgYJAEYcBIYIBiRGMEYgBjyGIEYohiGGB8YKRgdGDoYSRj4GOIYSgIYzhh/GNgY/RiPGFwYcRi5GNYY1BEYZhhFGFoCGMwYaBhyGGUYcxhvGGwYdhhlGGQY9RhoGG0YZRh0GGEYZBhhGHQYYRihGGUYXxhzGGkYehhlGGEYNhgwGNAYwRhtGDoYSwoHGG0YZRhzGHMYYRhnGGUSDQoEGGYYchhvGG0SAxh0GDAYMBgYARIYMQoCGHQYbxIYKRh0GDIYcBhjGDQYahhpGGIYbxhiGDMYdhhzGHQYMxh2GHAYchg1GDYYaBhuGDIYaRhpGGYYZhhlGGwYdhhsGHkYbRhoGHYYZBh5GGMYeRhoGGEYGAE=", "proof": null, "height": "6017", "codespace": "" @@ -743,7 +736,8 @@ mod tests { let object_cid = Cid::new_v1(fvm_ipld_encoding::IPLD_RAW, digest); let params = AddParams { key: key.to_vec(), - kind: ObjectKind::External(object_cid), + cid: object_cid, + metadata: HashMap::new(), overwrite: true, }; let params = RawBytes::serialize(params).unwrap(); @@ -759,7 +753,7 @@ mod tests { to, sequence: 0, value: TokenAmount::from_atto(0), - method_num: fendermint_actor_objectstore::Method::PutObject as u64, + method_num: fendermint_actor_objectstore::Method::AddObject as u64, params, gas_limit: 3000000, gas_fee_cap: TokenAmount::from_atto(0), diff --git a/fendermint/vm/interpreter/src/chain.rs b/fendermint/vm/interpreter/src/chain.rs index 4a53f1c6..b61bbc85 100644 --- a/fendermint/vm/interpreter/src/chain.rs +++ b/fendermint/vm/interpreter/src/chain.rs @@ -761,10 +761,7 @@ where .map_err(|e| anyhow!("error parsing as Option: {e}"))?; Ok(match object { - Some(object) => match object { - fendermint_actor_objectstore::Object::External((_, resolved)) => resolved, - fendermint_actor_objectstore::Object::Internal(_) => true, // cannot happen - }, + Some(object) => object.resolved, None => { // The object was deleted before it was resolved. // We can return true here because the objectstore actor will ignore the final implicit From 24d3dae978b280241c00beac3a36541d85a4b21a Mon Sep 17 00:00:00 2001 From: Bruno Calza Date: Thu, 4 Jul 2024 13:50:57 -0300 Subject: [PATCH 3/3] adds size to the object struct Signed-off-by: Bruno Calza --- fendermint/actors/objectstore/src/actor.rs | 1 + fendermint/actors/objectstore/src/shared.rs | 2 ++ fendermint/actors/objectstore/src/state.rs | 28 ++++++++++++++++++--- fendermint/app/src/cmd/objects.rs | 3 ++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/fendermint/actors/objectstore/src/actor.rs b/fendermint/actors/objectstore/src/actor.rs index 83239018..612db8a2 100644 --- a/fendermint/actors/objectstore/src/actor.rs +++ b/fendermint/actors/objectstore/src/actor.rs @@ -44,6 +44,7 @@ impl Actor { rt.store(), BytesKey(params.key), params.cid, + params.size, params.metadata, params.overwrite, ) diff --git a/fendermint/actors/objectstore/src/shared.rs b/fendermint/actors/objectstore/src/shared.rs index eac86334..70dc0859 100644 --- a/fendermint/actors/objectstore/src/shared.rs +++ b/fendermint/actors/objectstore/src/shared.rs @@ -21,6 +21,8 @@ pub struct AddParams { pub key: Vec, /// Object value. pub cid: Cid, + /// Object size. + pub size: usize, /// Object metadata. pub metadata: HashMap, /// Whether to overwrite a key if it already exists. diff --git a/fendermint/actors/objectstore/src/state.rs b/fendermint/actors/objectstore/src/state.rs index b72e8f1e..d245ef45 100644 --- a/fendermint/actors/objectstore/src/state.rs +++ b/fendermint/actors/objectstore/src/state.rs @@ -45,6 +45,8 @@ impl MachineState for State { pub struct Object { /// The object content identifier. pub cid: ByteBuf, + /// The size of the content. + pub size: usize, /// Whether the object has been resolved. pub resolved: bool, /// User-defined object metadata (e.g., size, last modified timestamp, etc.). @@ -87,12 +89,14 @@ impl State { store: &BS, key: BytesKey, cid: Cid, + size: usize, metadata: HashMap, overwrite: bool, ) -> anyhow::Result { let mut hamt = Hamt::<_, Object>::load_with_bit_width(&self.root, store, BIT_WIDTH)?; let object = Object { cid: ByteBuf(cid.to_bytes()), + size, resolved: false, metadata, }; @@ -219,6 +223,7 @@ mod tests { fn arbitrary(g: &mut quickcheck::Gen) -> Self { Object { cid: ByteBuf(ArbCid::<64>::arbitrary(g).0.to_bytes()), + size: usize::arbitrary(g), metadata: HashMap::arbitrary(g), resolved: false, } @@ -228,6 +233,7 @@ mod tests { fn default_object() -> Object { Object { cid: ByteBuf(Cid::default().to_bytes()), + size: 0, metadata: HashMap::::new(), resolved: false, } @@ -243,12 +249,13 @@ mod tests { metadata.insert("_modified".to_string(), String::from("1718464345")); Object { cid: ByteBuf(cid.to_bytes()), + size: 5, metadata, resolved: false, } } - const GOLDEN_CID: &str = "bafy2bzacebi2bfkcucl3r2oih4du4hd7jrlvm7cqgggofnkzzz3m4asycsbmq"; + const GOLDEN_CID: &str = "bafy2bzacebmog6w3ept45xctbw3lrt76i3rdbeaib6bikuhcddu5y5bqspozu"; #[test] fn test_constructor() { @@ -272,6 +279,7 @@ mod tests { &store, BytesKey(vec![1, 2, 3]), Cid::from_bytes(&object.cid.0).unwrap(), + object.size, object.metadata, true ) @@ -287,7 +295,9 @@ mod tests { let key = BytesKey(vec![1, 2, 3]); let cid = Cid::from_bytes(&object.cid.0).unwrap(); let md = object.metadata.clone(); - state.add(&store, key.clone(), cid, md, true).unwrap(); + state + .add(&store, key.clone(), cid, object.size, md, true) + .unwrap(); assert!(state.resolve(&store, key.clone(), cid).is_ok()); object.resolved = true; @@ -306,6 +316,7 @@ mod tests { &store, key.clone(), Cid::from_bytes(&object.cid.0).unwrap(), + object.size, object.metadata, true, ) @@ -324,7 +335,9 @@ mod tests { let key = BytesKey(vec![1, 2, 3]); let cid = Cid::from_bytes(&object.cid.0).unwrap(); let md = object.metadata.clone(); - state.add(&store, key.clone(), cid, md, true).unwrap(); + state + .add(&store, key.clone(), cid, object.size, md, true) + .unwrap(); let result = state.get(&store, &key); assert!(result.is_ok()); @@ -340,6 +353,7 @@ mod tests { store, jpeg_key.clone(), Cid::default(), + 0, HashMap::::new(), false, )?; @@ -348,6 +362,7 @@ mod tests { store, bar_key.clone(), Cid::default(), + 0, HashMap::::new(), false, )?; @@ -356,6 +371,7 @@ mod tests { store, baz_key.clone(), Cid::default(), + 0, HashMap::::new(), false, )?; @@ -366,6 +382,7 @@ mod tests { &store, other_key.clone(), Cid::default(), + 0, HashMap::::new(), false, )?; @@ -438,6 +455,7 @@ mod tests { &store, jpeg_key.clone(), Cid::default(), + 0, HashMap::::new(), false, ) @@ -448,6 +466,7 @@ mod tests { &store, bar_key.clone(), Cid::default(), + 0, HashMap::::new(), false, ) @@ -458,6 +477,7 @@ mod tests { &store, baz_key.clone(), Cid::default(), + 0, HashMap::::new(), false, ) @@ -503,6 +523,7 @@ mod tests { &store, one.clone(), Cid::default(), + 0, HashMap::::new(), false, ) @@ -513,6 +534,7 @@ mod tests { &store, two.clone(), Cid::default(), + 0, HashMap::::new(), false, ) diff --git a/fendermint/app/src/cmd/objects.rs b/fendermint/app/src/cmd/objects.rs index cdbd2d62..d371da22 100644 --- a/fendermint/app/src/cmd/objects.rs +++ b/fendermint/app/src/cmd/objects.rs @@ -660,7 +660,7 @@ mod tests { "info": "", "index": "0", "key": "", - "value": "mJoSGEcYoxhjGGMYaRhkGFgYJAEYcBIYIBiRGMEYgBjyGIEYohiGGB8YKRgdGDoYSRj4GOIYSgIYzhh/GNgY/RiPGFwYcRi5GNYY1BEYZhhFGFoCGMwYaBhyGGUYcxhvGGwYdhhlGGQY9RhoGG0YZRh0GGEYZBhhGHQYYRihGGUYXxhzGGkYehhlGGEYNhgwGNAYwRhtGDoYSwoHGG0YZRhzGHMYYRhnGGUSDQoEGGYYchhvGG0SAxh0GDAYMBgYARIYMQoCGHQYbxIYKRh0GDIYcBhjGDQYahhpGGIYbxhiGDMYdhhzGHQYMxh2GHAYchg1GDYYaBhuGDIYaRhpGGYYZhhlGGwYdhhsGHkYbRhoGHYYZBh5GGMYeRhoGGEYGAE=", + "value": "mKASGE0YpBhjGGMYaRhkGFgYJAEYcBIYIBilGGgY5AQY2BjqGNoY7BiSGFoYwxi8GBsYNhglGJwPGG0YcAANGHIYnBjZGBgYxBhqGIMYbxhJGHYYVxhkGHMYaRh6GGUGGGgYchhlGHMYbxhsGHYYZRhkGPUYaBhtGGUYdBhhGGQYYRh0GGEYoRhlGF8YcxhpGHoYZRhhGDYYMBiiGNgYbhg6GEsKBxhtGGUYcxhzGGEYZxhlEg0KBBhmGHIYbxhtEgMYdBgwGDAYGAESGDEKAhh0GG8SGCkYdBgyGHcYdRhoGHEYNxh0GGEYMxgzGGUYdxgzGGMYMhhuGGQYbRhnGGIYbBg3GDcYNxh6GDUYeBg0GGQYbBh5GGYYbhhtGHkYbBhqGGkYaBhxGBgB", "proof": null, "height": "6017", "codespace": "" @@ -737,6 +737,7 @@ mod tests { let params = AddParams { key: key.to_vec(), cid: object_cid, + size: 11, metadata: HashMap::new(), overwrite: true, };