Skip to content

Conversation

@xermicus
Copy link
Member

@xermicus xermicus commented Jan 31, 2025

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 catching this, which doesn't work if we are trapping the caller. For example, the change makes the following snippet work:

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

@xermicus xermicus added R0-no-crate-publish-required The change does not require any crates to be re-published. T7-smart_contracts This PR/Issue is related to smart contracts. labels Jan 31, 2025
@xermicus xermicus requested review from athei and pgherveou January 31, 2025 16:43
@xermicus
Copy link
Member Author

/cmd prdoc --audience runtime_dev --bump major

@athei
Copy link
Member

athei commented Feb 3, 2025

Nice catch. Thank you. I am wondering: The catch blocks of Solidty do also apply to reverts where some data is returned. Do we need to encode some error data here, too?

@xermicus
Copy link
Member Author

xermicus commented Feb 3, 2025

Good point. There should be no need for us to encode anything, because the call family opcodes return success (where as the create family returns the address). resolc directly translates (inverts) the call return value to the success variable and the error data is already correctly encoded in the return data buffer (done by the callee contract). However, there still could be mismatches where we trap instead of returning a non-zero code. When I implemented this I reckoned we don't have any mismatch, however it has been a while ago and we had some changes.

@athei
Copy link
Member

athei commented Feb 3, 2025

You are right we are good here. AFAIK the EVM never encodes anything into the return buffer. That is completely controlled by the callee contract. So for every error in a subcall that is not a revert the returndatasize should be 0.

Especially ContractTrapped is interesting here as it does not exist on Ethereum. Only reverts. But this would be the same as an invalid opcode on EVM which will also yield an empty return data.

@athei athei enabled auto-merge February 3, 2025 15:57
@athei athei added this pull request to the merge queue Feb 3, 2025
Merged via the queue into master with commit 274a781 Feb 3, 2025
204 of 208 checks passed
@athei athei deleted the cl/duplicate-contract branch February 3, 2025 16:32
Ank4n pushed a commit that referenced this pull request Feb 6, 2025
…te 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

R0-no-crate-publish-required The change does not require any crates to be re-published. T7-smart_contracts This PR/Issue is related to smart contracts.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants