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

Introduce ext_println to contract runtime #2239

Merged
merged 6 commits into from
Apr 11, 2019
Merged

Conversation

ascjones
Copy link
Contributor

@ascjones ascjones commented Apr 9, 2019

Rel: use-ink/ink#20

Introduces the ext_println function which can be imported by smart contracts and used for debugging.

There was some discussion with @pepyakin and @Robbepop about whether any debugging code should be allowed in the runtime at all, with the preferred long term solution being Diagnostic Runtimes.

However that is an epic task, and we agreed that in the interest of improving the smart contract debugging experience in the short term (in time for Sub0) we should introduce the simple ext_println.

This MUST be used on dev chains only. Contracts which import ext_println will be rejected when deployed to a chain with contract debugging disabled.

@ascjones ascjones added A0-please_review Pull request needs code review. M8-contracts labels Apr 9, 2019
@ascjones ascjones requested review from pepyakin and Robbepop April 9, 2019 16:44
@xlc
Copy link
Contributor

xlc commented Apr 10, 2019

Can we make it command line flag to and have --dev turn on this flag?
So we can use it on non dev chain.

@bkchr
Copy link
Member

bkchr commented Apr 10, 2019

@xlc this is already done. development_config_genesis will be used when you use --dev.

srml/contract/src/wasm/runtime.rs Show resolved Hide resolved
@xlc
Copy link
Contributor

xlc commented Apr 10, 2019

I mean can we add a flag like --contract-debug so I can enable this with json genesis. e.g. --chain=./genesis.json --contract-debug

This way how this currently implemented won't work with node-template unless you modify https://github.com/paritytech/substrate/blob/master/node-template/src/chain_spec.rs as well.

@Robbepop Robbepop requested a review from shawntabrizi April 10, 2019 09:33
node/cli/src/chain_spec.rs Outdated Show resolved Hide resolved
srml/contract/src/wasm/runtime.rs Show resolved Hide resolved
Copy link
Contributor

@pepyakin pepyakin left a comment

Choose a reason for hiding this comment

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

Looks good modulo docs!

@ascjones ascjones added A8-looksgood and removed A0-please_review Pull request needs code review. labels Apr 11, 2019
@pepyakin pepyakin merged commit b5b5c32 into master Apr 11, 2019
@pepyakin pepyakin deleted the aj-contract-ext-println branch April 11, 2019 13:49
ascjones added a commit that referenced this pull request Apr 11, 2019
* Implement `ext_println` in contract runtime

* Only allow contracts to import `ext_println` on dev chains

* Configure dev chain to allow contracts with `ext_println`

* Increment spec version

* Docs

* Rename config to the more specific enable_println
bkchr pushed a commit that referenced this pull request Apr 11, 2019
* Implement `ext_println` in contract runtime

* Only allow contracts to import `ext_println` on dev chains

* Configure dev chain to allow contracts with `ext_println`

* Increment spec version

* Docs

* Rename config to the more specific enable_println
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Apr 12, 2019
* Implement `ext_println` in contract runtime

* Only allow contracts to import `ext_println` on dev chains

* Configure dev chain to allow contracts with `ext_println`

* Increment spec version

* Docs

* Rename config to the more specific enable_println
cmichi pushed a commit that referenced this pull request Apr 13, 2019
* Implement `ext_println` in contract runtime

* Only allow contracts to import `ext_println` on dev chains

* Configure dev chain to allow contracts with `ext_println`

* Increment spec version

* Docs

* Rename config to the more specific enable_println
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants