Cross-contract calling: simple debugger

[pmikolajczyk41]

Description

Motivation

While full, true contract debugging is currently not achievable, we still can try to get some insights from an outside client to the contracts pallet's internals. Since it is the pallet that owns and maintains call stack, we can take an advantage of this direct access.

Intended usage

Target environment/client

Surely, when we work in a full-stack setting (i.e. including node layer), there's no point in introducing breakpoints. In the case of an RPC call, we could try returning some trace, similarly to a debug buffer. However, my intention is to achieve more interactive experience.

Suppose that we own/hold the whole runtime (embedded within externalities) and have direct (synchronous) access to it. Then, we can establish a simple callback-based communication with the contracts pallet. This is exactly the case when working with tools like https://github.com/Cardinal-Cryptography/drink, but also for pallet's unit tests, or even try-runtime suite.

Workflow

For now, we add just two simple breakpoints: just before and after an Executable object is invoked. Hence, we can debug complex multi-contract execution traces. Since the whole runtime is synchronous, naturally the callbacks will stop execution and progress when the handling is done.

Plan

In the near future we plan to add more such breakpoints, providing further information, but firstly we want to check the approach and define what things would be useful.

Checklist

• My PR includes a detailed description as outlined in the "Description" section above
• My PR follows the labeling requirements of this project (at minimum one label for each A, B, C and D required)
• I have made corresponding changes to the documentation (if applicable)
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• If this PR alters any external APIs or interfaces used by Polkadot, the corresponding Polkadot PR is ready as well as the corresponding Cumulus PR (optional)

[athei]

I think this is a good idea. One thing that is important to me is that all this debugging code doesn't clutter the production code too much. What I mean by that is that we don't end up with a lot of compile time features and associated types added to Config.

I expect that over time you come up with more things you want to add to this debugging interface. Therefore I think we should make sure that we always only have a single feature and associated type.

Concretely:

1. Rename feature to unsafe-debug so that it will enable/disable all debugging (except debug buffer which is OK for production use).

2. Make sure all the debugging traits are grouped by a single trait to we always only need a single associated type on the Config:

trait Debug: ExecutionObserver + ExecutionTrace + ... 
{
}

The downside is that all traits need to be implemented but we can just have sensible default implementations.

[pmikolajczyk41]

In order to provide some reasonable usecase, I prepared an example scenario of cross contract calling and debugged it using drink library (after necessary on-spot enrichments). The code is available on a branch: https://github.com/Cardinal-Cryptography/drink/tree/pallet-breakpoint.

In the example we have 3 simple contacts: a, b and c. In the scenario, we call contract a with some u32 value. Contract a forwards the argument to b, b forwards it to c and c returns a result of some arithmetic operation, which is then returned up the stack to the caller.

Note: since it was faster to use call builder pattern in the contracts, I also passed the addresses of b and c along the u32 argument (so that delegation is possible).

After running ./debugger_showcase.sh you should see:

Building contract c
Building contract b
Building contract a
Drinking

running 1 test

successes:

---- tests::test stdout ----
Contract with hash `0x118a…8b5e` has been called with data: 
    new
and returned:
    ()

Contract with hash `0x2b1e…9b14` has been called with data: 
    new
and returned:
    ()

Contract with hash `0x38fa…c6bc` has been called with data: 
    new
and returned:
    ()

Contract with hash `0x118a…8b5e` has been called with data: 
    call { arg: 7 }
and returned:
    Ok(22)

Contract with hash `0x2b1e…9b14` has been called with data: 
    call { account_c: 5HHtX3hsCmVUTzpQtQKTnNyAQLnVzgUNrefA1kYvFwPfYbMX, arg: 7 }
and returned:
    Ok(22)

Contract with hash `0x38fa…c6bc` has been called with data: 
    call { account_b: 5CjYz6jzGja6WnwTWgC11mMCSyscknKoAb3JpmuCQ3TfUs3m, account_c: 5HHtX3hsCmVUTzpQtQKTnNyAQLnVzgUNrefA1kYvFwPfYbMX, arg: 7 }
and returned:
    Ok(22)

The hardest part is to enable arbitrary handler. Why the hardest? Because the handler is invoked by the runtime. And since it is passed as an associated type to the pallet configuration, it must be fixed - we cannot have e.g. generic runtime. The way I chose to solve the problem (and actually there's the only one I know) is to outsource handler via runtime interface to a specially registered externalities extension. That way we can pass to the drink's Session any type that implements certain trait.

[pmikolajczyk41]

now I'm getting:

error[E0437]: type `Debug` is not a member of trait `pallet_contracts::Config`
    --> bin/node/runtime/src/lib.rs:1262:2
     |
1262 |     type Debug = ();
     |     ^^^^^^^^^^^^^^^^ not a member of trait `pallet_contracts::Config`

after adding the type declaration (previously it was complaining about not including this member). I guess that this kitchen-sink thing is compiled in multiple configurations, but I don't know how to get it right :(

at least test passes correctly (at least locally for me 😛 )

[pmikolajczyk41]

bot rebase

[paritytech-processbot]

Error: Response error (status 404 Not Found):

{"documentation_url":"https://docs.github.com/rest/orgs/members#check-organization-membership-for-a-user","message":"User does not exist or is not a member of the organization"}

[athei]

Can you move the tests for the debugging feature to its own sub module of tests? Then we don't have to sprinkle the conditional compilation that much. Basically having:

tests.rs
tests/unsafe_debug.rs

[athei]

We need approval from CI team then we can merge.

[pgherveou]

bot merge