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

XVM MVP v2 #92

Merged
merged 14 commits into from
Nov 8, 2022
Merged

XVM MVP v2 #92

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
cc48aef
XVM interface refactoring
akru Sep 7, 2022
8451ee7
Switch precompiles & chain-extension to XVMv2
akru Sep 19, 2022
5340817
XVM gas handling prototype (#94)
akru Oct 25, 2022
902fcbf
Remove chain-extension-trait package (#109)
akru Oct 21, 2022
957f0e6
XVM types cargo deps fix
akru Oct 25, 2022
c2d862f
Build fixes
akru Oct 25, 2022
f8e19ed
Compilation fixes
akru Oct 25, 2022
2124931
Fix formatting
akru Oct 27, 2022
8483b6d
Merge remote-tracking branch 'origin/polkadot-v0.9.30' into feature/x…
akru Nov 2, 2022
649faa1
Fix formatting
akru Nov 2, 2022
e0a7ab9
Fix compilation issues
akru Nov 2, 2022
f88d719
Fix cargo fmt
akru Nov 2, 2022
72f8d69
Fix clippy errors
akru Nov 2, 2022
f1cbe67
Fix weight estimation
akru Nov 7, 2022
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
4 changes: 0 additions & 4 deletions chain-extensions/types/xvm/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,15 @@ description = "Types definitions for contracts using xvm chain-extension."

[dependencies]
codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false }
frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.30", default-features = false }
scale-info = { version = "2.1.0", default-features = false, features = ["derive"] }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.30", default-features = false }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.30", default-features = false }
sp-std = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.30", default-features = false }

[features]
default = ["std"]
std = [
"codec/std",
"frame-support/std",
"scale-info/std",
"sp-core/std",
"sp-runtime/std",
"sp-std/std",
]
5 changes: 2 additions & 3 deletions chain-extensions/types/xvm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#![cfg_attr(not(feature = "std"), no_std)]

use codec::{Decode, Encode};
use sp_runtime::{DispatchError, ModuleError};
use sp_std::vec::Vec; // TODO use ink_prelude::vec::Vec; ?
use sp_std::vec::Vec;

#[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, Debug)]
Expand Down Expand Up @@ -35,8 +36,6 @@ pub struct XvmCallArgs {
pub to: Vec<u8>,
/// Encoded call params
pub input: Vec<u8>,
/// Metadata for the encoded params
pub metadata: Vec<u8>,
}

pub const FRONTIER_VM_ID: u8 = 0x0F;
Expand Down
33 changes: 14 additions & 19 deletions chain-extensions/xvm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![cfg_attr(not(feature = "std"), no_std)]

