Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove storage read #847

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Changed
- [#847](https://github.com/FuelLabs/fuel-vm/pull/847): Remove `contract_read`, and change `load_contract_code`, `code_copy` and `code_root` to explicitly load the contract code in a buffer. Also check for mismatches between contract size stored and actual size of contract in those functions.

### Fixed
- [860](https://github.com/FuelLabs/fuel-vm/pull/860): Fixed missing fuzzing coverage report in CI.

Expand Down
58 changes: 44 additions & 14 deletions fuel-vm/src/interpreter/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use fuel_storage::{
use fuel_tx::{
consts::BALANCE_ENTRY_SIZE,
BlobId,
Contract,
ContractIdExt,
DependentCost,
Receipt,
Expand Down Expand Up @@ -636,8 +637,12 @@ where
self.gas_cost,
charge_len,
)?;
let contract = super::contract::contract(self.storage, &contract_id)?;
let contract_bytes = contract.as_ref().as_ref();

let contract_buffer: Vec<u8> = load_contract_code_from_storage(
self.storage,
contract_len as usize,
&contract_id,
)?;

let new_sp = ssp.saturating_add(length);
self.memory.grow_stack(new_sp)?;
Expand All @@ -654,7 +659,7 @@ where
copy_from_slice_zero_fill(
self.memory,
owner,
contract_bytes,
&contract_buffer,
region_start,
contract_offset,
length,
Expand Down Expand Up @@ -870,6 +875,28 @@ where
}
}

fn load_contract_code_from_storage<S>(
storage: &S,
contract_len: usize,
contract_id: &ContractId,
) -> Result<Vec<u8>, RuntimeError<<S as InterpreterStorage>::DataError>>
where
S: InterpreterStorage,
{
let mut contract_buffer: Vec<u8> = alloc::vec![0u8; contract_len as usize];
storage
.read_contract(&contract_id, &mut contract_buffer)
.transpose()
.ok_or(PanicReason::ContractNotFound)?
.map_err(RuntimeError::Storage)?;

if contract_buffer.len() != contract_len as usize {
Err(PanicReason::ContractMismatch)?
} else {
Ok(contract_buffer)
}
}

struct BurnCtx<'vm, S> {
storage: &'vm mut S,
context: &'vm Context,
Expand Down Expand Up @@ -994,9 +1021,14 @@ where
self.memory.write(self.owner, dst_addr, length)?;
self.input_contracts.check(&contract_id)?;

let contract = super::contract::contract(self.storage, &contract_id)?;
let contract_bytes = contract.as_ref().as_ref();
let contract_len = contract_bytes.len();
let contract_len = contract_size(self.storage, &contract_id)?;

let mut contract_buffer = load_contract_code_from_storage(
self.storage,
contract_len as usize,
&contract_id,
)?;

let charge_len = core::cmp::max(contract_len as u64, length);
let profiler = ProfileGas {
pc: self.pc.as_ref(),
Expand All @@ -1016,7 +1048,7 @@ where
copy_from_slice_zero_fill(
self.memory,
self.owner,
contract.as_ref().as_ref(),
&contract_buffer,
dst_addr,
contract_offset,
length,
Expand Down Expand Up @@ -1112,13 +1144,11 @@ impl<'vm, S> CodeRootCtx<'vm, S> {
self.gas_cost,
len as u64,
)?;
let root = self
.storage
.storage_contract(&contract_id)
.transpose()
.ok_or(PanicReason::ContractNotFound)?
.map_err(RuntimeError::Storage)?
.root();

let buf: Vec<u8> =
load_contract_code_from_storage(self.storage, len as usize, &contract_id)?;

let root = Contract::root_from_code(buf);

self.memory.write_bytes(self.owner, a, *root)?;

Expand Down
16 changes: 0 additions & 16 deletions fuel-vm/src/interpreter/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ use fuel_asm::{
};
use fuel_storage::StorageSize;
use fuel_tx::{
Contract,
Output,
Receipt,
};
Expand All @@ -55,8 +54,6 @@ use fuel_types::{
ContractId,
};

use alloc::borrow::Cow;

impl<M, S, Tx, Ecal> Interpreter<M, S, Tx, Ecal>
where
M: Memory,
Expand Down Expand Up @@ -179,19 +176,6 @@ where
}
}

pub(crate) fn contract<'s, S>(
storage: &'s S,
contract: &ContractId,
) -> IoResult<Cow<'s, Contract>, S::DataError>
where
S: InterpreterStorage,
{
storage
.storage_contract(contract)
.map_err(RuntimeError::Storage)?
.ok_or_else(|| PanicReason::ContractNotFound.into())
}

struct ContractBalanceCtx<'vm, S> {
storage: &'vm S,
memory: &'vm mut MemoryInstance,
Expand Down
9 changes: 0 additions & 9 deletions fuel-vm/src/storage/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,6 @@ pub trait InterpreterStorage:
Ok(())
}

/// Fetch a previously inserted contract code from the chain state for a
/// given contract.
fn storage_contract(
&self,
id: &ContractId,
) -> Result<Option<Cow<'_, Contract>>, Self::DataError> {
StorageInspect::<ContractsRawCode>::get(self, id)
}

/// Fetch the size of a previously inserted contract code from the chain state for a
/// given contract.
fn storage_contract_size(
Expand Down
3 changes: 2 additions & 1 deletion fuel-vm/src/storage/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ impl StorageRead<ContractsRawCode> for MemoryStorage {
) -> Result<Option<usize>, Self::Error> {
Ok(self.memory.contracts.get(key).map(|c| {
let len = buf.len().min(c.as_ref().len());
buf.copy_from_slice(&c.as_ref()[..len]);
buf[..len].copy_from_slice(&c.as_ref()[..len]);
buf[len..].fill(0);
len
}))
}
Expand Down
Loading