broken EpochLength and MinGasPrice events

[troggy]

Bounty

Given:

• fresh local network with a single node
• set Epoch Length to 1
• stop the network and clean local data
• start the network again

Expected:

• the node reads two EpochLength events: setting epoch length to 2 (default) and 1 correspondingly
• the node processes them one by one and sets epoch length 1 as the result of the sync

Actual:

• the node reads two EpochLength events: setting epoch length to 2 (default) and 1 correspondingly
• the node fails with leap-node:error Wrong epoch length error. Epoch length is 0 after sync

Scope

• figure out the way to handle multiple EpochLength/MinGasPrice evens in a row
• implement
• fix tests

Deliverables

• updated code
• updated tests

Gain for the project

working node

Expert

Publicly visible delivery

Roles

bounty gardener: @troggy / 20 DAI bounty worker: @troggy / 80% bounty reviewer: name / 20%

Funded by

Dev Circle

[troggy]

There are multiple problems here and all relate to epochLengthIndex not being reliable:

Problem 1 In situation, when two epoch lenght txs are being added to mempool almost at the same time (like in the case described in issue above), checkTx will be called for both of them with the same state. The state includes epochLengthIndex, thus checkEpochLength handle will fail for the second tx (index will be wrong).

Possible solution: don't keep epochLengthIndex in consensus state. Instead keep separate checkIndex and deliverIndex. 💩

Problem 2 (but only if we mutate bridgeState in tx handler, otherwise not a problem) Tendermint MAY call checkTx for the same tx multiple times (see Mempool Connection section here). The second call is a recheck which happens before Commit.

Possible solution: as of tendermint 0.32.1 the app is able to distinguish between CheckTx_New and CheckTx_Recheck. However, this requires tendermint upgrade. Alternatively, we can have some in-house checks to ensure CheckTx was already executed for a given epoch length tx. 💩

---

Probably, a better solution is to include the event index into the tx itself (like it is done with deposits).

UPD: opt to easier workaround. See #407

[troggy]

FYI @sunify 😅Enough season of "Epoch Length Clusterf*ck" saga. (Hopefuly) resolved here: #407