Set gas limit according to TxPermission.blockGasLimit.

[afck]

WIP: This passes the posdao-test-setup, but both the low and the regular gas limits are still hard-coded, and the call in verify_header_params uses the wrong header at the moment. (See TODOs.)

It also extends the Engine trait in a way that's not exactly sane. And it doesn't allow for a variable gas limit.

And crucially, I don't know yet whether the state of the block hash that we use for the limitBlockGas call is always available in the client's database, especially if there's a fork; or whether and how this will work together with snapshots and warp sync.

Closes #119.

[varasev]

Can we first try to do the same without calling the TxPermission.limitBlockGas()? Just imagine that for, e.g., 200-blocks staking epoch the gas limit should be decreased starting from the block 161 and should be restored after the block 239 (then for the second staking epoch the band is 361...439, and so on).

[afck]

Right, that would make things easier; basically we could just implement a configurable gas limit schedule, so one could write something like this into the chain spec:

"gasLimitSchedule": {
  "period": "200",
  "defaultLimit": "8000000",
  "schedule": [
    { "from": "161", "to": "200", "limit": "2000000" },
    { "from": "0", "to": "239", "limit": "2000000" }
  ]
}

[varasev]

The formula for those bands is: limit the block gas when the current block number

is greater than

stakingEpochEndBlock() - MAX_VALIDATORS * DELEGATORS_ALIQUOT - 1

or is less than

stakingEpochStartBlock() + MAX_VALIDATORS * DELEGATORS_ALIQUOT + 1

It would be equivalent of calling the TxPermission.limitBlockGas().

So, these values can be read once per staking epoch and used statically in Parity code.

What do you think about it?

[afck]

Or that, yes.
But it would be even easier to implement if we didn't have to ask the contract at all.

[varasev]

We could make in Parity a function like that:

fn limit_block_gas(block_number: U256) -> bool {
    if staking_epoch_duration <= 0 || max_validators <= 0 || delegators_aliquot <= 0 {
        return false;
    }

    let staking_epoch = block_number / staking_epoch_duration;

    let from = staking_epoch_duration * staking_epoch + max_validators * delegators_aliquot + 1;
    let to = staking_epoch_duration * (staking_epoch + 1) - max_validators * delegators_aliquot - 1;

    if block_number <= from || block_number >= to {
        true
    } else {
        false
    }
}

where

staking_epoch_duration is a constant taken from the stakingEpochDuration getter;
max_validators is a constant taken from the MAX_VALIDATORS getter;
delegators_aliquot is a constant taken from the DELEGATORS_ALIQUOT getter.

I think we should read these constant values from the contracts anyway because in case of writing their values to spec.json their values will be duplicated with those which are defined in the contracts.

This approach assumes that those constants and the formula will never be changed during the network's life.

[afck]

But then we might as well just call limitBlockGas directly, at just the correct block number (possibly a sibling), and assume that it only depends on the block number. That's a weaker requirement for the contract and moves less of the contract's implementation details into Parity than using those constants.

Still, I think the cleanest approach would be if we'd manage to call the function really at the right block. I'll try to dig through the code some more and see if we can do that.

[afck]

We also need to make a decision regarding the gas limit values if the function returns true or false. I don't like hard-coding values, and we can't e.g. use the target gas limit when switching back from true to false, because that's a per-node configuration, not part of the chain spec.

We could change limitBlockGas to gasLimit: Have the function return the gas limit for the current block itself? (Or for the next block, depending on where exactly we manage to call it…)

[varasev]

Ok, let's change the function so that it would return gas limit instead of boolean value. Let's do this after you ensure that we can call the function at the right block (as you wrote above).

[afck]

In Client::check_and_lock_block, the function that checks the gas limit, verify_block_family, is called before the new state is computed. But the good news is that at this point it is already required that the parent block's state is in the database, because enact_verified would fail with InvalidStateRoot otherwise.

We could:

• Change the order; but it's probably like this for a reason: if the gas limit can be checked without the state, it should be done first, to avoid the overhead of enacting an invalid block.
• Make the call at the parent block's state. (What this PR currently does.) We'd need to modify limitBlockGas or gasLimit to return the gas limit for the next block.
• Use the parent block's state and make the call with a modified env info: I think this corresponds to making the call at the beginning of the next block, and the contract function would then see the correct (i.e. the new block's) block number. Not sure how complicated that is.

We should also decide what to do in terms of backward compatibility:

• We could just make the contract call, and if it fails (because the contract doesn't have that function), we go with the default behavior.
• We could add another transition for this. (contractBasedGasLimitTransition?)

[varasev]

Let's postpone this task a while, I'll try to repeat stress tests again and recheck those 80 blocks when switching staking epoch to see more closely how it is critical to have reduced gas limit.

[afck]

I implemented the third option: blockGasLimit is now called before each new block, but with the new block's number.

[afck]

As discussed, I will add a separate blockGasLimitContract option for this.
I will probably also move it out of the engine, since it is not Aura-specific. (That's not as easy as I had hoped…)

[varasev]

Works fine 👍