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

Cross-contract calling: simple debugger #14678

Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
1361556
Provide basic breakpoints
pmikolajczyk41 Jul 28, 2023
4ce4d35
Rename to Observer
pmikolajczyk41 Jul 28, 2023
f1bbb0c
Rename feature. Single trait. Borrow-checker
pmikolajczyk41 Jul 31, 2023
1e8707d
: frame_system::Config
pmikolajczyk41 Jul 31, 2023
2c1dd10
Confused type name
pmikolajczyk41 Jul 31, 2023
76c2c8c
Minor bugs
pmikolajczyk41 Jul 31, 2023
c75d8d5
pub trait
pmikolajczyk41 Jul 31, 2023
7a89df0
No unnecessary cloning
pmikolajczyk41 Aug 1, 2023
39bff7d
Make node compile with all features
pmikolajczyk41 Aug 1, 2023
bd32939
Move everything debug-related to a single module
pmikolajczyk41 Aug 1, 2023
a6e6d6e
Add docs and implementation for ()
pmikolajczyk41 Aug 1, 2023
386c7ee
fmt
pmikolajczyk41 Aug 1, 2023
19e9da0
Make it feature-gated or for tests
pmikolajczyk41 Aug 1, 2023
9d28acc
Prepare testing kit
pmikolajczyk41 Aug 1, 2023
55962ea
Testcase
pmikolajczyk41 Aug 1, 2023
9d96ff4
Merge remote-tracking branch 'origin/master' into pmikolajczyk41/cont…
pmikolajczyk41 Aug 1, 2023
52b267d
Fmt
pmikolajczyk41 Aug 1, 2023
8387b60
Use feature in dev-deps
pmikolajczyk41 Aug 2, 2023
91cc170
Merge remote-tracking branch 'origin/master' into pmikolajczyk41/cont…
pmikolajczyk41 Aug 2, 2023
04780a7
?
pmikolajczyk41 Aug 2, 2023
725cfcc
feature propagation
pmikolajczyk41 Aug 2, 2023
232f12c
AAAA
pmikolajczyk41 Aug 2, 2023
e8ac68a
lol, that doesn't make much sense to me
pmikolajczyk41 Aug 2, 2023
d1a8027
Turn on
pmikolajczyk41 Aug 2, 2023
5a95151
clippy
pmikolajczyk41 Aug 2, 2023
539e6b6
Remove self dep
pmikolajczyk41 Aug 2, 2023
eb8f34c
fmt, feature-gating test
pmikolajczyk41 Aug 2, 2023
9dd8b33
Noop to trigger CI
pmikolajczyk41 Aug 3, 2023
3a3aa45
idk
pmikolajczyk41 Aug 3, 2023
cae093f
add feature to pipeline
pmikolajczyk41 Aug 3, 2023
17a25ba
Corrupt test to see if it is actually being run
pmikolajczyk41 Aug 3, 2023
a3f8461
Revert change
pmikolajczyk41 Aug 3, 2023
986e9e2
Doc for conf type
pmikolajczyk41 Aug 3, 2023
23b03b7
Review
pmikolajczyk41 Aug 3, 2023
5970e8a
Imports
pmikolajczyk41 Aug 3, 2023
19c74f9
...
pmikolajczyk41 Aug 3, 2023
b2aa12f
Merge remote-tracking branch 'origin/master' into pmikolajczyk41/cont…
pmikolajczyk41 Aug 4, 2023
6485e4d
Remove debug for kitchen-sink
pmikolajczyk41 Aug 4, 2023
01ef583
Move test
pmikolajczyk41 Aug 4, 2023
1eaabb1
Fix imports
pmikolajczyk41 Aug 4, 2023
2316414
I must have already tried this one...
pmikolajczyk41 Aug 4, 2023
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
2 changes: 1 addition & 1 deletion bin/node/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pallet-balances = { version = "4.0.0-dev", default-features = false, path = "../
pallet-bounties = { version = "4.0.0-dev", default-features = false, path = "../../../frame/bounties" }
pallet-child-bounties = { version = "4.0.0-dev", default-features = false, path = "../../../frame/child-bounties" }
pallet-collective = { version = "4.0.0-dev", default-features = false, path = "../../../frame/collective" }
pallet-contracts = { version = "4.0.0-dev", default-features = false, path = "../../../frame/contracts" }
pallet-contracts = { version = "4.0.0-dev", default-features = false, path = "../../../frame/contracts", features = ["unsafe-debug"] }
athei marked this conversation as resolved.
Show resolved Hide resolved
pallet-contracts-primitives = { version = "24.0.0", default-features = false, path = "../../../frame/contracts/primitives/" }
pallet-conviction-voting = { version = "4.0.0-dev", default-features = false, path = "../../../frame/conviction-voting" }
pallet-core-fellowship = { version = "4.0.0-dev", default-features = false, path = "../../../frame/core-fellowship" }
Expand Down
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,7 @@ impl pallet_contracts::Config for Runtime {
type Migrations = pallet_contracts::migration::codegen::BenchMigrations;
type MaxDelegateDependencies = ConstU32<32>;
type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
type Debug = ();
}

impl pallet_sudo::Config for Runtime {
Expand Down
3 changes: 2 additions & 1 deletion frame/contracts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -111,5 +111,6 @@ try-runtime = [
"pallet-proxy/try-runtime",
"pallet-timestamp/try-runtime",
"pallet-utility/try-runtime",
"sp-runtime/try-runtime"
"sp-runtime/try-runtime",
]
unsafe-debug = []
14 changes: 13 additions & 1 deletion frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#[cfg(feature = "unsafe-debug")]
use crate::unsafe_debug::ExecutionObserver;
use crate::{
gas::GasMeter,
storage::{self, DepositAccount, WriteOutcome},
Expand Down Expand Up @@ -343,7 +345,7 @@ pub trait Ext: sealing::Sealed {
}

/// Describes the different functions that can be exported by an [`Executable`].
#[derive(Clone, Copy, PartialEq)]
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pgherveou marked this conversation as resolved.
Show resolved Hide resolved
pub enum ExportedFunction {
/// The constructor function which is executed on deployment of a contract.
Constructor,
Expand Down Expand Up @@ -903,11 +905,21 @@ where
// Every non delegate call or instantiate also optionally transfers the balance.
self.initial_transfer()?;

#[cfg(feature = "unsafe-debug")]
let (code_hash, input_clone) = {
let code_hash = *executable.code_hash();
T::Debug::before_call(&code_hash, entry_point, &input_data);
(code_hash, input_data.clone())
};

// Call into the Wasm blob.
let output = executable
.execute(self, &entry_point, input_data)
.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;

#[cfg(feature = "unsafe-debug")]
T::Debug::after_call(&code_hash, entry_point, &input_clone, &output);
athei marked this conversation as resolved.
Show resolved Hide resolved

// Avoid useless work that would be reverted anyways.
if output.did_revert() {
return Ok(output)
Expand Down
7 changes: 6 additions & 1 deletion frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ mod wasm;

pub mod chain_extension;
pub mod migration;
#[cfg(feature = "unsafe-debug")]
athei marked this conversation as resolved.
Show resolved Hide resolved
pub mod unsafe_debug;
pub mod weights;

#[cfg(test)]
Expand Down Expand Up @@ -137,7 +139,7 @@ pub use weights::WeightInfo;

pub use crate::{
address::{AddressGenerator, DefaultAddressGenerator},
exec::Frame,
exec::{ExportedFunction, Frame},
pgherveou marked this conversation as resolved.
Show resolved Hide resolved
migration::{MigrateSequence, Migration, NoopMigration},
pallet::*,
schedule::{HostFnWeights, InstructionWeights, Limits, Schedule},
Expand Down Expand Up @@ -345,6 +347,9 @@ pub mod pallet {
/// type Migrations = (v10::Migration<Runtime>,);
/// ```
type Migrations: MigrateSequence;

#[cfg(feature = "unsafe-debug")]
type Debug: unsafe_debug::UnsafeDebug<Self>;
athei marked this conversation as resolved.
Show resolved Hide resolved
}

#[pallet::hooks]
Expand Down
144 changes: 144 additions & 0 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ use crate::{
DebugInfo, DefaultAddressGenerator, DeletionQueueCounter, Error, MigrationInProgress, Origin,
Pallet, PristineCode, Schedule,
};
#[cfg(feature = "unsafe-debug")]
use crate::{unsafe_debug::ExecutionObserver, ExportedFunction};
use assert_matches::assert_matches;
use codec::Encode;
use frame_support::{
Expand All @@ -49,6 +51,8 @@ use frame_support::{
};
use frame_system::{EventRecord, Phase};
use pallet_contracts_primitives::CodeUploadReturnValue;
#[cfg(feature = "unsafe-debug")]
use pallet_contracts_primitives::ExecReturnValue;
use pretty_assertions::{assert_eq, assert_ne};
use sp_core::ByteArray;
use sp_io::hashing::blake2_256;
Expand All @@ -58,6 +62,8 @@ use sp_runtime::{
traits::{BlakeTwo256, Convert, Hash, IdentityLookup},
AccountId32, BuildStorage, Perbill, TokenError,
};
#[cfg(feature = "unsafe-debug")]
use std::cell::RefCell;
use std::ops::Deref;

type Block = frame_system::mocking::MockBlock<Test>;
Expand Down Expand Up @@ -438,6 +444,52 @@ parameter_types! {
pub static UnstableInterface: bool = true;
}

#[cfg(feature = "unsafe-debug")]
pgherveou marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Clone, PartialEq, Eq, Debug)]
struct DebugFrame {
code_hash: CodeHash<Test>,
call: ExportedFunction,
input: Vec<u8>,
result: Option<Vec<u8>>,
}

#[cfg(feature = "unsafe-debug")]
thread_local! {
static DEBUG_EXECUTION_TRACE: RefCell<Vec<DebugFrame>> = RefCell::new(Vec::new());
}

#[cfg(feature = "unsafe-debug")]
pub struct TestDebugger;
#[cfg(feature = "unsafe-debug")]
impl ExecutionObserver<CodeHash<Test>> for TestDebugger {
fn before_call(code_hash: &CodeHash<Test>, entry_point: ExportedFunction, input_data: &[u8]) {
DEBUG_EXECUTION_TRACE.with(|d| {
d.borrow_mut().push(DebugFrame {
code_hash: code_hash.clone(),
call: entry_point,
input: input_data.to_vec(),
result: None,
})
});
}

fn after_call(
code_hash: &CodeHash<Test>,
entry_point: ExportedFunction,
input_data: &[u8],
output: &ExecReturnValue,
) {
DEBUG_EXECUTION_TRACE.with(|d| {
d.borrow_mut().push(DebugFrame {
code_hash: code_hash.clone(),
call: entry_point,
input: input_data.to_vec(),
result: Some(output.data.clone()),
})
});
}
}

impl Config for Test {
type Time = Timestamp;
type Randomness = Randomness;
Expand All @@ -462,6 +514,8 @@ impl Config for Test {
type Migrations = crate::migration::codegen::BenchMigrations;
type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
type MaxDelegateDependencies = MaxDelegateDependencies;
#[cfg(feature = "unsafe-debug")]
type Debug = TestDebugger;
}

pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);
Expand Down Expand Up @@ -5897,3 +5951,93 @@ fn root_cannot_instantiate() {
);
});
}

#[cfg(feature = "unsafe-debug")]
#[test]
fn unsafe_debugging_works() {
let (wasm_caller, code_hash_caller) = compile_module::<Test>("call").unwrap();
let (wasm_callee, code_hash_callee) = compile_module::<Test>("store_call").unwrap();

fn current_stack() -> Vec<DebugFrame> {
DEBUG_EXECUTION_TRACE.with(|stack| stack.borrow().clone())
}

fn deploy(wasm: Vec<u8>) -> AccountId32 {
Contracts::bare_instantiate(
ALICE,
0,
GAS_LIMIT,
None,
Code::Upload(wasm),
vec![],
vec![],
DebugInfo::Skip,
CollectEvents::Skip,
)
.result
.unwrap()
.account_id
}

fn constructor_frame(hash: CodeHash<Test>, after: bool) -> DebugFrame {
DebugFrame {
code_hash: hash,
call: ExportedFunction::Constructor,
input: vec![],
result: if after { Some(vec![]) } else { None },
}
}

fn call_frame(hash: CodeHash<Test>, args: Vec<u8>, after: bool) -> DebugFrame {
DebugFrame {
code_hash: hash,
call: ExportedFunction::Call,
input: args,
result: if after { Some(vec![]) } else { None },
}
}

use frame_support::traits::Currency;

ExtBuilder::default().existential_deposit(200).build().execute_with(|| {
let _ = Balances::deposit_creating(&ALICE, 1_000_000);

assert_eq!(current_stack(), vec![]);

let addr_caller = deploy(wasm_caller);
let addr_callee = deploy(wasm_callee);

assert_eq!(
current_stack(),
vec![
constructor_frame(code_hash_caller, false),
constructor_frame(code_hash_caller, true),
constructor_frame(code_hash_callee, false),
constructor_frame(code_hash_callee, true),
]
);

let main_args = (100u32, &addr_callee).encode();
let inner_args = (100u32).encode();

assert_ok!(Contracts::call(
RuntimeOrigin::signed(ALICE),
addr_caller,
0,
GAS_LIMIT,
None,
main_args.clone()
));

let stack_top = current_stack()[4..].to_vec();
assert_eq!(
stack_top,
vec![
call_frame(code_hash_caller, main_args.clone(), false),
call_frame(code_hash_callee, inner_args.clone(), false),
call_frame(code_hash_callee, inner_args, true),
call_frame(code_hash_caller, main_args, true),
]
);
});
}
44 changes: 44 additions & 0 deletions frame/contracts/src/unsafe_debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use crate::{exec::ExportedFunction, CodeHash};
use pallet_contracts_primitives::ExecReturnValue;

/// Umbrella trait for all interfaces that serves for debugging, but are not suitable for any
/// production or benchmarking use.
pub trait UnsafeDebug<T: frame_system::Config>: ExecutionObserver<CodeHash<T>> {}

impl<T: frame_system::Config, D> UnsafeDebug<T> for D where D: ExecutionObserver<CodeHash<T>> {}

/// Defines the interface between pallet contracts and the outside observer.
///
/// The intended use is the environment, where the observer holds directly the whole runtime
/// (externalities) and thus can react to the execution breakpoints synchronously.
///
/// This definitely *should not* be used in any production or benchmarking setting, since handling
/// callbacks might be arbitrarily expensive and thus significantly influence performance.
pub trait ExecutionObserver<CodeHash> {
/// Called just before the execution of a contract.
///
/// # Arguments
///
/// * `code_hash` - The code hash of the contract being called.
/// * `entry_point` - Describes whether the call is the constructor or a regular call.
/// * `input_data` - The raw input data of the call.
fn before_call(_code_hash: &CodeHash, _entry_point: ExportedFunction, _input_data: &[u8]) {}

/// Called just after the execution of a contract.
///
/// # Arguments
///
/// * `code_hash` - The code hash of the contract being called.
/// * `entry_point` - Describes whether the call was the constructor or a regular call.
/// * `input_data` - The raw input data of the call.
/// * `output` - The raw output of the call.
fn after_call(
_code_hash: &CodeHash,
_entry_point: ExportedFunction,
_input_data: &[u8],
_output: &ExecReturnValue,
) {
}
}

impl<CodeHash> ExecutionObserver<CodeHash> for () {}
2 changes: 1 addition & 1 deletion scripts/ci/gitlab/pipeline/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ test-linux-stable:
--locked
--release
--verbose
--features runtime-benchmarks,try-runtime,experimental
--features runtime-benchmarks,try-runtime,experimental,unsafe-debug
--manifest-path ./bin/node/cli/Cargo.toml
--partition count:${CI_NODE_INDEX}/${CI_NODE_TOTAL}
# we need to update cache only from one job
Expand Down