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.

Sign up for GitHub

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

Jump to bottom

feat(pop-api): charge proper weights in chain-extension #23

Merged
merged 7 commits into from
Mar 7, 2024
Merged

feat(pop-api): charge proper weights in chain-extension #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7d73b31
test(pop-api): read_state chain extension test
evilrobot-01 Mar 6, 2024
3e984f5
refactor: use generic T for ParachainSystem pallet
peterwht Mar 6, 2024
3deaa6f
feat(pop-api): add proper weight handling to extension
peterwht Mar 7, 2024
d25fa7d
refactor: fix NFT contract and update redundant LOG_TARGET
peterwht Mar 7, 2024
59e4e99
style: cargo fmt
peterwht Mar 7, 2024
747ebcb
refactor: simplify return
evilrobot-01 Mar 7, 2024
6b99213
chore: update comments
peterwht Mar 7, 2024
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
1 change: 1 addition & 0 deletions contracts/pop-api-examples/nfts/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ impl From<nfts::Error> for ContractError {
}
}

#[ink::contract(env = pop_api::Environment)]
mod pop_api_extension_demo {
use super::ContractError;

Expand Down
6 changes: 3 additions & 3 deletions pop-api/primitives/src/storage_keys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use scale::{Decode, Encode};
use scale::{Decode, Encode, MaxEncodedLen};

#[derive(Encode, Decode, Debug)]
#[derive(Encode, Decode, Debug, MaxEncodedLen)]
pub enum RuntimeStateKeys {
ParachainSystem(ParachainSystemKeys),
}

#[derive(Encode, Decode, Debug)]
#[derive(Encode, Decode, Debug, MaxEncodedLen)]
pub enum ParachainSystemKeys {
LastRelayChainBlockNumber,
}
168 changes: 122 additions & 46 deletions runtime/src/extensions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
dispatch::{GetDispatchInfo, PostDispatchInfo, RawOrigin},
pallet_prelude::*,
};
use log;

Check warning on line 6 in runtime/src/extensions.rs

View workflow job for this annotation

GitHub Actions / clippy

this import is redundant 

warning: this import is redundant
 --> runtime/src/extensions.rs:6:1
  |
6 | use log;
  | ^^^^^^^^ help: remove it entirely
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
  = note: `#[warn(clippy::single_component_path_imports)]` on by default
use pallet_contracts::chain_extension::{
ChainExtension, Environment, Ext, InitState, RetVal, SysConfig,
};
Expand All @@ -13,6 +13,8 @@

const LOG_TARGET: &str = "pop-api::extension";

type ContractSchedule<T> = <T as pallet_contracts::Config>::Schedule;

#[derive(Default)]
pub struct PopApiExtension;

Expand Down Expand Up @@ -77,7 +79,7 @@
}
}

