Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Remove pre charging when reading values of fixed size
Browse files Browse the repository at this point in the history
  • Loading branch information
athei committed Jun 1, 2021
1 parent 4b655d2 commit 8e295bf
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 96 deletions.
21 changes: 12 additions & 9 deletions frame/contracts/src/chain_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use crate::{
wasm::{Runtime, RuntimeCosts},
};
use codec::Decode;
use frame_support::weights::Weight;
use frame_support::{weights::Weight, traits::MaxEncodedLen};
use sp_runtime::DispatchError;
use sp_std::{
marker::PhantomData,
Expand Down Expand Up @@ -300,18 +300,21 @@ where
Ok(())
}

/// Reads `in_len` from contract memory and scale decodes it.
/// Reads and decodes a type with a size fixed at compile time from contract memory.
///
/// This function is secure and recommended for all input types of fixed size
/// as long as the cost of reading the memory is included in the overall already charged
/// weight of the chain extension. This should usually be the case when fixed input types
/// are used. Non fixed size types (like everything using `Vec`) usually need to use
/// [`in_len()`](Self::in_len) in order to properly charge the necessary weight.
pub fn read_as<T: Decode>(&mut self) -> Result<T> {
self.inner.runtime.read_sandbox_memory_as(
self.inner.input_ptr,
self.inner.input_len,
)
/// are used.
pub fn read_as<T: Decode + MaxEncodedLen>(&mut self) -> Result<T> {
self.inner.runtime.read_sandbox_memory_as(self.inner.input_ptr)
}

/// Reads and decodes a type with a dynamic size from contract memory.
///
/// Make sure to include `len` in your weight calculations.
pub fn read_as_unbounded<T: Decode + MaxEncodedLen>(&mut self, len: u32) -> Result<T> {
self.inner.runtime.read_sandbox_memory_as_unbounded(self.inner.input_ptr, len)
}

/// The length of the input as passed in as `input_len`.
Expand Down
22 changes: 2 additions & 20 deletions frame/contracts/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ use sp_core::crypto::UncheckedFrom;
#[cfg(test)]
use std::{any::Any, fmt::Debug};

#[derive(Debug, PartialEq, Eq)]
pub struct ChargedAmount(Weight);

impl ChargedAmount {
pub fn amount(&self) -> Weight {
self.0
}
}

#[cfg(not(test))]
pub trait TestAuxiliaries {}
#[cfg(not(test))]
Expand Down Expand Up @@ -135,7 +126,7 @@ where
/// NOTE that amount is always consumed, i.e. if there is not enough gas
/// then the counter will be set to zero.
#[inline]
pub fn charge<Tok: Token<T>>(&mut self, token: Tok) -> Result<ChargedAmount, DispatchError> {
pub fn charge<Tok: Token<T>>(&mut self, token: Tok) -> Result<(), DispatchError> {
#[cfg(test)]
{
// Unconditionally add the token to the storage.
Expand All @@ -153,20 +144,11 @@ where
self.gas_left = new_value.unwrap_or_else(Zero::zero);

match new_value {
Some(_) => Ok(ChargedAmount(amount)),
Some(_) => Ok(()),
None => Err(Error::<T>::OutOfGas.into()),
}
}

/// Refund previously charged gas back to the gas meter.
///
/// This can be used if a gas worst case estimation must be charged before
/// performing a certain action. This way the difference can be refundend when
/// the worst case did not happen.
pub fn refund(&mut self, amount: ChargedAmount) {
self.gas_left = self.gas_left.saturating_add(amount.0).min(self.gas_limit)
}

/// Returns how much gas was used.
pub fn gas_spent(&self) -> Weight {
self.gas_limit - self.gas_left
Expand Down
15 changes: 4 additions & 11 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2003,25 +2003,18 @@ mod tests {
"#;

#[test]
fn contract_decode_failure() {
fn contract_decode_length_ignored() {
let mut mock_ext = MockExt::default();
let result = execute(
CODE_DECODE_FAILURE,
vec![],
&mut mock_ext,
);

assert_eq!(
result,
Err(ExecError {
error: Error::<Test>::DecodingFailed.into(),
origin: ErrorOrigin::Caller,
})
);
// AccountID implements `MaxEncodeLen` and therefore the supplied length is
// no longer needed nor used to determine how much is read from contract memory.
assert_ok!(result);
}



#[test]
#[cfg(feature = "unstable-interface")]
fn rent_params_work() {
Expand Down
Loading

0 comments on commit 8e295bf

Please sign in to comment.