feat: refactor block_read to return the offset from the end

Previously, it would return the amount of data read. Unfortunately, this
made it impossible to tell if we were at the end of the block.

Now, we return the offset (negative or positive) from the end of the
block, to the end of the user provided buffer.

Author: Stebalien

fvm/src/kernel/default.rs

@@ -363,23 +363,23 @@ where
         Ok(k)
     }
 
-    fn block_read(&mut self, id: BlockId, offset: u32, buf: &mut [u8]) -> Result<u32> {
-        let data = self.blocks.get(id).or_illegal_argument()?.data();
+    fn block_read(&mut self, id: BlockId, offset: u32, buf: &mut [u8]) -> Result<i32> {
+        let block = self.blocks.get(id)?;
+        let data = block.data();
 
-        let len = if offset as usize >= data.len() {
-            0
-        } else {
-            buf.len().min(data.len())
-        };
+        let start = offset as usize;
 
+        let to_read = std::cmp::min(data.len().saturating_sub(start), buf.len());
         self.call_manager
-            .charge_gas(self.call_manager.price_list().on_block_read(len))?;
+            .charge_gas(self.call_manager.price_list().on_block_read(to_read))?;
 
-        if len != 0 {
-            buf.copy_from_slice(&data[offset as usize..][..len]);
+        let end = start + to_read;
+        if to_read != 0 {
+            buf[..to_read].copy_from_slice(&data[start..end]);
         }
 
-        Ok(len as u32)
+        // Returns the difference between the end of the block, and the end of the data we've read.
+        Ok((data.len() as i32) - (end as i32))
     }
 
     fn block_stat(&mut self, id: BlockId) -> Result<BlockStat> {

fvm/src/kernel/mod.rs

@@ -116,7 +116,7 @@ pub trait BlockOps {
     /// Read data from a block.
     ///
     /// This method will fail if the block handle is invalid.
-    fn block_read(&mut self, id: BlockId, offset: u32, buf: &mut [u8]) -> Result<u32>;
+    fn block_read(&mut self, id: BlockId, offset: u32, buf: &mut [u8]) -> Result<i32>;
 
     /// Returns the blocks codec & size.
     ///
@@ -130,8 +130,8 @@ pub trait BlockOps {
         let stat = self.block_stat(id)?;
         let mut ret = vec![0; stat.size as usize];
         // TODO error handling.
-        let read = self.block_read(id, 0, &mut ret)?;
-        debug_assert_eq!(stat.size, read, "didn't read expected bytes");
+        let remaining = self.block_read(id, 0, &mut ret)?;
+        debug_assert_eq!(remaining, 0, "didn't read expected bytes");
         Ok((stat.codec, ret))
     }
 

fvm/src/syscalls/ipld.rs

@@ -48,7 +48,7 @@ pub fn read(
     offset: u32,
     obuf_off: u32,
     obuf_len: u32,
-) -> Result<u32> {
+) -> Result<i32> {
     let data = context.memory.try_slice_mut(obuf_off, obuf_len)?;
     context.kernel.block_read(id, offset, data)
 }

sdk/src/ipld.rs

@@ -32,24 +32,38 @@ pub fn get(cid: &Cid) -> SyscallResult<Vec<u8>> {
             .expect("CID encoding should not fail");
         let fvm_shared::sys::out::ipld::IpldOpen { id, size, .. } =
             sys::ipld::open(cid_buf.as_mut_ptr())?;
-        get_block(id, Some(size))
+        let mut block = Vec::with_capacity(size as usize);
+        let remaining = sys::ipld::read(id, 0, block.as_mut_ptr(), size)?;
+        debug_assert_eq!(remaining, 0, "expected to read the block exactly");
+        Ok(block)
     }
 }
 
-/// Gets the data of the block referenced by BlockId. If the caller knows the
-/// size, this function will avoid statting the block.
-pub fn get_block(id: fvm_shared::sys::BlockId, size: Option<u32>) -> SyscallResult<Vec<u8>> {
-    let size = match size {
-        Some(size) => size,
-        None => unsafe { sys::ipld::stat(id).map(|out| out.size)? },
-    };
-    let mut block = Vec::with_capacity(size as usize);
+/// Gets the data of the block referenced by BlockId. If the caller knows the size, this function
+/// will read the block in a single syscall. Otherwise, any block over 1KiB will take two syscalls.
+pub fn get_block(id: fvm_shared::sys::BlockId, size_hint: Option<u32>) -> SyscallResult<Vec<u8>> {
+    // Check for the "empty" block first.
+    if id == UNIT {
+        return Ok(Vec::new());
+    }
+
+    let mut buf = Vec::with_capacity(size_hint.unwrap_or(1024) as usize);
     unsafe {
-        let bytes_read = sys::ipld::read(id, 0, block.as_mut_ptr(), size)?;
-        debug_assert!(bytes_read == size, "read an unexpected number of bytes");
-        block.set_len(size as usize);
+        let mut remaining = sys::ipld::read(id, 0, buf.as_mut_ptr(), buf.capacity() as u32)?;
+        if remaining > 0 {
+            buf.set_len(buf.capacity());
+            buf.reserve_exact(remaining as usize);
+            remaining = sys::ipld::read(
+                id,
+                buf.len() as u32,
+                buf.as_mut_ptr_range().end,
+                (buf.capacity() - buf.len()) as u32,
+            )?;
+            debug_assert!(remaining <= 0, "should have read whole block");
+        }
+        buf.set_len(buf.capacity() + (remaining as usize));
     }
-    Ok(block)
+    Ok(buf)
 }
 
 /// Writes the supplied block and returns the BlockId.

sdk/src/message.rs

@@ -42,22 +42,6 @@ pub fn params_raw(id: BlockId) -> SyscallResult<(Codec, Vec<u8>)> {
     }
     unsafe {
         let fvm_shared::sys::out::ipld::IpldStat { codec, size } = sys::ipld::stat(id)?;
-        log::trace!(
-            "params_raw -> ipld stat: size={:?}; codec={:?}",
-            size,
-            codec
-        );
-
-        let mut buf: Vec<u8> = Vec::with_capacity(size as usize);
-        let ptr = buf.as_mut_ptr();
-        let bytes_read = sys::ipld::read(id, 0, ptr, size)?;
-        buf.set_len(bytes_read as usize);
-        log::trace!(
-            "params_raw -> ipld read: bytes_read={:?}, data: {:x?}",
-            bytes_read,
-            &buf
-        );
-        debug_assert!(bytes_read == size, "read an unexpected number of bytes");
-        Ok((codec, buf))
+        Ok((codec, crate::ipld::get_block(id, Some(size))?))
     }
 }

sdk/src/send.rs

@@ -48,13 +48,8 @@ pub fn send(
         let exit_code = ExitCode::new(exit_code);
         let return_data = match exit_code {
             ExitCode::OK if return_id != NO_DATA_BLOCK_ID => {
-                // Allocate a buffer to read the return data.
-                let fvm_shared::sys::out::ipld::IpldStat { size, .. } = sys::ipld::stat(return_id)?;
-                let mut bytes = vec![0; size as usize];
-
                 // Now read the return data.
-                let read = sys::ipld::read(return_id, 0, bytes.as_mut_ptr(), size)?;
-                assert_eq!(read, size);
+                let bytes = crate::ipld::get_block(return_id, None)?;
                 RawBytes::from(bytes)
             }
             _ => Default::default(),

sdk/src/sys/ipld.rs

@@ -54,7 +54,12 @@ super::fvm_syscalls! {
     /// Reads the block identified by `id` into `obuf`, starting at `offset`, reading _at most_
     /// `max_len` bytes.
     ///
-    /// Returns the number of bytes read.
+    /// Returns the difference between the length of the block and `offset + max_len`. This can be
+    /// used to find the end of the block relative to the buffer the block is being read into:
+    ///
+    /// - A zero return value means that the block was read into the output buffer exactly.
+    /// - A positive return value means that that many more bytes need to be read.
+    /// - A negative return value means that the buffer should be truncated by the return value.
     ///
     /// # Arguments
     ///
@@ -64,15 +69,16 @@ super::fvm_syscalls! {
     /// - `max_len` is the maximum amount of block data to read.
     ///
     /// Passing a length/offset that exceed the length of the block will not result in an error, but
-    /// will result in no data being read.
+    /// will result in no data being read and a negative return value indicating where the block
+    /// actually ended (relative to `offset + max_len`).
     ///
     /// # Errors
     ///
     /// | Error               | Reason                                            |
     /// |---------------------|---------------------------------------------------|
     /// | [`InvalidHandle`]   | if the handle isn't known.                        |
     /// | [`IllegalArgument`] | if the passed buffer isn't valid, in memory, etc. |
-    pub fn read(id: u32, offset: u32, obuf: *mut u8, max_len: u32) -> Result<u32>;
+    pub fn read(id: u32, offset: u32, obuf: *mut u8, max_len: u32) -> Result<i32>;
 
     /// Returns the codec and size of the specified block.
     ///

testing/conformance/src/vm.rs

@@ -362,7 +362,7 @@ where
         self.0.block_link(id, hash_fun, hash_len)
     }
 
-    fn block_read(&mut self, id: BlockId, offset: u32, buf: &mut [u8]) -> Result<u32> {
+    fn block_read(&mut self, id: BlockId, offset: u32, buf: &mut [u8]) -> Result<i32> {
         self.0.block_read(id, offset, buf)
     }