Split ext_clear_storage out from ext_set_storage (#5103)

* split out ext_clear_storage from ext_set_storage contracts API

* update tests to adjust for the ext_set_storage changes

* adjust COMPLEXITY for the ext_set_storage API changes

* remove value_len == 0 constraint for ext_set_storage

* bump spec_version

* remove guarantee from COMPLEXITY of ext_clear_storage

Co-authored-by: Gavin Wood <gavin@parity.io>

substrate/bin/node/runtime/src/lib.rs

@@ -82,8 +82,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to 0. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 235,
+	spec_version: 236,
-	impl_version: 1,
+	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 };
 

substrate/frame/contracts/COMPLEXITY.md

@@ -261,10 +261,20 @@ Each external function invoked from a contract can involve some overhead.
 This function receives a `key` and `value` as arguments. It consists of the following steps:
 
 1. Reading the sandbox memory for `key` and `value` (see sandboxing memory get).
-2. Setting the storage by the given `key` with the given `value` (see `set_storage`).
+2. Setting the storage at the given `key` to the given `value` (see `set_storage`).
 
 **complexity**: Complexity is proportional to the size of the `value`. This function induces a DB write of size proportional to the `value` size (if flushed to the storage), so should be priced accordingly.
 
+## ext_clear_storage
+
+This function receives a `key` as argument. It consists of the following steps:
+
+1. Reading the sandbox memory for `key` (see sandboxing memory get).
+2. Clearing the storage at the given `key` (see `set_storage`).
+
+**complexity**: Complexity is constant. This function induces a DB write to clear the storage entry
+(upon being flushed to the storage) and should be priced accordingly.
+
 ## ext_get_storage
 
 This function receives a `key` as an argument. It consists of the following steps:

substrate/frame/contracts/src/tests.rs

@@ -770,7 +770,8 @@ fn run_out_of_gas() {
 const CODE_SET_RENT: &str = r#"
 (module
 	(import "env" "ext_dispatch_call" (func $ext_dispatch_call (param i32 i32)))
-	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32)))
+	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32)))
+	(import "env" "ext_clear_storage" (func $ext_clear_storage (param i32)))
 	(import "env" "ext_set_rent_allowance" (func $ext_set_rent_allowance (param i32 i32)))
 	(import "env" "ext_scratch_size" (func $ext_scratch_size (result i32)))
 	(import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32)))
@@ -779,7 +780,6 @@ const CODE_SET_RENT: &str = r#"
 	;; insert a value of 4 bytes into storage
 	(func $call_0
 		(call $ext_set_storage
-			(i32.const 1)
 			(i32.const 1)
 			(i32.const 0)
 			(i32.const 4)
@@ -788,11 +788,8 @@ const CODE_SET_RENT: &str = r#"
 
 	;; remove the value inserted by call_1
 	(func $call_1
-		(call $ext_set_storage
+		(call $ext_clear_storage
 			(i32.const 1)
-			(i32.const 0)
-			(i32.const 0)
-			(i32.const 0)
 		)
 	)
 
@@ -852,7 +849,6 @@ const CODE_SET_RENT: &str = r#"
 		)
 		(call $ext_set_storage
 			(i32.const 0)
-			(i32.const 1)
 			(i32.const 0)
 			(i32.const 4)
 		)
@@ -1327,7 +1323,7 @@ fn default_rent_allowance_on_instantiate() {
 
 const CODE_RESTORATION: &str = r#"
 (module
-	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32)))
+	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32)))
 	(import "env" "ext_restore_to" (func $ext_restore_to (param i32 i32 i32 i32 i32 i32 i32 i32)))
 	(import "env" "memory" (memory 1 1))
 
@@ -1352,7 +1348,6 @@ const CODE_RESTORATION: &str = r#"
 		;; Data to restore
 		(call $ext_set_storage
 			(i32.const 0)
-			(i32.const 1)
 			(i32.const 0)
 			(i32.const 4)
 		)
@@ -1360,7 +1355,6 @@ const CODE_RESTORATION: &str = r#"
 		;; ACL
 		(call $ext_set_storage
 			(i32.const 100)
-			(i32.const 1)
 			(i32.const 0)
 			(i32.const 4)
 		)
@@ -1377,8 +1371,8 @@ const CODE_RESTORATION: &str = r#"
 
 	;; Code hash of SET_RENT
 	(data (i32.const 264)
-		"\14\eb\65\3c\86\98\d6\b2\3d\8d\3c\4a\54\c6\c4\71"
+		"\c2\1c\41\10\a5\22\d8\59\1c\4c\77\35\dd\2d\bf\a1"
-		"\b9\fc\19\36\df\ca\a0\a1\f2\dc\ad\9d\e5\36\0b\25"
+		"\13\0b\50\93\76\9b\92\31\97\b7\c5\74\26\aa\38\2a"
 	)
 
 	;; Rent allowance
