[pallet-revive] do not trap the caller on instantiations with duplicate contracts (#7414)

This PR changes the behavior of `instantiate` when the resulting
contract address already exists (because the caller tried to instantiate
the same contract with the same salt multiple times): Instead of
trapping the caller, return an error code.

Solidity allows `catch`ing this, which doesn't work if we are trapping
the caller. For example, the change makes the following snippet work:

```Solidity
try new Foo{salt: hex"00"}() returns (Foo) {
    // Instantiation was successful (contract address was free and constructor did not revert)
} catch {
    // This branch is expected to be taken if the instantiation failed because of a duplicate salt
}
```

`revive` PR: paritytech/revive#188

---------

Signed-off-by: Cyrill Leutwiler <[email protected]>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: xermicus

prdoc/pr_7414.prdoc

@@ -0,0 +1,20 @@
+title: '[pallet-revive] do not trap the caller on instantiations with duplicate contracts'
+doc:
+- audience: Runtime Dev
+  description: |-
+    This PR changes the behavior of `instantiate` when the resulting contract address already exists (because the caller tried to instantiate the same contract with the same salt multiple times): Instead of trapping the caller, return an error code.
+
+    Solidity allows `catch`ing this, which doesn't work if we are trapping the caller. For example, the change makes the following snippet work:
+
+    ```Solidity
+    try new Foo{salt: hex"00"}() returns (Foo) {
+        // Instantiation was successful (contract address was free and constructor did not revert)
+    } catch {
+        // This branch is expected to be taken if the instantiation failed because of a duplicate salt
+    }
+    ```
+crates:
+- name: pallet-revive
+  bump: major
+- name: pallet-revive-uapi
+  bump: major

substrate/frame/revive/src/tests.rs

@@ -1614,6 +1614,18 @@ fn instantiate_return_code() {
 			.data(callee_hash.iter().chain(&2u32.to_le_bytes()).cloned().collect())
 			.build_and_unwrap_result();
 		assert_return_code!(result, RuntimeReturnCode::CalleeTrapped);
+
+		// Contract instantiation succeeds
+		let result = builder::bare_call(contract.addr)
+			.data(callee_hash.iter().chain(&0u32.to_le_bytes()).cloned().collect())
+			.build_and_unwrap_result();
+		assert_return_code!(result, 0);
+
+		// Contract instantiation fails because the same salt is being used again.
+		let result = builder::bare_call(contract.addr)
+			.data(callee_hash.iter().chain(&0u32.to_le_bytes()).cloned().collect())
+			.build_and_unwrap_result();
+		assert_return_code!(result, RuntimeReturnCode::DuplicateContractAddress);
 	});
 }
 

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

@@ -792,10 +792,12 @@ impl<'a, E: Ext, M: ?Sized + Memory<E::T>> Runtime<'a, E, M> {
 		let transfer_failed = Error::<E::T>::TransferFailed.into();
 		let out_of_gas = Error::<E::T>::OutOfGas.into();
 		let out_of_deposit = Error::<E::T>::StorageDepositLimitExhausted.into();
+		let duplicate_contract = Error::<E::T>::DuplicateContract.into();
 
 		// errors in the callee do not trap the caller
 		match (from.error, from.origin) {
 			(err, _) if err == transfer_failed => Ok(TransferFailed),
+			(err, _) if err == duplicate_contract => Ok(DuplicateContractAddress),
 			(err, Callee) if err == out_of_gas || err == out_of_deposit => Ok(OutOfResources),
 			(_, Callee) => Ok(CalleeTrapped),
 			(err, _) => Err(err),

substrate/frame/revive/uapi/src/lib.rs

@@ -98,6 +98,9 @@ define_error_codes! {
 	XcmExecutionFailed = 9,
 	/// The `xcm_send` call failed.
 	XcmSendFailed = 10,
+	/// Contract instantiation failed because the address already exists.
+	/// Occurs when instantiating the same contract with the same salt more than once.
+	DuplicateContractAddress = 11,
 }
 
 /// The raw return code returned by the host side.