pub(crate) fn dispatch<T, E>(env: Environment<E, InitState>) -> Result<(), DispatchError>
fn dispatch<T, E>(env: Environment<E, InitState>) -> Result<(), DispatchError>
where
T: pallet_contracts::Config + frame_system::Config,
<T as SysConfig>::AccountId: UncheckedFrom<<T as SysConfig>::Hash> + AsRef<[u8]>,
Expand All @@ -87,77 +89,93 @@
+ From<frame_system::Call<T>>,
E: Ext<T = T>,
{
const LOG_TARGET: &str = "pop-api::extension::dispatch";
const LOG_PREFIX: &str = " dispatch |";

let mut env = env.buf_in_buf_out();

// charge max weight before reading contract memory
// TODO: causing "1010: block limits exhausted" error
// let weight_limit = env.ext().gas_meter().gas_left();
// let charged_weight = env.charge_weight(weight_limit)?;

// TODO: debug_message weight is a good approximation of the additional overhead of going
// from contract layer to substrate layer.
let contract_host_weight = ContractSchedule::<T>::get().host_fn_weights;

// input length
let len = env.in_len();
let call: <T as SysConfig>::RuntimeCall = env.read_as_unbounded(len)?;

// conservative weight estimate for deserializing the input. The actual weight is less and should utilize a custom benchmark
let base_weight: Weight = T::DbWeight::get().reads(len.into());
// calculate weight for reading bytes of `len`
// reference: https://github.com/paritytech/polkadot-sdk/blob/117a9433dac88d5ac00c058c9b39c511d47749d2/substrate/frame/contracts/src/wasm/runtime.rs#L267
let base_weight: Weight = contract_host_weight.return_per_byte.saturating_mul(len.into());

// debug_message weight is a good approximation of the additional overhead of going
// from contract layer to substrate layer.
// reference: https://github.com/paritytech/ink-examples/blob/b8d2caa52cf4691e0ddd7c919e4462311deb5ad0/psp22-extension/runtime/psp22-extension-example.rs#L236
let overhead = contract_host_weight.debug_message;

let charged_weight = env.charge_weight(base_weight.saturating_add(overhead))?;
log::debug!(target:LOG_TARGET, "{} charged weight: {:?}", LOG_PREFIX, charged_weight);

// weight for dispatching the call
let dispatch_weight = call.get_dispatch_info().weight;
// read the input as RuntimeCall
let call: <T as SysConfig>::RuntimeCall = env.read_as_unbounded(len)?;

// charge weight for the cost of the deserialization and the dispatch
let _ = env.charge_weight(base_weight.saturating_add(dispatch_weight))?;
let charged_dispatch_weight = env.charge_weight(call.get_dispatch_info().weight)?;

log::debug!(target:LOG_TARGET, " dispatch inputted RuntimeCall: {:?}", call);
log::debug!(target:LOG_TARGET, "{} inputted RuntimeCall: {:?}", LOG_PREFIX, call);

let sender = env.ext().caller();
evilrobot-01 marked this conversation as resolved.
Show resolved Hide resolved
let origin: T::RuntimeOrigin = RawOrigin::Signed(sender.account_id()?.clone()).into();

// TODO: uncomment once charged_weight is fixed
// let actual_weight = call.get_dispatch_info().weight;
// env.adjust_weight(charged_weight, actual_weight);
let result = call.dispatch(origin);
match result {
match call.dispatch(origin) {
Ok(info) => {
log::debug!(target:LOG_TARGET, "dispatch success, actual weight: {:?}", info.actual_weight);
log::debug!(target:LOG_TARGET, "{} success, actual weight: {:?}", LOG_PREFIX, info.actual_weight);

// refund weight if the actual weight is less than the charged weight
if let Some(actual_weight) = info.actual_weight {
env.adjust_weight(charged_dispatch_weight, actual_weight);
}

Ok(())
},
Err(err) => {
log::debug!(target:LOG_TARGET, "dispatch failed: error: {:?}", err.error);
return Err(err.error);
log::debug!(target:LOG_TARGET, "{} failed: error: {:?}", LOG_PREFIX, err.error);
Err(err.error)
},
}
Ok(())
}

pub(crate) fn read_state<T, E>(env: Environment<E, InitState>) -> Result<(), DispatchError>
fn read_state<T, E>(env: Environment<E, InitState>) -> Result<(), DispatchError>
where
T: pallet_contracts::Config + frame_system::Config,
T: pallet_contracts::Config + cumulus_pallet_parachain_system::Config + frame_system::Config,
E: Ext<T = T>,
{
const LOG_TARGET: &str = "pop-api::extension::read_state";
const LOG_PREFIX: &str = " read_state |";

let mut env = env.buf_in_buf_out();

// TODO: Substitute len u32 with pop_api::src::impls::pop_network::StringLimit.
// Move StringLimit to pop_api_primitives first.
let len: u32 = env.in_len();
let key: ParachainSystemKeys = env.read_as_unbounded(len)?;
// To be conservative, we charge the weight for reading the input bytes of a fixed-size type.
let base_weight: Weight = ContractSchedule::<T>::get()
.host_fn_weights
.return_per_byte
.saturating_mul(env.in_len().into());
let charged_weight = env.charge_weight(base_weight)?;

log::debug!(target:LOG_TARGET, "{} charged weight: {:?}", LOG_PREFIX, charged_weight);
evilrobot-01 marked this conversation as resolved.
Show resolved Hide resolved

let key: ParachainSystemKeys = env.read_as()?;

let result = match key {
ParachainSystemKeys::LastRelayChainBlockNumber => {
let relay_block_num: BlockNumber = crate::ParachainSystem::last_relay_block_number();
env.charge_weight(T::DbWeight::get().reads(1_u64))?;
let relay_block_num: BlockNumber =
cumulus_pallet_parachain_system::Pallet::<T>::last_relay_block_number();
log::debug!(
target:LOG_TARGET,
"last relay chain block number is: {:?}.", relay_block_num
"{} last relay chain block number is: {:?}.", LOG_PREFIX, relay_block_num
);
relay_block_num
},
}
.encode()
// Double-encode result for extension return type of bytes
.encode();
log::trace!(
target:LOG_TARGET,
"{} result: {:?}.", LOG_PREFIX, result
);
env.write(&result, false, None).map_err(|e| {
log::trace!(target: LOG_TARGET, "{:?}", e);
DispatchError::Other("unable to write results to contract memory")
Expand All @@ -174,14 +192,14 @@
use parachains_common::CollectionId;
pub use sp_runtime::{traits::Hash, AccountId32};

pub const DEBUG_OUTPUT: pallet_contracts::DebugInfo = pallet_contracts::DebugInfo::UnsafeDebug;
const DEBUG_OUTPUT: pallet_contracts::DebugInfo = pallet_contracts::DebugInfo::UnsafeDebug;

pub const ALICE: AccountId32 = AccountId32::new([1_u8; 32]);
pub const BOB: AccountId32 = AccountId32::new([2_u8; 32]);
pub const INITIAL_AMOUNT: u128 = 100_000 * UNIT;
pub const GAS_LIMIT: Weight = Weight::from_parts(100_000_000_000, 3 * 1024 * 1024);
const ALICE: AccountId32 = AccountId32::new([1_u8; 32]);
const BOB: AccountId32 = AccountId32::new([2_u8; 32]);
const INITIAL_AMOUNT: u128 = 100_000 * UNIT;
const GAS_LIMIT: Weight = Weight::from_parts(100_000_000_000, 3 * 1024 * 1024);

pub fn new_test_ext() -> sp_io::TestExternalities {
fn new_test_ext() -> sp_io::TestExternalities {
let mut t = frame_system::GenesisConfig::<Runtime>::default()
.build_storage()
.expect("Frame system builds valid default genesis config");
Expand All @@ -197,9 +215,7 @@
ext
}

pub fn load_wasm_module<T>(
path: &str,
) -> std::io::Result<(Vec<u8>, <T::Hashing as Hash>::Output)>
fn load_wasm_module<T>(path: &str) -> std::io::Result<(Vec<u8>, <T::Hashing as Hash>::Output)>
where
T: frame_system::Config,
{
Expand All @@ -208,7 +224,7 @@
Ok((wasm_binary, code_hash))
}

pub fn function_selector(name: &str) -> Vec<u8> {
fn function_selector(name: &str) -> Vec<u8> {
let hash = sp_io::hashing::blake2_256(name.as_bytes());
[hash[0..4].to_vec()].concat()
}
Expand Down Expand Up @@ -439,4 +455,64 @@
assert!(result.did_revert());
});
}

#[test]
#[ignore]
fn reading_last_relay_chain_block_number_works() {
new_test_ext().execute_with(|| {
let _ = env_logger::try_init();

let (wasm_binary, _) = load_wasm_module::<Runtime>("../contracts/pop-api-examples/read-runtime-state/target/ink/pop_api_extension_demo.wasm").unwrap();

let init_value = 100;

let contract = Contracts::bare_instantiate(
ALICE,
init_value,
GAS_LIMIT,
None,
Code::Upload(wasm_binary),
function_selector("new"),
vec![],
DEBUG_OUTPUT,
pallet_contracts::CollectEvents::Skip,
)
.result
.unwrap();

assert!(
!contract.result.did_revert(),
"deploying contract reverted {:?}",
contract
);

let addr = contract.account_id;

let function = function_selector("read_relay_block_number");
let params = [function].concat();

let result = Contracts::bare_call(
ALICE,
addr.clone(),
0,
Weight::from_parts(100_000_000_000, 3 * 1024 * 1024),
None,
params,
DEBUG_OUTPUT,
pallet_contracts::CollectEvents::UnsafeCollect,
pallet_contracts::Determinism::Relaxed,
);

if DEBUG_OUTPUT == pallet_contracts::DebugInfo::UnsafeDebug {
log::debug!(
"Contract debug buffer - {:?}",
String::from_utf8(result.debug_message.clone())
);
log::debug!("result: {:?}", result);
}

// check for revert
assert!(!result.result.unwrap().did_revert(), "Contract reverted!");
});
}
}
Loading