Add set_code_hash to EnvAccess

[ascjones]

• Add a new method set_code_hash2 which accepts the Hash environment type instead of concrete [u8; 32] to match other methods which handle code hashes.
• Add a method to EnvAccess to allow calling self.env.set_code_hash(&code_hash), which calls set_code_hash2.
• For backwards compatibility we keep the existing set_code_hash impl, which will be replaced by the set_code_hash2 impl in the next MAJOR release

[paritytech-cicd-pr]

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

⚠️ The ink! master is ahead of your branch, this might skew the comparison data below.

These are the results when building the examples/* contracts from this branch with cargo-contract 2.0.2-755ea06 and comparing them to ink! master:

	Δ Optimized Size	Δ Used Gas	Total Optimized Size	Total Used Gas
accumulator			1.13 K
adder			1.75 K
call-runtime			1.79 K
contract-terminate			1.28 K	129_990
contract-transfer			1.44 K	129_990
custom-environment			2.55 K
custom_allocator			8.24 K
delegator			6.10 K	259_980
dns			8.84 K	389_970
erc1155			17.03 K	779_940
erc20			7.53 K	389_970
erc721			11.31 K	1_039_920
flipper			1.40 K	129_990
forward-calls			2.44 K	259_980
incrementer			1.23 K
mapping_integration_tests			3.35 K
mother			10.49 K
multisig			23.51 K	779_940
payment-channel			6.07 K
psp22-extension			7.02 K
rand-extension			2.93 K
set-code-hash			1.60 K
subber			1.77 K
trait-erc20			7.95 K	389_970
trait-flipper			1.18 K	129_990
trait-incrementer			1.34 K	259_980
updated-incrementer			1.60 K

Link to the run | Last update: Thu Mar 2 13:59:11 CET 2023

[codecov-commenter]

Codecov Report

Merging #1698 (669b567) into master (7cf12af) will increase coverage by 0.10%.
The diff coverage is n/a.

❗ Current head 669b567 differs from pull request most recent head 3628ee6. Consider uploading reports for the commit 3628ee6 to get more accurate results

@@            Coverage Diff             @@
##           master    #1698      +/-   ##
==========================================
+ Coverage   70.72%   70.82%   +0.10%     
==========================================
  Files         205      205              
  Lines        6409     6407       -2     
==========================================
+ Hits         4533     4538       +5     
+ Misses       1876     1869       -7     

Impacted Files	Coverage Δ
crates/env/src/api.rs	35.82% <ø> (+1.03%)	⬆️
crates/ink/src/env_access.rs	9.09% <ø> (ø)
crates/ink/ir/src/ast/attr_args.rs	82.60% <0.00%> (-2.18%)	⬇️
crates/ink/ir/src/ir/attrs.rs	82.02% <0.00%> (+0.28%)	⬆️
crates/metadata/src/layout/mod.rs	80.83% <0.00%> (+0.83%)	⬆️
crates/allocator/src/bump.rs	87.60% <0.00%> (+1.65%)	⬆️
crates/metadata/src/lib.rs	55.00% <0.00%> (+10.00%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[HCastano]

I like having Hash instead of [u8; 32], but changing the env/api function signature would require a MAJOR version bump.

It's too early in the life of 4.0 to start keeping release branches around so that master can contain breaking changes like this.

While it would be fine to add the function with the new signature to EnvAccess, I'd add it with the old signature ([u8; 32] ) for consistency.

[ascjones]

I like having Hash instead of [u8; 32], but changing the env/api function signature would require a MAJOR version bump.

You're right, didn't consider that. I wonder if there is a compromise using AsRef<[u8; 32]>

[ascjones]

I like having Hash instead of [u8; 32], but changing the env/api function signature would require a MAJOR version bump.

You're right, didn't consider that. I wonder if there is a compromise using AsRef<[u8; 32]>

Adding a AsRef<[u8; 32]> impl to types::Hash worked, but was also a breaking change because of the existing AsRef<[u8]> impl. e.g. the code some_hash.as_ref() would now require a type annotation to pick the correct impl.

Therefore I have gone with adding a set_code_hash2 method to the api which is called by EnvAccess::set_code_hash. Then in the next MAJOR we replace set_code_hash with set_code_hash2.

[cmichi]

Can you create a follow-up ticket for removing the comment and renaming the function? Can also add the Hernando wishlist there:

I like having Hash instead of [u8; 32], but changing the env/api function signature would require a MAJOR version bump.

[cmichi]

Clarification of the comment please, besides that LGTM.

Oh, and CHANGELOG.md :-).

[cmichi]

Thanks, much cleaner!

[HCastano]

Thanks 👌