From 3dac96cec5ac4fd61ed7172c415865970b10b4cb Mon Sep 17 00:00:00 2001 From: Arturo Beccar-Varela <107512933+arturoBeccar@users.noreply.github.com> Date: Fri, 17 Nov 2023 17:58:29 -0300 Subject: [PATCH] Added Update on Correcting this Issue --- test-cases/instantiate-contract/README.md | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/test-cases/instantiate-contract/README.md b/test-cases/instantiate-contract/README.md index 67420b6..e74bd08 100644 --- a/test-cases/instantiate-contract/README.md +++ b/test-cases/instantiate-contract/README.md @@ -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.