feature: Implement compute usage aggregation and limiting

[aborg-dev]

feature: Limit compute usage of a chunk

The commit addresses #8265 by introducing aggregation of compute usage across all operations performed during chunk application and limiting this compute usage to 1s.

This should not change the behavior of nodes in the short run because compute costs match gas costs which is validated by the assert, so any discrepancy should be caught on canary nodes.

There are two follow-ups to this work:

• Introduce Compute Limit chunk property #8859
• Make compute_usage a first-class citizen in ExecutionOutcome #8860

[jakmeier]

The limit check looks good but I am afraid the change to ExecutionOutcome has unwanted consequences. Please check out my longer comment on it.

[Longarithm]

I didn’t realise before that we replace gas limit checks with compute limit checks completely. It makes sense, but shouldn’t we add some runtime check that total compute cost is always > 0, or config check that each separate compute cost is not lower than gas cost? I’m thinking about the case when compute costs are accidentally set too low - as we don’t put restrictions on their values - which can result in very long execution. Also this changes intuition “1 chunk is 1 Pgas” - if we don’t tie compute costs to gas costs, now we have to say “1 chunk is 1 s compute cost”. Perhaps it’s even better.

Also, to clarify - am I right that we don’t count compute costs during wasm execution at all; instead we compute them afterwards to ensure that contracts are not broken?

[aborg-dev]

@Longarithm , thank you for the comment! See answer below:

I didn’t realise before that we replace gas limit checks with compute limit checks completely. It makes sense, but shouldn’t we add some runtime check that total compute cost is always > 0, or config check that each separate compute cost is not lower than gas cost? I’m thinking about the case when compute costs are accidentally set too low - as we don’t put restrictions on their values - which can result in very long execution.

The idea behind the compute costs is that they represent the pessimistic estimate of the time it takes to process a particular operation - as long as that is maintained, the time to execute the transactions that fit within the chunk limit will be bound by target 1s. In other words, we need to be careful to set compute costs high enough, just as we need to be careful when setting gas costs, there is no way around that :)

I don't think there is a strict need for compute costs to be higher than the gas cost as long as the property above is satisfied. For example, if some operation is overcharged, it should be safe to set a compute cost for it that is lower than the gas cost (though we probably would decrease the gas cost in that situation as it wouldn't break contracts).

We've actually discussed the case when compute cost could be 0 in the NEP. This is quite a rare case, so I'm open to adding runtime check for this for now, though I don't think it would add much to safety on top of a thorough review of new compute costs, given that all their adjustments will be a protocol change that requires a review.

Also this changes intuition “1 chunk is 1 Pgas” - if we don’t tie compute costs to gas costs, now we have to say “1 chunk is 1 s compute cost”. Perhaps it’s even better.

Definitely, we would need some new vocabulary here, I like your proposal of "1s of compute cost"!

Also, to clarify - am I right that we don’t count compute costs during wasm execution at all; instead we compute them afterwards to ensure that contracts are not broken?

Yes, we don't limit the WASM contract execution based on compute costs and we compute them from profiles after the execution. This is to avoid breaking existing contracts. Here is the implementation #8783.

[jakmeier]

If we use borsh skip, that means when we read an outcome from the DB, it will always be zero, right? This might become problematic for replaying in the state viewer at some point.

Do you think it would be too cumbersome to make it an Option<Compute> to make it clear when the field is initialized and when not? We would probably unwrap it in places where it must be initialized, and ignore it in all other places, so it shouldn't change any actual behavior. But it would prevent future code from accidentally using this uninitialized value.

[aborg-dev]

That's a good idea. I was initially thinking of backfilling this value using borsh_init [1] and setting it to the burnt_gas, but that would eventually break when we set non-trivial compute costs. Option at least makes it clear that one can't rely on the value if it was read from the database.

[1] https://github.com/near/borsh-rs#features

[aborg-dev]

@jakmeier , I've added a unit test for this (though one can probably argue that it's an integration test as well), PTAL

[jakmeier]

LGTM, just some nitpicking for additional comments and an extra checks in the test :)

(Instead of multiple inline comments for the test, you could also add a single function level comment. Anything that explains how the test works really.)