Skip to content

Commit

Permalink
Added Update on Correcting this Issue
Browse files Browse the repository at this point in the history
  • Loading branch information
arturoBeccar authored Nov 17, 2023
1 parent a542058 commit 3dac96c
Showing 1 changed file with 16 additions and 6 deletions.
22 changes: 16 additions & 6 deletions test-cases/instantiate-contract/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,24 @@ The implementation of instantiate_contract() is somewhat tied to that of code_ha

## Update on Correcting this Issue

In order to allow contract instantiation, we modified the `instantiate_contract()` function to call the dispatch code generated by the ink! code generator.
In order to allow contract instantiation, we modified the `instantiate_contract()` function in [PR #1988](https://github.com/paritytech/ink/pull/1988) to call the dispatch code generated by the ink! code generator. This way, `instantiate_contract()` can call contract constructors, allowing for the instantiation of contracts in integration tests. We also correctly keep track of callees as we move along the call stack, and set the code hash for the new contract account, as well as give it its endowment.

We also added a new `set_contract_storage_test()` function in order to obtain the storage key of the contract, which is now called from the generated code. This function calls `set_contract_storage()` with a specially computed key. This was needed because the code generated for integration tests does not properly generate the storage key for the contract.
For correct behavior emulation, a test should call `ink::env::test::upload_code()` before calling `instantiate_contract()`, which will simulate uploading the code of a contract to the environment and giving back a code hash.

This way, `instantiate_contract()` can call contract constructors, allowing for the instantiation of contracts in integration tests.
Something else worth noting is that we had to move the types under `ink::reflect` to the `ink_primitives` crate, since now both the ink and `ink_env` crates depend on those types and there was no other way to avoid a circular dependency.

Observation: In order to use contract instantiation in integration tests it is necessary to invoke cargo with:

`cargo test --features test_instantiate`
**Observation 1**:

In order to use contract instantiation in integration tests it is necessary to invoke cargo with:

```console
cargo test --features test_instantiate
```

This is necessary because getting back the return value of the constructor involved allowing the `execute_dispatchable()` function to return normally, but on-chain it never returns and instead the on-chain implementation of `return_value()` triggers a WASM trap that the runtime catches. In order to avoid changing the behavior of on-chain contracts, we preferred requiring enabling a feature just for off-chain tests, which modifies the signature of `return_value()` and allows a normal return from `execute_dispatchable()`.

**Observation 2**:

Initially, for Milestone 2, we opened [PR #1963](https://github.com/paritytech/ink/pull/1963) just for instantiate_contract(), since we internally organized our work separated by function. However, when we got to invoke_contract() in Milestone 3 we realized that these functions were too interconnected for separate PRs, so we decided to just have a single [PR #1988](https://github.com/paritytech/ink/pull/1988) for all of them. In the first [PR #1963](https://github.com/paritytech/ink/pull/1963) we included a function set_contract_storage_test() in order to obtain the storage key of the contract, which we removed in the second [PR #1988](https://github.com/paritytech/ink/pull/1988) because we deemed it was not necessary.

This was necessary because it was not possible to conditionally generate the call to `set_contract_storage_test()` based on whether there is a test being run.

0 comments on commit 3dac96c

Please sign in to comment.