@@ -1624,7 +1618,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 const CODE_STORAGE_SIZE: &str = r#"
 (module
 	(import "env" "ext_get_storage" (func $ext_get_storage (param i32) (result i32)))
-	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32)))
+	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32)))
 	(import "env" "ext_scratch_size" (func $ext_scratch_size (result i32)))
 	(import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32)))
 	(import "env" "memory" (memory 16 16))
@@ -1657,7 +1651,6 @@ const CODE_STORAGE_SIZE: &str = r#"
 		;; place a garbage value in storage, the size of which is specified by the call input.
 		(call $ext_set_storage
 			(i32.const 0)		;; Pointer to storage key
-			(i32.const 1)		;; Value is not null
 			(i32.const 0)		;; Pointer to value
 			(i32.load (i32.const 32))	;; Size of value
 		)
@@ -2278,7 +2271,7 @@ const CODE_DESTROY_AND_TRANSFER: &str = r#"
 	(import "env" "ext_scratch_size" (func $ext_scratch_size (result i32)))
 	(import "env" "ext_scratch_read" (func $ext_scratch_read (param i32 i32 i32)))
 	(import "env" "ext_get_storage" (func $ext_get_storage (param i32) (result i32)))
-	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32)))
+	(import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32)))
 	(import "env" "ext_call" (func $ext_call (param i32 i32 i64 i32 i32 i32 i32) (result i32)))
 	(import "env" "ext_instantiate" (func $ext_instantiate (param i32 i32 i64 i32 i32 i32 i32) (result i32)))
 	(import "env" "memory" (memory 1 1))
@@ -2340,7 +2333,6 @@ const CODE_DESTROY_AND_TRANSFER: &str = r#"
 		;; Store the return address.
 		(call $ext_set_storage
 			(i32.const 16)	;; Pointer to the key
-			(i32.const 1)	;; Value is not null
 			(i32.const 80)	;; Pointer to the value
 			(i32.const 8)	;; Length of the value
 		)

substrate/frame/contracts/src/wasm/runtime.rs

@@ -348,30 +348,42 @@ define_env!(Env, <E: Ext>,
 		Ok(())
 	},
 
-	// Change the value at the given key in the storage or remove the entry.
+	// Set the value at the given key in the contract storage.
-	// The value length must not exceed the maximum defined by the Contracts module parameters.
 	//
-	// - key_ptr: pointer into the linear
+	// The value length must not exceed the maximum defined by the contracts module parameters.
-	//   memory where the location of the requested value is placed.
+	// Storing an empty value is disallowed.
-	// - value_non_null: if set to 0, then the entry
+	//
-	//   at the given location will be removed.
+	// # Parameters
-	// - value_ptr: pointer into the linear memory
+	//
-	//   where the value to set is placed. If `value_non_null` is set to 0, then this parameter is ignored.
+	// - `key_ptr`: pointer into the linear memory where the location to store the value is placed.
-	// - value_len: the length of the value. If `value_non_null` is set to 0, then this parameter is ignored.
+	// - `value_ptr`: pointer into the linear memory where the value to set is placed.
-	ext_set_storage(ctx, key_ptr: u32, value_non_null: u32, value_ptr: u32, value_len: u32) => {
+	// - `value_len`: the length of the value in bytes.
-		if value_non_null != 0 && ctx.ext.max_value_size() < value_len {
+	//
+	// # Errors
+	//
+	// - If value length exceeds the configured maximum value length of a storage entry.
+	// - Upon trying to set an empty storage entry (value length is 0).
+	ext_set_storage(ctx, key_ptr: u32, value_ptr: u32, value_len: u32) => {
+		if value_len > ctx.ext.max_value_size() {
+			// Bail out if value length exceeds the set maximum value size.
 			return Err(sp_sandbox::HostError);
 		}
 		let mut key: StorageKey = [0; 32];
 		read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?;
-		let value =
+		let value = Some(read_sandbox_memory(ctx, value_ptr, value_len)?);
-			if value_non_null != 0 {
-				Some(read_sandbox_memory(ctx, value_ptr, value_len)?)
-			} else {
-				None
-			};
 		ctx.ext.set_storage(key, value).map_err(|_| sp_sandbox::HostError)?;
+		Ok(())
+	},
 
+	// Clear the value at the given key in the contract storage.
+	//
+	// # Parameters
+	//
+	// - `key_ptr`: pointer into the linear memory where the location to clear the value is placed.
+	ext_clear_storage(ctx, key_ptr: u32) => {
+		let mut key: StorageKey = [0; 32];
+		read_sandbox_memory_into_buf(ctx, key_ptr, &mut key)?;
+		ctx.ext.set_storage(key, None).map_err(|_| sp_sandbox::HostError)?;
 		Ok(())
 	},