contracts: Move Schedule from Storage to Config (paritytech#8773)

* Move `Schedule` from Storage to Config

* Updated CHANGELOG

* Fix nits from review

* Fix migration

* Print the debug buffer as tracing message

* Use `debug` instead of `trace` and update README

* Add additional assert to test

* Rename `schedule_version` to `instruction_weights_version`

* Fixed typo

* Added more comments to wat fixtures

* Add clarification for the `debug_message` field

bin/node/cli/src/chain_spec.rs

@@ -22,7 +22,7 @@ use sc_chain_spec::ChainSpecExtension;
 use sp_core::{Pair, Public, crypto::UncheckedInto, sr25519};
 use serde::{Serialize, Deserialize};
 use node_runtime::{
-	AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, ContractsConfig, CouncilConfig,
+	AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, CouncilConfig,
 	DemocracyConfig, GrandpaConfig, ImOnlineConfig, SessionConfig, SessionKeys, StakerStatus,
 	StakingConfig, ElectionsConfig, IndicesConfig, SocietyConfig, SudoConfig, SystemConfig,
 	TechnicalCommitteeConfig, wasm_binary_unwrap, MAX_NOMINATIONS,
@@ -146,7 +146,7 @@ fn staging_testnet_config_genesis() -> GenesisConfig {
 
 	let endowed_accounts: Vec<AccountId> = vec![root_key.clone()];
 
-	testnet_genesis(initial_authorities, vec![], root_key, Some(endowed_accounts), false)
+	testnet_genesis(initial_authorities, vec![], root_key, Some(endowed_accounts))
 }
 
 /// Staging testnet config.
@@ -212,7 +212,6 @@ pub fn testnet_genesis(
 	initial_nominators: Vec<AccountId>,
 	root_key: AccountId,
 	endowed_accounts: Option<Vec<AccountId>>,
-	enable_println: bool,
 ) -> GenesisConfig {
 	let mut endowed_accounts: Vec<AccountId> = endowed_accounts.unwrap_or_else(|| {
 		vec![
@@ -308,11 +307,6 @@ pub fn testnet_genesis(
 						.collect(),
 			phantom: Default::default(),
 		},
-		pallet_contracts: ContractsConfig {
-			// println should only be enabled on development chains
-			current_schedule: pallet_contracts::Schedule::default()
-				.enable_println(enable_println),
-		},
 		pallet_sudo: SudoConfig {
 			key: root_key,
 		},
@@ -352,7 +346,6 @@ fn development_config_genesis() -> GenesisConfig {
 		vec![],
 		get_account_id_from_seed::<sr25519::Public>("Alice"),
 		None,
-		true,
 	)
 }
 
@@ -380,7 +373,6 @@ fn local_testnet_genesis() -> GenesisConfig {
 		vec![],
 		get_account_id_from_seed::<sr25519::Public>("Alice"),
 		None,
-		false,
 	)
 }
 
@@ -414,7 +406,6 @@ pub(crate) mod tests {
 			vec![],
 			get_account_id_from_seed::<sr25519::Public>("Alice"),
 			None,
-			false,
 		)
 	}
 

bin/node/runtime/src/lib.rs

@@ -793,7 +793,7 @@ parameter_types! {
 			<Runtime as pallet_contracts::Config>::WeightInfo::on_initialize_per_queue_item(1) -
 			<Runtime as pallet_contracts::Config>::WeightInfo::on_initialize_per_queue_item(0)
 		)) / 5) as u32;
-	pub MaxCodeSize: u32 = 128 * 1024;
+	pub Schedule: pallet_contracts::Schedule<Runtime> = Default::default();
 }
 
 impl pallet_contracts::Config for Runtime {
@@ -810,13 +810,12 @@ impl pallet_contracts::Config for Runtime {
 	type RentFraction = RentFraction;
 	type SurchargeReward = SurchargeReward;
 	type CallStack = [pallet_contracts::Frame<Self>; 31];
-	type MaxValueSize = MaxValueSize;
 	type WeightPrice = pallet_transaction_payment::Module<Self>;
 	type WeightInfo = pallet_contracts::weights::SubstrateWeight<Self>;
 	type ChainExtension = ();
 	type DeletionQueueDepth = DeletionQueueDepth;
 	type DeletionWeightLimit = DeletionWeightLimit;
-	type MaxCodeSize = MaxCodeSize;
+	type Schedule = Schedule;
 }
 
 impl pallet_sudo::Config for Runtime {
@@ -1114,7 +1113,7 @@ construct_runtime!(
 		TechnicalMembership: pallet_membership::<Instance1>::{Pallet, Call, Storage, Event<T>, Config<T>},
 		Grandpa: pallet_grandpa::{Pallet, Call, Storage, Config, Event, ValidateUnsigned},
 		Treasury: pallet_treasury::{Pallet, Call, Storage, Config, Event<T>},
-		Contracts: pallet_contracts::{Pallet, Call, Config<T>, Storage, Event<T>},
+		Contracts: pallet_contracts::{Pallet, Call, Storage, Event<T>},
 		Sudo: pallet_sudo::{Pallet, Call, Config<T>, Storage, Event<T>},
 		ImOnline: pallet_im_online::{Pallet, Call, Storage, Event<T>, ValidateUnsigned, Config<T>},
 		AuthorityDiscovery: pallet_authority_discovery::{Pallet, Call, Config},
@@ -1354,7 +1353,7 @@ impl_runtime_apis! {
 			gas_limit: u64,
 			input_data: Vec<u8>,
 		) -> pallet_contracts_primitives::ContractExecResult {
-			Contracts::bare_call(origin, dest, value, gas_limit, input_data)
+			Contracts::bare_call(origin, dest, value, gas_limit, input_data, true)
 		}
 
 		fn instantiate(
@@ -1366,7 +1365,7 @@ impl_runtime_apis! {
 			salt: Vec<u8>,
 		) -> pallet_contracts_primitives::ContractInstantiateResult<AccountId, BlockNumber>
 		{
-			Contracts::bare_instantiate(origin, endowment, gas_limit, code, data, salt, true)
+			Contracts::bare_instantiate(origin, endowment, gas_limit, code, data, salt, true, true)
 		}
 
 		fn get_storage(

bin/node/testing/src/genesis.rs

@@ -22,7 +22,7 @@ use crate::keyring::*;
 use sp_keyring::{Ed25519Keyring, Sr25519Keyring};
 use node_runtime::{
 	GenesisConfig, BalancesConfig, SessionConfig, StakingConfig, SystemConfig,
-	GrandpaConfig, IndicesConfig, ContractsConfig, SocietyConfig, wasm_binary_unwrap,
+	GrandpaConfig, IndicesConfig, SocietyConfig, wasm_binary_unwrap,
 	AccountId, StakerStatus, BabeConfig, BABE_GENESIS_EPOCH_CONFIG,
 };
 use node_runtime::constants::currency::*;
@@ -97,9 +97,6 @@ pub fn config_endowed(
 			invulnerables: vec![alice(), bob(), charlie()],
 			.. Default::default()
 		},
-		pallet_contracts: ContractsConfig {
-			current_schedule: Default::default(),
-		},
 		pallet_babe: BabeConfig {
 			authorities: vec![],
 			epoch_config: Some(BABE_GENESIS_EPOCH_CONFIG),

bin/utils/chain-spec-builder/src/main.rs

@@ -106,14 +106,11 @@ fn genesis_constructor(
 		.map(chain_spec::authority_keys_from_seed)
 		.collect::<Vec<_>>();
 
-	let enable_println = true;
-
 	chain_spec::testnet_genesis(
 		authorities,
 		nominator_accounts.to_vec(),
 		sudo_account.clone(),
 		Some(endowed_accounts.to_vec()),
-		enable_println,
 	)
 }
 

frame/contracts/CHANGELOG.md

@@ -20,7 +20,11 @@ In other words: Upgrading this pallet will not break pre-existing contracts.
 
 ### Added
 
+- Replaced `seal_println` with `seal_debug_message` which allows output to an RPC client.
+[1](https://github.com/paritytech/substrate/pull/8773)
+
 - Add new `instantiate` RPC that allows clients to dry-run contract instantiation.
+[1](https://github.com/paritytech/substrate/pull/8451)
 
 - Make storage and fields of `Schedule` private to the crate.
 [1](https://github.com/paritytech/substrate/pull/8359)

frame/contracts/README.md

@@ -59,17 +59,26 @@ writing WebAssembly based smart contracts in the Rust programming language.
 
 ## Debugging
 
-Contracts can emit messages to the node console when run on a development chain through the
-`seal_println` API. This is exposed in ink! via
+Contracts can emit messages to the client when called as RPC through the `seal_debug_message`
+API. This is exposed in ink! via
 [`ink_env::debug_println()`](https://docs.rs/ink_env/latest/ink_env/fn.debug_println.html).
 
-In order to see these messages the log level for the `runtime::contracts` target needs to be raised
-to at least the `info` level which is the default. However, those messages are easy to overlook
-because of the noise generated by block production. A good starting point for contract debugging
-could be:
+Those messages are gathered into an internal buffer and send to the RPC client.
+It is up the the individual client if and how those messages are presented to the user.
+
+This buffer is also printed as a debug message. In order to see these messages on the node
+console the log level for the `runtime::contracts` target needs to be raised to at least
+the `debug` level. However, those messages are easy to overlook because of the noise generated
+by block production. A good starting point for observing them on the console is:
 
 ```bash
-cargo run --release -- --dev --tmp -lerror,runtime::contracts
+cargo run --release -- --dev --tmp -lerror,runtime::contracts=debug
 ```
 
+This raises the log level of `runtime::contracts` to `debug` and all other targets
+to `error` in order to prevent them from spamming the console.
+
+`--dev`: Use a dev chain spec
+`--tmp`: Use temporary storage for chain data (the chain state is deleted on exit)
+
 License: Apache-2.0

frame/contracts/common/src/lib.rs

@@ -37,18 +37,22 @@ use serde::{Serialize, Deserialize};
 pub struct ContractResult<T> {
 	/// How much gas was consumed during execution.
 	pub gas_consumed: u64,
-	/// An optional debug message. This message is only non-empty when explicitly requested
-	/// by the code that calls into the contract.
+	/// An optional debug message. This message is only filled when explicitly requested
+	/// by the code that calls into the contract. Otherwise it is empty.
 	///
 	/// The contained bytes are valid UTF-8. This is not declared as `String` because
-	/// this type is not allowed within the runtime. A client should decode them in order
-	/// to present the message to its users.
+	/// this type is not allowed within the runtime.
+	///
+	/// Clients should not make any assumptions about the format of the buffer.
+	/// They should just display it as-is. It is **not** only a collection of log lines
+	/// provided by a contract but a formatted buffer with different sections.
 	///
 	/// # Note
 	///
 	/// The debug message is never generated during on-chain execution. It is reserved for
 	/// RPC calls.
-	pub debug_message: Bytes,
+	#[cfg_attr(feature = "std", serde(with = "as_string"))]
+	pub debug_message: Vec<u8>,
 	/// The execution result of the wasm code.
 	pub result: T,
 }
@@ -146,3 +150,19 @@ pub enum Code<Hash> {
 	/// The code hash of an on-chain wasm blob.
 	Existing(Hash),
 }
+
+#[cfg(feature = "std")]
+mod as_string {
+	use super::*;
+	use serde::{Serializer, Deserializer, ser::Error};
+
+	pub fn serialize<S: Serializer>(bytes: &Vec<u8>, serializer: S) -> Result<S::Ok, S::Error> {
+		std::str::from_utf8(bytes)
+			.map_err(|e| S::Error::custom(format!("Debug buffer contains invalid UTF8: {}", e)))?
+			.serialize(serializer)
+	}
+
+	pub fn deserialize<'de, D: Deserializer<'de>>(deserializer: D) -> Result<Vec<u8>, D::Error> {
+		Ok(String::deserialize(deserializer)?.into_bytes())
+	}
+}

frame/contracts/fixtures/caller_contract.wat

@@ -5,7 +5,6 @@
 	(import "seal0" "seal_instantiate" (func $seal_instantiate
 		(param i32 i32 i64 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32) (result i32)
 	))
-	(import "seal0" "seal_println" (func $seal_println (param i32 i32)))
 	(import "env" "memory" (memory 1 1))
 
 	(func $assert (param i32)

frame/contracts/fixtures/debug_message_invalid_utf8.wat

@@ -0,0 +1,18 @@
+;; Emit a "Hello World!" debug message
+(module
+	(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
+	(import "env" "memory" (memory 1 1))
+
+	(data (i32.const 0) "\fc")
+
+	(func (export "call")
+		(call $seal_debug_message
+			(i32.const 0)	;; Pointer to the text buffer
+			(i32.const 12)	;; The size of the buffer
+		)
+		;; the above call traps because we supplied invalid utf8
+		unreachable
+	)
+
+	(func (export "deploy"))
+)

frame/contracts/fixtures/debug_message_logging_disabled.wat

@@ -0,0 +1,28 @@
+;; Emit a "Hello World!" debug message but assume that logging is disabled.
+(module
+	(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
+	(import "env" "memory" (memory 1 1))
+
+	(data (i32.const 0) "Hello World!")
+
+	(func $assert_eq (param i32 i32)
+		(block $ok
+			(br_if $ok
+				(i32.eq (get_local 0) (get_local 1))
+			)
+			(unreachable)
+		)
+	)
+
+	(func (export "call")
+		(call $assert_eq
+			(call $seal_debug_message
+				(i32.const 0)	;; Pointer to the text buffer
+				(i32.const 12)	;; The size of the buffer
+			)
+			(i32.const 9)	;; LoggingDisabled return code
+		)
+	)
+
+	(func (export "deploy"))
+)

frame/contracts/fixtures/debug_message_works.wat

@@ -0,0 +1,28 @@
+;; Emit a "Hello World!" debug message
+(module
+	(import "seal0" "seal_debug_message" (func $seal_debug_message (param i32 i32) (result i32)))
+	(import "env" "memory" (memory 1 1))
+
+	(data (i32.const 0) "Hello World!")
+
+	(func $assert_eq (param i32 i32)
+		(block $ok
+			(br_if $ok
+				(i32.eq (get_local 0) (get_local 1))
+			)
+			(unreachable)
+		)
+	)
+
+	(func (export "call")
+		(call $assert_eq
+			(call $seal_debug_message
+				(i32.const 0)	;; Pointer to the text buffer
+				(i32.const 12)	;; The size of the buffer
+			)
+			(i32.const 0)	;; success return code
+		)
+	)
+
+	(func (export "deploy"))
+)

frame/contracts/src/benchmarking/code.rs

@@ -24,7 +24,7 @@
 //! we define this simple definition of a contract that can be passed to `create_code` that
 //! compiles it down into a `WasmModule` that can be used as a contract's code.
 
-use crate::{Config, CurrentSchedule};
+use crate::Config;
 use parity_wasm::elements::{
 	Instruction, Instructions, FuncBody, ValueType, BlockType, Section, CustomSection,
 };
@@ -33,6 +33,7 @@ use sp_core::crypto::UncheckedFrom;
 use sp_runtime::traits::Hash;
 use sp_sandbox::{EnvironmentDefinitionBuilder, Memory};
 use sp_std::{prelude::*, convert::TryFrom, borrow::ToOwned};
+use frame_support::traits::Get;
 
 /// Pass to `create_code` in order to create a compiled `WasmModule`.
 ///
@@ -223,7 +224,7 @@ where
 		if def.inject_stack_metering {
 			code = inject_limiter(
 				code,
-				<CurrentSchedule<T>>::get().limits.stack_height
+				T::Schedule::get().limits.stack_height
 			)
 			.unwrap();
 		}
@@ -503,5 +504,5 @@ where
 	T: Config,
 	T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
 {
-	<CurrentSchedule<T>>::get().limits.memory_pages
+	T::Schedule::get().limits.memory_pages
 }