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

refactor: constrain environment associated type bounds to ease mocking #265

Merged
merged 6 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 30 additions & 23 deletions extension/src/decoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,7 @@ pub trait Decode {
///
/// # Parameters
/// - `env` - The current execution environment.
fn decode<E: Environment + BufIn>(env: &mut E) -> Result<Self::Output> {
// Charge appropriate weight for copying from contract, based on input length, prior to
// decoding. reference: https://github.com/paritytech/polkadot-sdk/pull/4233/files#:~:text=CopyToContract(len)%20%3D%3E%20T%3A%3AWeightInfo%3A%3Aseal_return(len)%2C
let len = env.in_len();
let weight = ContractWeights::<E::Config>::seal_return(len);
let charged = env.charge_weight(weight)?;
log::debug!(target: Self::LOG_TARGET, "pre-decode weight charged: len={len}, weight={weight}, charged={charged:?}");
// Read encoded input supplied by contract for buffer.
let mut input = env.read(len)?;
log::debug!(target: Self::LOG_TARGET, "input read: input={input:?}");
// Perform any additional processing required. Any implementation is expected to charge
// weight as appropriate.
input = Self::Processor::process(input, env);
// Finally decode and return.
Self::Output::decode(&mut &input[..]).map_err(|_| {
log::error!(target: Self::LOG_TARGET, "decoding failed: unable to decode input into output type. input={input:?}");
Self::Error::get()
})
}
fn decode<E: Environment + BufIn>(env: &mut E) -> Result<Self::Output>;
}

/// Trait for processing a value based on additional information available from the environment.
Expand Down Expand Up @@ -68,18 +50,43 @@ impl<Value> Processor for Identity<Value> {
}

/// Default implementation for decoding data read from contract memory.
pub struct Decodes<O, E, P = Identity<Vec<u8>>, L = ()>(PhantomData<(O, E, P, L)>);
pub struct Decodes<O, W, E, P = Identity<Vec<u8>>, L = ()>(PhantomData<(O, W, E, P, L)>);
impl<
Output: codec::Decode,
Weight: WeightInfo,
Error: Get<DispatchError>,
ValueProcessor: Processor<Value = Vec<u8>>,
Logger: LogTarget,
> Decode for Decodes<Output, Error, ValueProcessor, Logger>
> Decode for Decodes<Output, Weight, Error, ValueProcessor, Logger>
{
type Output = Output;
type Processor = ValueProcessor;
type Error = Error;
const LOG_TARGET: &'static str = Logger::LOG_TARGET;

/// Decodes data read from contract memory.
///
/// # Parameters
/// - `env` - The current execution environment.
fn decode<E: Environment + BufIn>(env: &mut E) -> Result<Self::Output> {
// Charge appropriate weight for copying from contract, based on input length, prior to
// decoding. reference: https://github.com/paritytech/polkadot-sdk/pull/4233/files#:~:text=CopyToContract(len)%20%3D%3E%20T%3A%3AWeightInfo%3A%3Aseal_return(len)%2C
let len = env.in_len();
let weight = Weight::seal_return(len);
let charged = env.charge_weight(weight)?;
log::debug!(target: Self::LOG_TARGET, "pre-decode weight charged: len={len}, weight={weight}, charged={charged:?}");
// Read encoded input supplied by contract for buffer.
let mut input = env.read(len)?;
log::debug!(target: Self::LOG_TARGET, "input read: input={input:?}");
// Perform any additional processing required. Any implementation is expected to charge
// weight as appropriate.
input = Self::Processor::process(input, env);
// Finally decode and return.
Output::decode(&mut &input[..]).map_err(|_| {
log::error!(target: Self::LOG_TARGET, "decoding failed: unable to decode input into output type. input={input:?}");
Error::get()
})
}
}

/// Error to be returned when decoding fails.
Expand All @@ -100,7 +107,7 @@ mod tests {
use codec::{Decode as OriginalDecode, Encode};
use frame_support::assert_ok;

type EnumDecodes = Decodes<ComprehensiveEnum, DecodingFailed<Test>>;
type EnumDecodes = Decodes<ComprehensiveEnum, ContractWeightsOf<Test>, DecodingFailed<Test>>;

#[test]
fn identity_processor_works() {
Expand Down Expand Up @@ -162,7 +169,7 @@ mod tests {
let mut env = MockEnvironment::new(0, input.clone());
assert!(EnumDecodes::decode(&mut env).is_err());
// Decode charges weight based on the length of the input, also when decoding fails.
assert_eq!(env.charged(), ContractWeights::<Test>::seal_return(input.len() as u32));
assert_eq!(env.charged(), ContractWeightsOf::<Test>::seal_return(input.len() as u32));
}

#[derive(Debug, Clone, PartialEq, Encode, OriginalDecode)]
Expand Down
41 changes: 26 additions & 15 deletions extension/src/environment.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
use crate::AccountIdOf;
use core::fmt::Debug;
use frame_support::pallet_prelude::Weight;
use pallet_contracts::{
chain_extension::{BufInBufOutState, ChargedAmount, Result, State},
Config,
};
use pallet_contracts::chain_extension::{BufInBufOutState, ChargedAmount, Result, State};
use sp_std::vec::Vec;

/// Provides access to the parameters passed to a chain extension and its execution environment.
///
/// A wrapper trait for `pallet_contracts::chain_extension::Environment`. All comments have been
/// copied solely for consistent developer experience in line with the wrapped type.
pub trait Environment {
/// The configuration of the contracts module.
type Config: Config;
/// The charged weight.
/// The account identifier type for the runtime.
type AccountId;
/// The charged weight type.
type ChargedAmount: Debug;

/// The function id within the `id` passed by a contract.
Expand Down Expand Up @@ -52,7 +50,7 @@ pub trait Environment {
///
/// Consult the functions on the returned type before re-implementing those functions.
// TODO: improve the return type to &mut
fn ext(&mut self) -> impl Ext<Config = Self::Config>;
fn ext(&mut self) -> impl Ext<AccountId = Self::AccountId>;
}

/// A wrapper type for `pallet_contracts::chain_extension::Environment`.
Expand All @@ -63,7 +61,7 @@ pub(crate) struct Env<'a, 'b, E: pallet_contracts::chain_extension::Ext, S: Stat
impl<'a, 'b, E: pallet_contracts::chain_extension::Ext, S: State> Environment
for Env<'a, 'b, E, S>
{
type Config = E::T;
type AccountId = AccountIdOf<E::T>;
type ChargedAmount = ChargedAmount;

fn func_id(&self) -> u16 {
Expand All @@ -82,7 +80,7 @@ impl<'a, 'b, E: pallet_contracts::chain_extension::Ext, S: State> Environment
self.0.adjust_weight(charged, actual_weight)
}

fn ext(&mut self) -> impl Ext<Config = E::T> {
fn ext(&mut self) -> impl Ext<AccountId = Self::AccountId> {
ExternalEnvironment(self.0.ext())
}
}
Expand Down Expand Up @@ -164,19 +162,32 @@ impl<'a, 'b, E: pallet_contracts::chain_extension::Ext> BufOut
/// A wrapper trait for `pallet_contracts::chain_extension::Ext`. All comments have been copied
/// solely for consistent developer experience in line with the wrapped type.
pub trait Ext {
/// The configuration of the contracts module.
type Config: Config;
/// The account identifier type for the runtime.
type AccountId;

/// Returns a reference to the account id of the current contract.
fn address(&self) -> &<Self::Config as frame_system::Config>::AccountId;
fn address(&self) -> &Self::AccountId;
}

impl Ext for () {
type AccountId = ();

fn address(&self) -> &Self::AccountId {
&()
}
}

/// A wrapper type for a type implementing `pallet_contracts::chain_extension::Ext`.
pub(crate) struct ExternalEnvironment<'a, T: pallet_contracts::chain_extension::Ext>(&'a mut T);

impl<'a, T: pallet_contracts::chain_extension::Ext> Ext for ExternalEnvironment<'a, T> {
type Config = T::T;
fn address(&self) -> &<Self::Config as frame_system::Config>::AccountId {
type AccountId = AccountIdOf<T::T>;
evilrobot-01 marked this conversation as resolved.
Show resolved Hide resolved
fn address(&self) -> &Self::AccountId {
self.0.address()
}
}

#[test]
fn default_ext_works() {
assert_eq!(().address(), &())
}
20 changes: 11 additions & 9 deletions extension/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/// # Parameters
/// - `env` - The current execution environment.
fn execute(
env: &mut (impl Environment<Config = Self::Config> + BufIn + BufOut),
env: &mut (impl Environment<AccountId = AccountIdOf<Self::Config>> + BufIn + BufOut),
) -> Result<RetVal>;
}

Expand All @@ -25,8 +25,8 @@
+ frame_system::Config<
RuntimeCall: GetDispatchInfo + Dispatchable<PostInfo = PostDispatchInfo>,
>,
Decoder: Decode<Output: codec::Decode + Into<<Config as frame_system::Config>::RuntimeCall>>,
Filter: Contains<<Config as frame_system::Config>::RuntimeCall> + 'static,
Decoder: Decode<Output: codec::Decode + Into<RuntimeCallOf<Config>>>,
Filter: Contains<RuntimeCallOf<Config>> + 'static,
Error: ErrorConverter,
Logger: LogTarget,
> Function for DispatchCall<Matcher, Config, Decoder, Filter, Error, Logger>
Expand All @@ -40,7 +40,9 @@
///
/// # Parameters
/// - `env` - The current execution environment.
fn execute(env: &mut (impl Environment<Config = Config> + BufIn)) -> Result<RetVal> {
fn execute(
env: &mut (impl Environment<AccountId = Config::AccountId> + BufIn),
) -> Result<RetVal> {
// Decode runtime call.
let call = Decoder::decode(env)?.into();
log::debug!(target: Logger::LOG_TARGET, "decoded: call={call:?}");
Expand Down Expand Up @@ -77,7 +79,7 @@

/// A function for reading runtime state.
pub struct ReadState<M, C, R, D, F, RC = DefaultConverter<<R as Readable>::Result>, E = (), L = ()>(
PhantomData<(M, C, R, D, F, RC, E, L)>,

Check warning on line 82 in extension/src/functions.rs

View workflow job for this annotation

GitHub Actions / clippy

very complex type used. Consider factoring parts into `type` definitions

warning: very complex type used. Consider factoring parts into `type` definitions --> extension/src/functions.rs:82:2 | 82 | PhantomData<(M, C, R, D, F, RC, E, L)>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity = note: `#[warn(clippy::type_complexity)]` on by default
);
impl<
Matcher: Matches,
Expand Down Expand Up @@ -116,7 +118,7 @@
let result = ResultConverter::convert(result, env).into();
log::debug!(target: Logger::LOG_TARGET, "converted: result={result:?}");
// Charge appropriate weight for writing to contract, based on result length.
let weight = ContractWeights::<Config>::seal_input(result.len() as u32);
let weight = ContractWeightsOf::<Config>::seal_input(result.len() as u32);
let charged = env.charge_weight(weight)?;
log::trace!(target: Logger::LOG_TARGET, "return result to contract: weight={weight}, charged={charged:?}");
env.write(&result, false, None)?; // weight charged above
Expand Down Expand Up @@ -202,7 +204,7 @@
type Error = ();

fn execute(
env: &mut (impl Environment<Config = Self::Config> + BufIn + BufOut),
env: &mut (impl Environment<AccountId = AccountIdOf<Self::Config>> + BufIn + BufOut),
) -> Result<RetVal> {
// Attempts to match a specified extension/function identifier to its corresponding
// function, as configured by the runtime.
Expand Down Expand Up @@ -280,7 +282,7 @@
type DispatchCallWithFilter<Filter> = super::DispatchCall<
WithFuncId<FuncId>,
Test,
Decodes<RuntimeCall, DecodingFailed<Test>>,
Decodes<RuntimeCall, ContractWeightsOf<Test>, DecodingFailed<Test>>,
Filter,
>;

Expand Down Expand Up @@ -398,14 +400,14 @@
WithFuncId<FuncId>,
Test,
RuntimeRead,
Decodes<RuntimeRead, DecodingFailed<Test>>,
Decodes<RuntimeRead, ContractWeightsOf<Test>, DecodingFailed<Test>>,
Filter,
>;
type ReadStateWithResultConverter<ResultConverter> = super::ReadState<
WithFuncId<FuncId>,
Test,
RuntimeRead,
Decodes<RuntimeRead, DecodingFailed<Test>>,
Decodes<RuntimeRead, ContractWeightsOf<Test>, DecodingFailed<Test>>,
Everything,
ResultConverter,
>;
Expand Down
15 changes: 8 additions & 7 deletions extension/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![cfg_attr(not(feature = "std"), no_std)]

use codec::Decode as _;
use core::marker::PhantomData;
pub use decoding::{Decode, Decodes, DecodingFailed, Identity, Processor};
pub use environment::{BufIn, BufOut, Environment, Ext};
Expand Down Expand Up @@ -34,7 +33,9 @@ mod mock;
#[cfg(test)]
mod tests;

type ContractWeights<T> = <T as pallet_contracts::Config>::WeightInfo;
type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
pub type ContractWeightsOf<T> = <T as pallet_contracts::Config>::WeightInfo;
type RuntimeCallOf<T> = <T as frame_system::Config>::RuntimeCall;

/// A configurable chain extension.
#[derive(Default)]
Expand Down Expand Up @@ -67,14 +68,14 @@ impl<
{
fn call(
&mut self,
env: &mut (impl Environment<Config = Runtime> + BufIn + BufOut),
env: &mut (impl Environment<AccountId = Runtime::AccountId> + BufIn + BufOut),
) -> Result<RetVal> {
log::trace!(target: Config::LOG_TARGET, "extension called");
// Charge weight for making a call from a contract to the runtime.
// `debug_message` weight is a good approximation of the additional overhead of going from
// contract layer to substrate layer. reference: https://github.com/paritytech/polkadot-sdk/pull/4233/files#:~:text=DebugMessage(len)%20%3D%3E%20T%3A%3AWeightInfo%3A%3Aseal_debug_message(len)%2C
let len = env.in_len();
let overhead = ContractWeights::<Runtime>::seal_debug_message(len);
let overhead = ContractWeightsOf::<Runtime>::seal_debug_message(len);
let charged = env.charge_weight(overhead)?;
log::debug!(target: Config::LOG_TARGET, "extension call weight charged: len={len}, weight={overhead}, charged={charged:?}");
// Execute the function
Expand Down Expand Up @@ -212,16 +213,16 @@ mod extension {

// Weight charged for calling into the runtime from a contract.
fn overhead_weight(input_len: u32) -> Weight {
ContractWeights::<Test>::seal_debug_message(input_len)
ContractWeightsOf::<Test>::seal_debug_message(input_len)
}

// Weight charged for reading function call input from buffer.
pub(crate) fn read_from_buffer_weight(input_len: u32) -> Weight {
ContractWeights::<Test>::seal_return(input_len)
ContractWeightsOf::<Test>::seal_return(input_len)
}

// Weight charged for writing to contract memory.
pub(crate) fn write_to_contract_weight(len: u32) -> Weight {
ContractWeights::<Test>::seal_input(len)
ContractWeightsOf::<Test>::seal_input(len)
}
}
Loading
Loading