put_code takes too much time

[pepyakin]

#3819 reports that put_code can take a lot of time for a big-but-still-reasonable-sized (IMO, 240k, see the issue) contract. While, integration of wasmtime can fix that, the situation can imply that put_code requires some optimization.

One clue how to improve this might be in the insight that we are doing multiple iterations over the wasm binary. We could try to leverage the streaming approach to read, check and instrument binary in one go. This however would require quite some refactorings of the wasm-utils side.

[expenses]

I've done some benchmarking on the wasm file in #3819 and (as expected) the hot functions are parity_wasm::elements::deserialize_buffer (4.46s) and wasmi_validation::validate_module (3.00s). The file actually fails to validate as it uses f64s, so a streaming approach would definitely speed things up by returning early.

[pepyakin]

Oh, thanks for looking into this!

Good to know. I think we can't rely on an early return since we always have to take into account the worst case (e.g. an attacker can always put f64 at the very end of code section).

[Stefie]

A little update on this:

I just ran into the same issue with:

• The latest versions of Substrate FRAME-contracts (spec_version: 195, impl_version: 196, at commit # 6e9c675)
• Latest version of cargo-contract (#c54fef9)
• and the latest version of ink! (use-ink/ink@414eca1))

with a more than reasonably sized contract (52k).

The problem persists and might be worse than expected. It has already been reported in #4153, that deploying contracts much smaller than 240k will bring block production to halt.

The compiled ERC20 example in the ink/examples/lang2/erc20 folder has a size of 52k and will cause this bug.
The .wasm file of the Flipper example has a size of 17k and can be deployed without problems.

So this might need some more investigation.

[expenses]

@Stefie I might try and write some benchmarks for this so we can examine why it's taking so long.

[Stefie]

There were some reports in the Substrate Technical & the Smart Contracts Riot channel in the last 2 weeks about the ERC20 example not working with 2.0 and at least 2 of them seem to be directly related to this issue. So if you have time that would be great!

[Stefie]

Seems like @rphmeier accidentally fixed it with this commit 4723156

With #a3fb0fda6 the error still does occur, but it's gone after that commit.

[gui1117]

Note that if I understand correctly this additional time to produce the block is under certain circumstances:

linear back-off.
in normal cases we only attempt to issue blocks up to the end of the slot.
when the chain has been stalled for a few slots, we give more lenience.

So even if indeed with our testing chain that does nothing it seems we have this additional time, in production we can't actually use on it really.

[gui1117]

additional idea that pop-up with @expenses maybe we can split the work in multiple extrinsics:

instead of having only put_code that put the code and do the check we could do that in multiple extrinsic such as:

• put_code: put_code but does no check
• check_1: do some check, and mark those check as done
• check_2: do some other check, and mark those check as done
• checked: check all mark are true, and accept code to be instantiable.

[rphmeier]

Seems like @rphmeier accidentally fixed it with this commit 4723156

Definitely intentional :)

[expenses]

Here's the instrumentation trace.