From 819c2003b97b397cd2215018c9722a8ca8b9343b Mon Sep 17 00:00:00 2001 From: Fridrik Asmundsson Date: Wed, 5 Jul 2023 11:19:45 +0000 Subject: [PATCH] feat: Improved event syscall API --- Cargo.lock | 1 + fvm/src/gas/price_list.rs | 5 +- fvm/src/kernel/default.rs | 141 +++++++++++------- fvm/src/kernel/mod.rs | 2 +- fvm/src/syscalls/event.rs | 26 +++- sdk/Cargo.toml | 1 + sdk/src/event.rs | 54 ++++++- sdk/src/sys/event.rs | 7 +- testing/conformance/src/vm.rs | 4 +- testing/integration/tests/events_test.rs | 6 +- testing/integration/tests/gasfuzz.rs | 6 +- .../actors/fil-events-actor/src/actor.rs | 12 +- 12 files changed, 190 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c0c54f0e6..b33d73f07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2725,6 +2725,7 @@ dependencies = [ name = "fvm_sdk" version = "3.3.0" dependencies = [ + "byteorder", "cid 0.10.1", "fvm_ipld_encoding 0.4.0", "fvm_shared 3.4.0", diff --git a/fvm/src/gas/price_list.rs b/fvm/src/gas/price_list.rs index 5390bd666..3186027d5 100644 --- a/fvm/src/gas/price_list.rs +++ b/fvm/src/gas/price_list.rs @@ -946,9 +946,8 @@ impl PriceList { } // The estimated size of the serialized StampedEvent event, which - // includes the ActorEvent + 8 bytes for the actor ID + some bytes - // for CBOR framing. - const STAMP_EXTRA_SIZE: usize = 12; + // includes the ActorEvent + 8 bytes for the actor ID + const STAMP_EXTRA_SIZE: usize = 8; let stamped_event_size = serialized_len + STAMP_EXTRA_SIZE; // Charge for 3 memory copy operations. diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 997334948..b2774f03c 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -1,5 +1,6 @@ // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT +use byteorder::{BigEndian, ByteOrder}; use std::collections::BTreeMap; use std::convert::{TryFrom, TryInto}; use std::panic::{self, UnwindSafe}; @@ -17,7 +18,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; +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,41 +1007,102 @@ impl EventOps for DefaultKernel where C: CallManager, { - fn emit_event(&mut self, raw_evt: &[u8]) -> Result<()> { + 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; + const MAX_VALUE_LEN: usize = 8 << 10; + if self.read_only { return Err(syscall_error!(ReadOnly; "cannot emit events while read-only").into()); } - let len = raw_evt.len(); - let t = self - .call_manager - .charge_gas(self.call_manager.price_list().on_actor_event_validate(len))?; - // This is an over-estimation of the maximum event size, for safety. No valid event can even - // get close to this. We check this first so we don't try to decode a large event. - const MAX_ENCODED_SIZE: usize = 1 << 20; - if raw_evt.len() > MAX_ENCODED_SIZE { - return Err(syscall_error!(IllegalArgument; "event WAY too large").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 actor_evt = { - let res = match panic::catch_unwind(|| { - fvm_ipld_encoding::from_slice(raw_evt).or_error(ErrorNumber::IllegalArgument) - }) { - Ok(v) => v, - Err(e) => { - log::error!("panic when decoding event cbor from actor: {:?}", e); - Err(syscall_error!(IllegalArgument; "panic when decoding event cbor from actor").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 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 flags = match Flags::from_bits(BigEndian::read_u64(&view[..8])) { + Some(f) => f, + None => return Err(syscall_error!(IllegalArgument, "invalid flag").into()), }; - t.stop(); - res - }?; - validate_actor_event(&actor_evt)?; + let key_len = BigEndian::read_u32(&view[8..12]); + let codec = BigEndian::read_u64(&view[12..20]); + let val_len = BigEndian::read_u32(&view[20..24]); + + // make sure that the fixed parsed values are within bounds before we do any allocation + // + if key_len > MAX_KEY_LEN as u32 { + return Err(syscall_error!(IllegalArgument; "event key exceeded max size: {} > {MAX_KEY_LEN}", key_len).into()); + } + if val_len > MAX_VALUE_LEN as u32 { + return Err(syscall_error!(IllegalArgument; "event value exceeded max size: {} > {MAX_VALUE_LEN}", val_len).into()); + } + if codec != IPLD_RAW { + return Err( + syscall_error!(IllegalCodec; "event codec must be IPLD_RAW, was: {}", codec) + .into(), + ); + } + + // 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 + key_len as usize]) + { + Ok(s) => s, + Err(_) => return Err(syscall_error!(IllegalArgument, "event key").into()), + }; + let value = &raw_val[val_offset..val_offset + val_len as usize]; + + // we have all we need to construct a new Entry + let entry = Entry { + flags: flags, + key: key.to_string(), + codec: codec, + value: value.to_vec(), + }; + + // shift the key/value offsets + key_offset += key_len as usize; + val_offset += 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, len), + .on_actor_event_accept(&actor_evt, total_buf_len), )?; let stamped_evt = StampedEvent::new(self.actor_id, actor_evt); @@ -1062,35 +1124,6 @@ fn catch_and_log_panic Result + UnwindSafe, R>(context: &str, } } -fn validate_actor_event(evt: &ActorEvent) -> Result<()> { - const MAX_ENTRIES: usize = 256; - const MAX_DATA: usize = 8 << 10; - const MAX_KEY_LEN: usize = 32; - - if evt.entries.len() > MAX_ENTRIES { - return Err(syscall_error!(IllegalArgument; "event exceeded max entries: {} > {MAX_ENTRIES}", evt.entries.len()).into()); - } - let mut total_value_size: usize = 0; - for entry in &evt.entries { - if entry.key.len() > MAX_KEY_LEN { - return Err(syscall_error!(IllegalArgument; "event key exceeded max size: {} > {MAX_KEY_LEN}", entry.key.len()).into()); - } - if entry.codec != IPLD_RAW { - return Err( - syscall_error!(IllegalCodec; "event codec must be IPLD_RAW, was: {}", entry.codec) - .into(), - ); - } - total_value_size += entry.value.len(); - } - if total_value_size > MAX_DATA { - return Err( - syscall_error!(IllegalArgument; "event total values exceeded max size: {total_value_size} > {MAX_DATA}").into(), - ); - } - Ok(()) -} - fn prover_id_from_u64(id: u64) -> ProverId { let mut prover_id = ProverId::default(); let prover_bytes = Address::new_id(id).payload().to_raw_bytes(); diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index b094c3dc5..59997ea7e 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -373,5 +373,5 @@ pub trait LimiterOps { /// Eventing APIs. pub trait EventOps { /// Records an event emitted throughout execution. - fn emit_event(&mut self, raw_evt: &[u8]) -> Result<()>; + fn emit_event(&mut self, raw_evt: &[u8], raw_key: &[u8], raw_val: &[u8]) -> Result<()>; } diff --git a/fvm/src/syscalls/event.rs b/fvm/src/syscalls/event.rs index 490518414..653fd62e1 100644 --- a/fvm/src/syscalls/event.rs +++ b/fvm/src/syscalls/event.rs @@ -5,8 +5,18 @@ use super::Context; use crate::kernel::Result; use crate::Kernel; -/// Emits an actor event. It takes an DAG-CBOR encoded ActorEvent that has been -/// written to Wasm memory, as an offset and length tuple. +/// 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) +/// - key_off/key_len: The offset and length tuple of all entry keys in the event +/// - val_off/val_len: The offset and length tuple of all entry values in the event +/// +/// During deserialization, the key_len/value_len stored in each entry will be used +/// to parse the keys and values from the key/value buffers. /// /// The FVM validates the structural, syntatic, and semantic correctness of the /// supplied event, and errors with `IllegalArgument` if the payload was invalid. @@ -15,9 +25,15 @@ use crate::Kernel; /// if such condition arises. pub fn emit_event( context: Context<'_, impl Kernel>, - event_off: u32, // ActorEvent + event_off: u32, event_len: u32, + key_off: u32, + key_len: u32, + val_off: u32, + val_len: u32, ) -> Result<()> { - let raw = context.memory.try_slice(event_off, event_len)?; - context.kernel.emit_event(raw) + let raw_event = context.memory.try_slice(event_off, event_len)?; + 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) } diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index 739d448d5..1eb4bd456 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -19,6 +19,7 @@ lazy_static = { version = "1.4.0" } log = "0.4.19" thiserror = "1.0.40" fvm_ipld_encoding = { version = "0.4", path = "../ipld/encoding" } +byteorder = "1.4.3" [features] default = [] diff --git a/sdk/src/event.rs b/sdk/src/event.rs index cd26dfa1c..4c72e132d 100644 --- a/sdk/src/event.rs +++ b/sdk/src/event.rs @@ -1,12 +1,56 @@ // Copyright 2021-2023 Protocol Labs // SPDX-License-Identifier: Apache-2.0, MIT -use fvm_shared::event::ActorEvent; - use crate::{sys, SyscallResult}; +use byteorder::{BigEndian, ByteOrder}; +use fvm_shared::event::ActorEvent; pub fn emit_event(evt: &ActorEvent) -> SyscallResult<()> { - let encoded = fvm_ipld_encoding::to_vec(evt).expect("failed to marshal actor event"); - let entries = encoded.as_slice(); + // we manually serialize the ActorEvent (not using CBOR) into three byte arrays so + // we can accurately charge gas without needing to parse anything inside the FVM + const BYTES_PER_ENTRY: usize = 24; + + let mut total_key_len: usize = 0; + let mut total_val_len: usize = 0; + + let mut v = vec![0u8; evt.entries.len() * BYTES_PER_ENTRY]; + for i in 0..evt.entries.len() { + let e = &evt.entries[i]; + let offset = i * BYTES_PER_ENTRY; + + let view = &mut v[offset..offset + BYTES_PER_ENTRY]; + BigEndian::write_u64(&mut view[..8], e.flags.bits()); + BigEndian::write_u32(&mut view[8..12], e.key.len() as u32); + BigEndian::write_u64(&mut view[12..20], e.codec); + BigEndian::write_u32(&mut view[20..24], e.value.len() as u32); + + total_key_len += e.key.len(); + total_val_len += e.value.len(); + } + + let mut keys = vec![0u8; total_key_len]; + let mut offset: usize = 0; + for i in 0..evt.entries.len() { + let e = &evt.entries[i]; + keys[offset..offset + e.key.len()].copy_from_slice(e.key.as_bytes()); + offset += e.key.len(); + } + + let mut values = vec![0u8; total_val_len]; + let mut offset: usize = 0; + for i in 0..evt.entries.len() { + let e = &evt.entries[i]; + values[offset..offset + e.value.len()].copy_from_slice(e.value.as_slice()); + offset += e.value.len(); + } - unsafe { sys::event::emit_event(entries.as_ptr(), entries.len() as u32) } + unsafe { + sys::event::emit_event( + v.as_slice().as_ptr(), + v.as_slice().len() as u32, + keys.as_slice().as_ptr(), + keys.as_slice().len() as u32, + values.as_slice().as_ptr(), + values.as_slice().len() as u32, + ) + } } diff --git a/sdk/src/sys/event.rs b/sdk/src/sys/event.rs index 7e47c7646..99c6b7dca 100644 --- a/sdk/src/sys/event.rs +++ b/sdk/src/sys/event.rs @@ -11,15 +11,18 @@ super::fvm_syscalls! { /// Emits an actor event to be recorded in the receipt. /// - /// Expects a DAG-CBOR representation of the ActorEvent struct. - /// /// # Errors /// /// | Error | Reason | /// |---------------------|---------------------------------------------------------------------| /// | [`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_len: u32, + key_off: *const u8, + key_len: u32, + value_off: *const u8, + value_len: u32, ) -> Result<()>; } diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index 51a9bbc53..f3ffcf4ec 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -564,8 +564,8 @@ where C: CallManager>, K: Kernel, { - fn emit_event(&mut self, raw_evt: &[u8]) -> Result<()> { - self.0.emit_event(raw_evt) + 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) } } diff --git a/testing/integration/tests/events_test.rs b/testing/integration/tests/events_test.rs index 44ac03e1c..e800c80a8 100644 --- a/testing/integration/tests/events_test.rs +++ b/testing/integration/tests/events_test.rs @@ -36,7 +36,11 @@ fn events_test() { .execute_message(message.clone(), ApplyKind::Explicit, 100) .unwrap(); - assert_eq!(ExitCode::OK, res.msg_receipt.exit_code); + assert!( + res.msg_receipt.exit_code.is_success(), + "{:?}", + res.failure_info + ); let gas_used = res.msg_receipt.gas_used; diff --git a/testing/integration/tests/gasfuzz.rs b/testing/integration/tests/gasfuzz.rs index 3828b243d..a8b831358 100644 --- a/testing/integration/tests/gasfuzz.rs +++ b/testing/integration/tests/gasfuzz.rs @@ -88,7 +88,11 @@ fn gasfuzz_get_exec_trace() -> ExecutionTrace { .unwrap(); let create_res = testkit::fevm::create_contract(&mut tester, &mut account, &contract).unwrap(); - assert!(create_res.msg_receipt.exit_code.is_success()); + assert!( + create_res.msg_receipt.exit_code.is_success(), + "{:?}", + create_res.failure_info + ); let create_return: testkit::fevm::CreateReturn = create_res.msg_receipt.return_data.deserialize().unwrap(); 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 32d3705c3..9535cf4e3 100644 --- a/testing/test_actors/actors/fil-events-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-events-actor/src/actor.rs @@ -12,6 +12,8 @@ use fvm_shared::event::{Entry, Flags}; pub fn invoke(params: u32) -> u32 { sdk::initialize(); + //sdk::debug::log("I am here1".to_string()); + const EMIT_SEVERAL_OK: u64 = 2; const EMIT_MALFORMED: u64 = 3; const EMIT_SUBCALLS: u64 = 4; @@ -55,7 +57,15 @@ pub fn invoke(params: u32) -> u32 { serialized[1] = 0xff; assert!( - sdk::sys::event::emit_event(serialized.as_ptr(), serialized.len() as u32).is_err(), + sdk::sys::event::emit_event( + serialized.as_ptr(), + serialized.len() as u32, + 0 as *const u8, + 0, + 0 as *const u8, + 0, + ) + .is_err(), "expected failed syscall" ); },