diff --git a/prdoc/pr_7414.prdoc b/prdoc/pr_7414.prdoc new file mode 100644 index 0000000000000000000000000000000000000000..41edc1fe7cb38214c14e48f474b2eec3bd0a3dcf --- /dev/null +++ b/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 diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index 50b14c2a9987f703fcd167f6c0555f1ea03c0210..366a378cfd112715b80cfcac27bb250ecc2a1a79 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/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); }); } diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 5312ad2db1b0ec772e79961a1372081e13a1c4d8..ad92e5fecee2b8d4542f676d6816725149643c71 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/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), diff --git a/substrate/frame/revive/uapi/src/lib.rs b/substrate/frame/revive/uapi/src/lib.rs index bbd647a7faed59b3d20e5f4f1161ac780a41882d..744a2f0bca5d194c1b78c9369b36aa57bbf3321f 100644 --- a/substrate/frame/revive/uapi/src/lib.rs +++ b/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.