Prepare actual weights for runtime transactions

[svyatonik]

In #69 I had to switch to Substrate master to be able to use GrandpaJustification::decode_and_verify_finalizes() from builtin. It turned out that the main change is that now all runtime calls (transactions) require annotated weights. So before considering PoA -> Substrate headers sync to be finished, we need to prepare actual weights for all calls. I've started to do that, but it's a quite complicated process + as some runtime changes are still planned (I'm mostly talking about #62 and maybe #38 ), it isn't the right moment to do that now.

The main problem is that we can't compute weight of header import call by looking at the header itself. The main source of uncertainty is finality computations - we may need to read arbitrary number of headers (and write some updates after that) if validators haven't been finalized headers for too long. At the beginning I had some ideas that we'll cache computation results in the storage, so we don't need to recompute it on every block, but iirc I've stuck with something - so most probably this is impossible, and we'll need storage reads anyway :/ But may worth looking at it again.

That said, most probably, we need to introduce some amortized weight for import calls. This may also affect some params/default constants in relay, because weight may be too big to fit into 'normal' block.

So I'm proposing to mark all methods with #[weight=0] (or whatever you suggest) and change that afterwards.

[tomusdrw]

So I'm proposing to mark all methods with #[weight=0] (or whatever you suggest) and change that afterwards.

Sounds good. Let's add a TODO and this issue number to make sure we address that.

The main problem is that we can't compute weight of header import call by looking at the header itself. The main source of uncertainty is finality computations - we may need to read arbitrary number of headers (and write some updates after that) if validators haven't been finalized headers for too long.

I'd be down for defining some upper bounds and possibly (at least have a) fall-back to a slower mechanism, where we have to process the finality in smaller chunks to make sure we don't go over that upper bound.

Number of headers that are expected to be read can also be passed with the call itself to avoid storage read - then we check if that's actually the case when the call is being processed. That will allow having a dynamic weight without incurring extra state read on computation.

[tomusdrw]

Also worth noting:

[HCastano]

We should probably get some help from those doing the Substrate benchmarking when it comes time to figure out our weight values

[svyatonik]

Did some finality benches. There has been significant finalization delay in at Kovan chain start => it is a good illustration on how current finality affects block import time. So with current code, block import times are:

block1: 5ms
...
block10: 7ms
...
block50: 10ms
...
block100: 16ms
...
block150: 22ms
...
block500: 64ms
...
block750: 98ms
...
block1000: 123ms
...
block1926: 231ms

Without finality, everything is constant:

block1: 4ms
...
block10: 4ms
...
block1926: 4ms

This ^^^ doesn't include storage modifications during actual finalization, so when first block will be finalized, it'll be another spike in import time. So I'm going to:

1. try to reorganize finalization code to make it closer to O(1) instead of O(N);
2. if this ^^^ attempt will fail, or it'll require too much additional storage operations => will try to span finalization process over several transactions. This is what I really want to avoid, because it'll also affect relayer logic, but otherwise we might see stalls on actual chains later.

[tomusdrw]

will try to span finalization process over several transactions.

Note that with substrate we also have on_initialize method in our pallet, so we can do some chunk of work on every block without an extra transaction required to trigger this logic. I imagine it might be possible to:

1. Submit (& verify) all details needed for finalisation
2. Process these details in chunks within on_initialize for couple of subsequent blocks.

The "cost" would not be covered by the user though, but rather cause some chain slowdown, so we need to be super sure that this doesn't open up some DoS vector.

[svyatonik]

Great idea! Probably we can use the same logic for pruning.

[svyatonik]

So to compute weights we need to prepare some benchmarks - like it is done here. More docs here: https://www.shawntabrizi.com/substrate-graph-benchmarks/docs/#/

[HCastano]

I was talking with Shawn and he said that to prepare actual weights we should use the "new" automated approach. There's an issue for it in paritytech/substrate#6168, and code to generate weights automatically in paritytech/substrate#6567

[tomusdrw]

Closing for now. If we plan to switch to new weights, let's open a new issues instead.