[fix] #4082: Remove cloning for ISI and query execution in wasm

[mversic]

trafficstars

Description

I left Visit trait to work with references and have removed redundant cloning through other means. One of the reasons why I left it is so that we can keep calling mem::forget at the end of validation but also because it's just not necessary to take them by value

Linked issue

Closes #4082

Benefits

less cloning

Checklist

• [ ] I've read CONTRIBUTING.md
• [ ] I've used the standard signed-off commit format (or will squash just before merging)
• [ ] All applicable CI checks pass (or I promised to make them pass later)
• [ ] (optional) I've written unit tests for the code changes
• [ ] I replied to all comments after code review, marking all implemented changes with thumbs up

[DCNick3]

I kind of don't like how fragile this approach is: any mismatch between the encode_as_* function implementation and the derived scale encodings (that can be easily created by accident when modifying some instructions, for example) would lead to hard-to-diagnose encoding issues. But I don't there's a way to resolve this that doesn't rely on macros, and we already have enough of those in iroha..

If we decide to go the macro way we can either:

• automatically derive the encode_as_* functions (keeping the approach used in this PR)
• automatically generate the sister *BoxRef structures with owned values replaced by references (following the approach suggested by @Erigara)

I think the second one will be easier to implement, especially seeing how heterogeneous the encode methods for instructions are due to different levels of nesting..

Another far-fetched idea w/o using macros is to somehow make instructions/queries cheaper to clone (for example, by moving out all the expensive objects like strings to arenas/bump allocators/...). I don't think those are worth the complexity they bring though.

[Arjentix]

any mismatch between the encode_as_* function implementation and the derived scale encodings (that can be easily created by accident when modifying some instructions, for example)

I don't think that could actually happen to us. encode_as_* fallback to self.encode_to(), so changing internal fields won't break anything. And while we keep this enums everything should be fine. Throwing back these enums would be a big change and we certunally catch such problems with encodings (at least because of autogenerated *Type enums).

[Arjentix]

I suggest to create a derive macro called something like EncodeLikeInstructionBox and EncodeLikeQueryBox. Same for decoding. They will implement only existing traits from parity_scale_codec like Encode and EncodeLike, no new traits needed.

[coveralls]

Pull Request Test Coverage Report for Build 8063598500

Details

• -193 of 348 (44.54%) changed or added relevant lines in 12 files are covered.
• 3132 unchanged lines in 57 files lost coverage.
• Overall coverage decreased (-0.3%) to 56.494%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
data_model/src/metadata.rs	7	11	63.64%
smart_contract/executor/src/default/tokens.rs	0	7	0.0%
smart_contract/executor/src/default.rs	0	8	0.0%
data_model/src/query/mod.rs	0	11	0.0%
data_model/derive/src/enum_ref.rs	92	106	86.79%
data_model/src/lib.rs	0	22	0.0%
smart_contract/src/lib.rs	0	36	0.0%
data_model/src/isi.rs	2	93	2.15%
<!--	Total:	155	348

Files with Coverage Reduction	New Missed Lines	%
core/src/sumeragi/network_topology.rs	1	98.78%
smart_contract/executor/src/default/tokens.rs	1	0.0%
tools/kagami/src/crypto.rs	1	12.12%
data_model/src/metadata.rs	2	78.87%
primitives/src/unique_vec.rs	2	91.24%
config/src/kura.rs	3	80.0%
smart_contract/src/lib.rs	3	1.34%
config/src/logger.rs	5	72.73%
logger/src/lib.rs	5	94.12%
tools/kagami/src/main.rs	5	0.0%
<!--	Total:	3132

Totals
Change from base Build 7884695009:	-0.3%
Covered Lines:	22576
Relevant Lines:	39962

---

💛 - Coveralls

[mversic]

@Arjentix @DCNick3 @Erigara how do you like the modified hybrid approach now? I think it eliminates the previous concerns. I chose to expose encode_as_instruction_box rather than expose InstructionBoxRef in the public API

[github-actions[bot]]

@BAStos525