use frame_support::weights::Weight;
use frame_system::RawOrigin;
use pallet_contracts::chain_extension::{
ChainExtension, Environment, Ext, InitState, RetVal, SysConfig, UncheckedFrom,
Expand Down Expand Up @@ -52,27 +51,19 @@ where

match func_id {
XvmFuncId::XvmCall => {
// TODO: correct weight calculation directly from pallet!
let weight = Weight::from_ref_time(1_000_000_000);
env.charge_weight(weight)?;
// We need to immediately charge for the worst case scenario. Gas equals Weight in pallet-contracts context.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused at first by this because it seems so familiar lol 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe gas charging should be rewritten for new weighs, including comments. Let’s do it in XVMv3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to say that the reason this code looked so familiar is that I was the author. In the PR that added weight handling. xD

But it was like 2 months ago so I've forgotten about it.

This handling should be fine - some stuff needs to be benchmarked but that won't change the current weights too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

let remaining_weight = env.ext().gas_meter().gas_left();
let charged_weight = env.charge_weight(remaining_weight)?;

// Prepare parameters
let caller = env.ext().caller().clone();

let XvmCallArgs {
vm_id,
to,
input,
metadata,
} = env.read_as_unbounded(env.in_len())?;
let XvmCallArgs { vm_id, to, input } = env.read_as_unbounded(env.in_len())?;

// TODO: rethink this? How do we get valid env in chain extension? Need to know which data to encode.
// gas limit, taking into account used gas so far?
let _origin_address = env.ext().address().clone();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can delete these 2 unused values.
_origin_address will only be used in a case of an enum where contract can chose to use caller() or address()
_value will be used only when calling payable function will work

let _value = env.ext().value_transferred();
let _gas_limit = env.ext().gas_meter().gas_left();
let xvm_context = XvmContext {
id: T::VmId::from(vm_id),
id: vm_id,
max_weight: remaining_weight,
env: None,
};

Expand All @@ -81,12 +72,16 @@ where
xvm_context,
to,
input,
metadata,
);

// TODO: We need to know how much of gas was spent in the other call and update the gas meter!
// let consumed_xvm_weight = ...;
// env.charge_weight(consumed_xvm_weight)?;
// Adjust the actual weight used by the call if needed.
let actual_weight = match call_result {
Ok(e) => e.actual_weight,
Err(e) => e.post_info.actual_weight,
};
if let Some(actual_weight) = actual_weight {
env.adjust_weight(charged_weight, actual_weight);
}

return match call_result {
Err(e) => {
Expand Down
27 changes: 16 additions & 11 deletions contracts/xvm/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,24 @@ repository = "https://github.com/AstarNetwork/astar-frame"
description = "Chain extension for X-VM calls"
PierreOssun marked this conversation as resolved.
Show resolved Hide resolved

[dependencies]
ink_env = { version = "3", default-features = false }
ink_lang = { version = "3", default-features = false }
ink_metadata = { version = "3", default-features = false, features = ["derive"], optional = true }
ink_prelude = { version = "3", default-features = false }
ink_primitives = { version = "3", default-features = false }
ink_storage = { version = "3", default-features = false }
ink_env = { version = "3.3", default-features = false }
ink_lang = { version = "3.3", default-features = false }
ink_metadata = { version = "3.3", default-features = false, features = [
"derive",
], optional = true }
ink_prelude = { version = "3.3", default-features = false }
ink_primitives = { version = "3.3", default-features = false }
ink_storage = { version = "3.3", default-features = false }

scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["derive"] }
scale-info = { version = "2", default-features = false, features = ["derive"], optional = true }

xvm-chain-extension-types = { path = "../../chain-extensions/types/xvm", default-features = false }
scale = { package = "parity-scale-codec", version = "3", default-features = false, features = [
"derive",
] }
scale-info = { version = "2", default-features = false, features = [
"derive",
], optional = true }

[lib]
name = "xvm_chain_extension_contract"
name = "xvm_example"
path = "src/lib.rs"
crate-type = [
# Used for normal contract Wasm blobs.
Expand All @@ -36,6 +40,7 @@ std = [
"ink_env/std",
"ink_storage/std",
"ink_primitives/std",
"ink_prelude/std",
"scale/std",
"scale-info/std",
]
Expand Down
25 changes: 19 additions & 6 deletions contracts/xvm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
#![cfg_attr(not(feature = "std"), no_std)]

use ink_env::{DefaultEnvironment, Environment};
use ink_lang as ink;
use xvm_chain_extension_types::XvmCallArgs;

use ink_env::{DefaultEnvironment, Environment};
use ink_prelude::vec::Vec;

#[derive(Clone, PartialEq, Eq, scale::Encode, scale::Decode, Debug)]
pub struct XvmCallArgs {
/// virtual machine identifier
pub vm_id: u8,
/// Call destination (e.g. address)
pub to: Vec<u8>,
/// Encoded call params
pub input: Vec<u8>,
}

pub const FRONTIER_VM_ID: u8 = 0x0F;
pub const PARITY_WASM_VM_ID: u8 = 0x1F;

#[ink::chain_extension]
pub trait XvmChainExtension {
Expand Down Expand Up @@ -53,7 +67,7 @@ impl Environment for CustomEnvironment {
///
/// This will give us access to the chain extension that we've defined.
#[ink::contract(env = crate::CustomEnvironment)]
mod xvm_chain_extension_contract {
mod xvm_example {

use super::*;
use scale::Encode;
Expand All @@ -72,10 +86,9 @@ mod xvm_chain_extension_contract {
self.env()
.extension()
.xvm_call(XvmCallArgs {
vm_id: xvm_chain_extension_types::FRONTIER_VM_ID,
to: address.encode(), // TODO: is this correct?
vm_id: FRONTIER_VM_ID,
to: address.encode(),
input: Default::default(),
metadata: Default::default(),
})
.map_err(|_| ExtensionError::XvmCallFailed)?;

Expand Down
60 changes: 45 additions & 15 deletions frame/collator-selection/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ fn cannot_register_candidate_if_too_many() {

// reset desired candidates:
<crate::DesiredCandidates<Test>>::put(1);
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
assert_ok!(CollatorSelection::register_as_candidate(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are unrelated files changed by the formatter? Are you using different settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it’s new astar-frame formatting settings

RuntimeOrigin::signed(4)
));

// but no more
assert_noop!(
Expand All @@ -132,7 +134,9 @@ fn cannot_unregister_candidate_if_too_few() {
new_test_ext().execute_with(|| {
// reset desired candidates:
<crate::DesiredCandidates<Test>>::put(1);
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(4)
));

// can not remove too few
assert_noop!(
Expand Down Expand Up @@ -170,7 +174,9 @@ fn cannot_register_as_candidate_if_keys_not_registered() {
fn cannot_register_dupe_candidate() {
new_test_ext().execute_with(|| {
// can add 3 as candidate
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));
let addition = CandidateInfo {
who: 3,
deposit: 10,
Expand All @@ -194,7 +200,9 @@ fn cannot_register_as_candidate_if_poor() {
assert_eq!(Balances::free_balance(&33), 0);

// works
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));

// poor
assert_noop!(
Expand All @@ -217,8 +225,12 @@ fn register_as_candidate_works() {
assert_eq!(Balances::free_balance(&3), 100);
assert_eq!(Balances::free_balance(&4), 100);

assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(4)
));

assert_eq!(Balances::free_balance(&3), 90);
assert_eq!(Balances::free_balance(&4), 90);
Expand All @@ -231,11 +243,15 @@ fn register_as_candidate_works() {
fn leave_intent() {
new_test_ext().execute_with(|| {
// register a candidate.
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));
assert_eq!(Balances::free_balance(3), 90);

// register too so can leave above min candidates
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(5)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(5)
));
assert_eq!(Balances::free_balance(5), 90);

// cannot leave if not candidate.
Expand All @@ -259,7 +275,9 @@ fn authorship_event_handler() {

// 4 is the default author.
assert_eq!(Balances::free_balance(4), 100);
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(4)
));
// triggers `note_author`
Authorship::on_initialize(1);

Expand Down Expand Up @@ -287,7 +305,9 @@ fn fees_edgecases() {
Balances::make_free_balance_be(&CollatorSelection::account_id(), 5);
// 4 is the default author.
assert_eq!(Balances::free_balance(4), 100);
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(4)
));
// triggers `note_author`
Authorship::on_initialize(1);

Expand Down Expand Up @@ -319,7 +339,9 @@ fn session_management_works() {
assert_eq!(SessionHandlerCollators::get(), vec![1, 2]);

// add a new collator
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));

// session won't see this.
assert_eq!(SessionHandlerCollators::get(), vec![1, 2]);
Expand Down Expand Up @@ -348,8 +370,12 @@ fn kick_and_slash_mechanism() {
// Define slash destination account
<crate::SlashDestination<Test>>::put(5);
// add a new collator
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(4)
));
initialize_to_block(10);
assert_eq!(CollatorSelection::candidates().len(), 2);
initialize_to_block(20);
Expand Down Expand Up @@ -377,8 +403,12 @@ fn kick_and_slash_mechanism() {
fn should_not_kick_mechanism_too_few() {
new_test_ext().execute_with(|| {
// add a new collator
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(5)));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(3)
));
assert_ok!(CollatorSelection::register_as_candidate(
RuntimeOrigin::signed(5)
));
initialize_to_block(10);
assert_eq!(CollatorSelection::candidates().len(), 2);
initialize_to_block(20);
Expand Down
12 changes: 6 additions & 6 deletions frame/dapps-staking/src/testing_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,9 @@ pub(crate) fn assert_withdraw_from_unregistered(
RuntimeOrigin::signed(staker.clone()),
contract_id.clone()
));
System::assert_last_event(mock::RuntimeEvent::DappsStaking(Event::WithdrawFromUnregistered(
staker,
contract_id.clone(),
staked_value,
)));
System::assert_last_event(mock::RuntimeEvent::DappsStaking(
Event::WithdrawFromUnregistered(staker, contract_id.clone(), staked_value),
));

let final_state = MemorySnapshot::all(current_era, contract_id, staker);

Expand Down Expand Up @@ -365,7 +363,9 @@ pub(crate) fn assert_withdraw_unbonded(staker: AccountId) {
let expected_unbond_amount = valid_info.sum();

// Ensure op is successful and event is emitted
assert_ok!(DappsStaking::withdraw_unbonded(RuntimeOrigin::signed(staker),));
assert_ok!(DappsStaking::withdraw_unbonded(RuntimeOrigin::signed(
staker
),));
System::assert_last_event(mock::RuntimeEvent::DappsStaking(Event::Withdrawn(
staker,
expected_unbond_amount,
Expand Down
Loading