diff --git a/Cargo.lock b/Cargo.lock index 26adb68fc..4c6b13913 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2350,6 +2350,7 @@ dependencies = [ "replace_with", "serde", "serde_tuple", + "static_assertions", "thiserror", "wasmtime", "wasmtime-environ", diff --git a/fvm/Cargo.toml b/fvm/Cargo.toml index 11e75eb56..4cb34071f 100644 --- a/fvm/Cargo.toml +++ b/fvm/Cargo.toml @@ -38,6 +38,7 @@ rand = "0.8.5" quickcheck = { version = "1", optional = true } once_cell = "1.18" minstant = "0.1.2" +static_assertions = "1.1.0" [dev-dependencies] pretty_assertions = "1.3.0" diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index 83a97ddb1..47984826a 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -7,7 +7,6 @@ use std::ops::Mul; use anyhow::Context; use fvm_shared::crypto::signature::SignatureType; -use fvm_shared::event::{ActorEvent, Flags}; use fvm_shared::piece::PieceInfo; use fvm_shared::sector::{ AggregateSealVerifyProofAndInfos, RegisteredPoStProof, RegisteredSealProof, ReplicaUpdateInfo, @@ -27,6 +26,21 @@ use crate::kernel::SupportedHashes; // https://docs.rs/wasmtime/2.0.2/wasmtime/struct.InstanceLimits.html#structfield.table_elements const TABLE_ELEMENT_SIZE: u32 = 8; +// The maximum overhead (in bytes) of a single event when encoded into CBOR. +// +// 1: CBOR tuple with 2 fields (StampedEvent) +// 9: Emitter ID +// 2: Entry array overhead (max size 255) +const EVENT_OVERHEAD: u64 = 12; +// The maximum overhead (in bytes) of a single event entry when encoded into CBOR. +// +// 1: CBOR tuple with 4 fields +// 1: Flags (will adjust as more flags are added) +// 2: Key major type + length (up to 255 bytes) +// 2: Codec major type + value (codec should be <= 255) +// 3: Value major type + length (up to 8192 bytes) +const EVENT_ENTRY_OVERHEAD: u64 = 9; + /// Create a mapping from enum items to values in a way that guarantees at compile /// time that we did not miss any member, in any of the prices, even if the enum /// gets a new member later. @@ -293,26 +307,15 @@ lazy_static! { memory_fill_per_byte_cost: Gas::from_milligas(400), }, - // These parameters are specifically sized for EVM events. They will need - // to be revisited before Wasm actors are able to emit arbitrary events. - // - // Validation costs are dependent on encoded length, but also - // co-dependent on the number of entries. The latter is a chicken-and-egg - // situation because we can only learn how many entries were emitted once we - // decode the CBOR event. - // - // We will likely need to revisit the ABI of emit_event to remove CBOR - // as the exchange format. - event_validation_cost: ScalingCost { + // TODO(#1817): Per-entry event validation cost. These parameters were benchmarked for the + // EVM but haven't been revisited since revising the API. + event_per_entry: ScalingCost { flat: Gas::new(1750), scale: Gas::new(25), }, - // The protocol does not currently mandate indexing work, so these are - // left at zero. Once we start populating and committing indexing data - // structures, these costs will need to reflect the computation and - // storage costs of such actions. - event_accept_per_index_element: ScalingCost { + // TODO(#1817): Cost of validating utf8 (used in event parsing). + utf8_validation: ScalingCost { flat: Zero::zero(), scale: Zero::zero(), }, @@ -468,14 +471,14 @@ pub struct PriceList { /// Gas cost to validate an ActorEvent as soon as it's received from the actor, and prior /// to it being parsed. - pub(crate) event_validation_cost: ScalingCost, - - /// Gas cost of every indexed element, scaling per number of bytes indexed. - pub(crate) event_accept_per_index_element: ScalingCost, + pub(crate) event_per_entry: ScalingCost, /// Gas cost of doing lookups in the builtin actor mappings. pub(crate) builtin_actor_manifest_lookup: Gas, + /// Gas cost of utf8 parsing. + pub(crate) utf8_validation: ScalingCost, + /// Gas cost of accessing the network context. pub(crate) network_context: Gas, /// Gas cost of accessing the message context. @@ -919,58 +922,33 @@ impl PriceList { } #[inline] - pub fn on_actor_event_validate(&self, data_size: usize) -> GasCharge { - let memcpy = self.block_memcpy.apply(data_size); - let alloc = self.block_allocate.apply(data_size); - let validate = self.event_validation_cost.apply(data_size); - - GasCharge::new( - "OnActorEventValidate", - memcpy + alloc + validate, - Zero::zero(), - ) - } - - #[inline] - pub fn on_actor_event_accept(&self, evt: &ActorEvent, serialized_len: usize) -> GasCharge { - let (mut indexed_bytes, mut indexed_elements) = (0usize, 0u32); - for evt in evt.entries.iter() { - if evt.flags.contains(Flags::FLAG_INDEXED_KEY) { - indexed_bytes += evt.key.len(); - indexed_elements += 1; - } - if evt.flags.contains(Flags::FLAG_INDEXED_VALUE) { - indexed_bytes += evt.value.len(); - indexed_elements += 1; - } - } + pub fn on_actor_event(&self, entries: usize, keysize: usize, valuesize: usize) -> GasCharge { + // Here we estimate per-event overhead given the constraints on event values. - // The estimated size of the serialized StampedEvent event, which - // includes the ActorEvent + 8 bytes for the actor ID - const STAMP_EXTRA_SIZE: usize = 8; - let stamped_event_size = serialized_len + STAMP_EXTRA_SIZE; + let validate_entries = self.event_per_entry.apply(entries); + let validate_utf8 = self.utf8_validation.apply(keysize); - // Charge for 3 memory copy operations. - // This includes the cost of forming a StampedEvent, copying into the - // AMT's buffer on finish, and returning to the client. - let memcpy = self.block_memcpy.apply(stamped_event_size); + // Estimate the size, saturating at max-u64. Given how we calculate gas, this will saturate + // the gas maximum at max-u64 milligas. + let estimated_size = EVENT_OVERHEAD + .saturating_add(EVENT_ENTRY_OVERHEAD.saturating_mul(entries as u64)) + .saturating_add(keysize as u64) + .saturating_add(valuesize as u64); - // Charge for 2 memory allocations. - // This includes the cost of retaining the StampedEvent in the call manager, - // and allocaing into the AMT's buffer on finish. - let alloc = self.block_allocate.apply(stamped_event_size); + // Calculate the cost per copy (one memcpy + one allocation). + let mem = + self.block_memcpy.apply(estimated_size) + self.block_allocate.apply(estimated_size); // Charge for the hashing on AMT insertion. - let hash = self.hashing_cost[&SupportedHashes::Blake2b256].apply(stamped_event_size); + let hash = self.hashing_cost[&SupportedHashes::Blake2b256].apply(estimated_size); GasCharge::new( - "OnActorEventAccept", - memcpy + alloc, - self.event_accept_per_index_element.flat * indexed_elements - + self.event_accept_per_index_element.scale * indexed_bytes - + memcpy * 2u32 // deferred cost, placing here to hide from benchmark - + alloc // deferred cost, placing here to hide from benchmark - + hash, // deferred cost, placing here to hide from benchmark + "OnActorEvent", + // Charge for validation/storing events. + mem + validate_entries + validate_utf8, + // Charge for forming the AMT and returning the events to the client. + // one copy into the AMT, one copy to the client. + hash + (mem * 2u32), ) } } diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 10665c1d7..493b86f19 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -17,7 +17,7 @@ use fvm_shared::consensus::ConsensusFault; use fvm_shared::crypto::signature; use fvm_shared::econ::TokenAmount; use fvm_shared::error::ErrorNumber; -use fvm_shared::event::{ActorEvent, Entry, EntryFixed, Flags}; +use fvm_shared::event::{ActorEvent, Entry, Flags}; use fvm_shared::piece::{zero_piece_commitment, PaddedPieceSize}; use fvm_shared::sector::RegisteredPoStProof::{StackedDRGWindow32GiBV1, StackedDRGWindow32GiBV1P1}; use fvm_shared::sector::{RegisteredPoStProof, SectorInfo}; @@ -1006,72 +1006,55 @@ impl EventOps for DefaultKernel where C: CallManager, { - fn emit_event(&mut self, raw_evt: &[u8], raw_key: &[u8], raw_val: &[u8]) -> Result<()> { - const MAX_NR_ENTRIES: usize = 256; - const MAX_KEY_LEN: usize = 32; + fn emit_event( + &mut self, + event_headers: &[fvm_shared::sys::EventEntry], + event_keys: &[u8], + event_values: &[u8], + ) -> Result<()> { + const MAX_NR_ENTRIES: usize = 255; + const MAX_KEY_LEN: usize = 31; const MAX_VALUE_LEN: usize = 8 << 10; if self.read_only { return Err(syscall_error!(ReadOnly; "cannot emit events while read-only").into()); } - if raw_val.len() > MAX_VALUE_LEN { - return Err( - syscall_error!(IllegalArgument; "event total values exceeded max size: {} > {MAX_VALUE_LEN}", raw_val.len()).into(), - ); - } - - let total_buf_len = raw_evt.len() + raw_key.len() + raw_val.len(); - - let _t = self.call_manager.charge_gas( - self.call_manager - .price_list() - .on_actor_event_validate(total_buf_len), - )?; - - const BYTES_PER_ENTRY: usize = 24; - if raw_evt.len() % BYTES_PER_ENTRY != 0 { - return Err(syscall_error!(IllegalArgument; "event not divisible by 24 bytes").into()); - } - let nr_entries = raw_evt.len() / BYTES_PER_ENTRY; - - if nr_entries > MAX_NR_ENTRIES { - return Err(syscall_error!(IllegalArgument; "event exceeded max entries: {} > {MAX_NR_ENTRIES}", nr_entries).into()); + let t = self + .call_manager + .charge_gas(self.call_manager.price_list().on_actor_event( + event_headers.len(), + event_keys.len(), + event_values.len(), + ))?; + + if event_headers.len() > MAX_NR_ENTRIES { + return Err(syscall_error!(IllegalArgument; "event exceeded max entries: {} > {MAX_NR_ENTRIES}", event_headers.len()).into()); } let mut key_offset: usize = 0; let mut val_offset: usize = 0; - let mut entries: Vec = Vec::with_capacity(nr_entries); - for i in 0..nr_entries { - let offset = i * BYTES_PER_ENTRY; - let view = &raw_evt[offset..offset + BYTES_PER_ENTRY]; - - // parse the fixed sized fields from the raw_evt buffer - // - let entry_fixed: EntryFixed = unsafe { - std::mem::transmute::<[u8; BYTES_PER_ENTRY], EntryFixed>(view.try_into().unwrap()) - }; - + let mut entries: Vec = Vec::with_capacity(event_headers.len()); + for header in event_headers { // make sure that the fixed parsed values are within bounds before we do any allocation - // - let flags = entry_fixed.flags; + let flags = header.flags; if Flags::from_bits(flags.bits()).is_none() { return Err( syscall_error!(IllegalArgument; "event flags are invalid: {}", flags.bits()) .into(), ); } - if entry_fixed.key_len > MAX_KEY_LEN as u32 { - let tmp = entry_fixed.key_len; + if header.key_len > MAX_KEY_LEN as u32 { + let tmp = header.key_len; return Err(syscall_error!(IllegalArgument; "event key exceeded max size: {} > {MAX_KEY_LEN}", tmp).into()); } - if entry_fixed.val_len > MAX_VALUE_LEN as u32 { - let tmp = entry_fixed.val_len; + if header.val_len > MAX_VALUE_LEN as u32 { + let tmp = header.val_len; return Err(syscall_error!(IllegalArgument; "event value exceeded max size: {} > {MAX_VALUE_LEN}", tmp).into()); } - if entry_fixed.codec != IPLD_RAW { - let tmp = entry_fixed.codec; + if header.codec != IPLD_RAW { + let tmp = header.codec; return Err( syscall_error!(IllegalCodec; "event codec must be IPLD_RAW, was: {}", tmp) .into(), @@ -1079,40 +1062,37 @@ where } // parse the variable sized fields from the raw_key/raw_val buffers - // - let key = match std::str::from_utf8( - &raw_key[key_offset..key_offset + entry_fixed.key_len as usize], - ) { - Ok(s) => s, - Err(e) => { - return Err(syscall_error!(IllegalArgument, "invalid event key: {}", e).into()) - } - }; - let value = &raw_val[val_offset..val_offset + entry_fixed.val_len as usize]; + let key = &event_keys + .get(key_offset..key_offset + header.key_len as usize) + .context("event entry key out of range") + .or_illegal_argument()?; + + let key = std::str::from_utf8(key) + .context("invalid event key") + .or_illegal_argument()?; + + let value = &event_values + .get(val_offset..val_offset + header.val_len as usize) + .context("event entry value out of range") + .or_illegal_argument()?; // we have all we need to construct a new Entry let entry = Entry { - flags: entry_fixed.flags, + flags: header.flags, key: key.to_string(), - codec: entry_fixed.codec, + codec: header.codec, value: value.to_vec(), }; // shift the key/value offsets - key_offset += entry_fixed.key_len as usize; - val_offset += entry_fixed.val_len as usize; + key_offset += header.key_len as usize; + val_offset += header.val_len as usize; entries.push(entry); } let actor_evt = ActorEvent::from(entries); - let t = self.call_manager.charge_gas( - self.call_manager - .price_list() - .on_actor_event_accept(&actor_evt, total_buf_len), - )?; - let stamped_evt = StampedEvent::new(self.actor_id, actor_evt); self.call_manager.append_event(stamped_evt); diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index 59997ea7e..f64f7cb62 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -373,5 +373,10 @@ pub trait LimiterOps { /// Eventing APIs. pub trait EventOps { /// Records an event emitted throughout execution. - fn emit_event(&mut self, raw_evt: &[u8], raw_key: &[u8], raw_val: &[u8]) -> Result<()>; + fn emit_event( + &mut self, + event_headers: &[fvm_shared::sys::EventEntry], + raw_key: &[u8], + raw_val: &[u8], + ) -> Result<()>; } diff --git a/fvm/src/syscalls/event.rs b/fvm/src/syscalls/event.rs index 653fd62e1..aac2fba96 100644 --- a/fvm/src/syscalls/event.rs +++ b/fvm/src/syscalls/event.rs @@ -1,14 +1,15 @@ // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT +use anyhow::Context as _; + use super::Context; -use crate::kernel::Result; +use crate::kernel::{ClassifyResult, Result}; use crate::Kernel; /// Emits an actor event. The event is split into three raw byte buffers that have /// been written to Wasm memory. This is done so that the FVM can accurately charge /// for gas without needing to parse anything inside the FVM. -/// /// The buffers are serialized as follows: /// - event_off/event_len: The offset and length tuple of all the event entries /// flags:u64,key_len:u32,codec:u64,value_len:u32) @@ -32,8 +33,21 @@ pub fn emit_event( val_off: u32, val_len: u32, ) -> Result<()> { - let raw_event = context.memory.try_slice(event_off, event_len)?; + let event_headers = unsafe { + const EVENT_SIZE: u32 = std::mem::size_of::() as u32; + // assert the alignment so we can safely cast from a byte-slice + static_assertions::assert_eq_align!(fvm_shared::sys::EventEntry, u8); + let size = event_len + .checked_mul(EVENT_SIZE) + .context("events index out of bounds") + .or_illegal_argument()?; + let buf = context.memory.try_slice(event_off, size)?; + std::slice::from_raw_parts( + buf.as_ptr() as *const fvm_shared::sys::EventEntry, + event_len as usize, + ) + }; let raw_key = context.memory.try_slice(key_off, key_len)?; let raw_val = context.memory.try_slice(val_off, val_len)?; - context.kernel.emit_event(raw_event, raw_key, raw_val) + context.kernel.emit_event(event_headers, raw_key, raw_val) } diff --git a/sdk/src/event.rs b/sdk/src/event.rs index de3140ce2..b46244db1 100644 --- a/sdk/src/event.rs +++ b/sdk/src/event.rs @@ -1,9 +1,7 @@ -use std::mem::size_of; - // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT use crate::{sys, SyscallResult}; -use fvm_shared::event::{ActorEvent, EntryFixed}; +use fvm_shared::event::ActorEvent; pub fn emit_event(evt: &ActorEvent) -> SyscallResult<()> { // we manually serialize the ActorEvent (not using CBOR) into three byte arrays so @@ -15,7 +13,7 @@ pub fn emit_event(evt: &ActorEvent) -> SyscallResult<()> { for i in 0..evt.entries.len() { let e = &evt.entries[i]; - fixed_entries.push(EntryFixed { + fixed_entries.push(fvm_shared::sys::EventEntry { flags: e.flags, codec: e.codec, key_len: e.key.len() as u32, @@ -37,10 +35,9 @@ pub fn emit_event(evt: &ActorEvent) -> SyscallResult<()> { } unsafe { - let fixed_size_in_bytes = fixed_entries.len() * size_of::(); sys::event::emit_event( - fixed_entries.as_ptr() as *const u8, - fixed_size_in_bytes as u32, + fixed_entries.as_ptr(), + fixed_entries.len() as u32, keys.as_ptr(), keys.len() as u32, values.as_ptr(), diff --git a/sdk/src/sys/event.rs b/sdk/src/sys/event.rs index 99c6b7dca..bf091116e 100644 --- a/sdk/src/sys/event.rs +++ b/sdk/src/sys/event.rs @@ -6,6 +6,10 @@ #[cfg(doc)] use crate::sys::ErrorNumber::*; +// For documentation +#[doc(inline)] +pub use fvm_shared::sys::EventEntry; + super::fvm_syscalls! { module = "event"; @@ -18,7 +22,7 @@ super::fvm_syscalls! { /// | [`IllegalArgument`] | entries failed to validate due to improper encoding or invalid data | /// | [`ReadOnly`] | cannot send events while read-only | pub fn emit_event( - evt_off: *const u8, + evt_off: *const EventEntry, evt_len: u32, key_off: *const u8, key_len: u32, diff --git a/shared/src/event/mod.rs b/shared/src/event/mod.rs index faab88fae..ad983d7e5 100644 --- a/shared/src/event/mod.rs +++ b/shared/src/event/mod.rs @@ -39,6 +39,7 @@ impl From> for ActorEvent { bitflags! { /// Flags associated with an Event entry. #[derive(Deserialize, Serialize, Copy, Clone, Eq, PartialEq, Debug)] + #[repr(transparent)] // we pass this type through a syscall #[serde(transparent)] pub struct Flags: u64 { const FLAG_INDEXED_KEY = 0b00000001; @@ -60,12 +61,3 @@ pub struct Entry { #[serde(with = "strict_bytes")] pub value: Vec, } - -// A fixed sized struct for serializing Entry separately from the key/value bytes. -#[repr(C, packed)] -pub struct EntryFixed { - pub flags: Flags, - pub codec: u64, - pub key_len: u32, - pub val_len: u32, -} diff --git a/shared/src/sys/mod.rs b/shared/src/sys/mod.rs index 2517c0bcd..9529b8a36 100644 --- a/shared/src/sys/mod.rs +++ b/shared/src/sys/mod.rs @@ -67,6 +67,16 @@ impl SendFlags { } } +/// A fixed sized struct for serializing an [event `Entry`](crate::event::Entry) separately from the +/// key/value bytes. +#[repr(C, packed)] +pub struct EventEntry { + pub flags: crate::event::Flags, + pub codec: u64, + pub key_len: u32, + pub val_len: u32, +} + /// An unsafe trait to mark "syscall safe" types. These types must be safe to memcpy to and from /// WASM. This means: /// diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index f3ffcf4ec..5fd196242 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -28,7 +28,7 @@ use fvm_shared::sector::{ AggregateSealVerifyProofAndInfos, RegisteredSealProof, ReplicaUpdateInfo, SealVerifyInfo, WindowPoStVerifyInfo, }; -use fvm_shared::sys::SendFlags; +use fvm_shared::sys::{EventEntry, SendFlags}; use fvm_shared::version::NetworkVersion; use fvm_shared::{ActorID, MethodNum, TOTAL_FILECOIN}; @@ -564,8 +564,13 @@ where C: CallManager>, K: Kernel, { - fn emit_event(&mut self, raw_evt: &[u8], key_evt: &[u8], val_evt: &[u8]) -> Result<()> { - self.0.emit_event(raw_evt, key_evt, val_evt) + fn emit_event( + &mut self, + event_headers: &[EventEntry], + key_evt: &[u8], + val_evt: &[u8], + ) -> Result<()> { + self.0.emit_event(event_headers, key_evt, val_evt) } } diff --git a/testing/test_actors/actors/fil-events-actor/src/actor.rs b/testing/test_actors/actors/fil-events-actor/src/actor.rs index 8d4d95020..2281cf166 100644 --- a/testing/test_actors/actors/fil-events-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-events-actor/src/actor.rs @@ -50,16 +50,19 @@ pub fn invoke(params: u32) -> u32 { sdk::event::emit_event(&multi_entry.into()).unwrap(); } EMIT_MALFORMED => unsafe { - // mangle an event. - let mut serialized = fvm_ipld_encoding::to_vec(&single_entry_evt).unwrap(); - serialized[1] = 0xff; - + // Trigger an out of bounds. + let entry = fvm_shared::sys::EventEntry { + flags: Flags::empty(), + codec: IPLD_RAW, + key_len: 5, + val_len: 0, + }; assert!( sdk::sys::event::emit_event( - serialized.as_ptr(), - serialized.len() as u32, + &entry as *const fvm_shared::sys::EventEntry, + 1, 0 as *const u8, - 0, + 4, 0 as *const u8, 0, )