Bugfix test on Sokol testnet (step 1)

[phahulin]

Dear sokol validators,

we’re trying to find a workaround/solution to a possible parity bug https://github.com/paritytech/parity-ethereum/issues/9764 observed on some nodes.
To test our assumptions we’d like to propose a test on sokol network.
The test would require changing RewardByBlock contract to let it gracefully handle unexpected input parameters that seem to be sent by parity.
Here is the Pull Request with actual changes: https://github.com/poanetwork/poa-network-consensus-contracts/pull/205

The test would consist of two steps:

1. create a ballot to change implementation of ProxyStorage contract to allow it to change RewardByBlock implementation
2. after p.1 is done, create a ballot to change implementation of RewardByBlock contract that contains the actual workaround/fix

The proposed updated version of ProxyStorage contract has already been deployed and verified: https://blockscout.com/poa/sokol/address/0xff8dc77e54fa96eb158509c8dea7971197748332/contracts

So we’d like to ask one of you to proceed with step 1 and create the following ballot on SOKOL network:

 - ballot type:       Modify Proxy Contract Ballot
 - proposed address:  0xff8dc77e54fa96eb158509c8dea7971197748332
 - contract type:     ProxyStorage

[phahulin]

@1proof asks:

Could you describe the actual problem that might be affecting network performance and how this action will fix

The problem is described in the first github issue mentioned in the description. Basically it’s an error message in parity logs produced by RewardByBlock contract. The actual performance impact is not clear because the error is happening only on some nodes and because overall tx rate is not too high at the moment.

Also, if this is a bug fix, could you describe why this particular issue merits a ballot? Is the goal to be able to test the ability to change reward method or similar?

During our investigation of the issue we discovered that parity sometimes sends a zero address as a block author to the reward contract (https://github.com/paritytech/parity-ethereum/issues/9764#issuecomment-431070493) and this seems to cause the issue. It’s not clear why it does so. But reward contract doesn’t expect a zero address and reverts. The proposed change is to handle this situation without throwing an exception (https://github.com/poanetwork/poa-network-consensus-contracts/pull/205/files#diff-8315f75f7610f81de832bf0767b2ba6c).
This requires an update of reward contract, but reward method is not changed. To make it possible to change the reward contract we first need to add the ability to create a ballot for it. So there will be two ballots required. The first required ballot is described in this post

Maybe since chain spec will be modified it requires a vote?

Chainspec is not modified, this is not a hard fork.

[igorbarinov]

The ballot was created on Sokol testnet for the change
You can track it here:
https://voting.poa.network/poa-dapps-voting

[1proof]

Hello Pavel. From an earlier Github post at following link, there is an exchange that seems to treat this as a test, not a fix. It is not clear what problem will be fixed. Could you explain in a bit more detail, please? Here is relevant section from comments:

varasev commented on Dec 14, 2018
@phahulin I propose to upgrade BlockReward contract in Sokol network in the following manner: varasev/poa-network-consensus-contracts@cde6f30 - and watch for some time whether the error still appears.

@phahulin
phahulin commented on Dec 14, 2018
@varasev I think it’s a good idea. If the error would still persist, we should revert back to the original BlockReward contract so that they are the same on Sokol and on Core. @igorbarinov do you agree with this test?

@igorbarinov
igorbarinov commented on Dec 14, 2018
Sounds good

Here’s the Github link:

Issue: Closing the block failed Cannot decode address[]

Thanks in advance!

[igorbarinov]

Sokol is a testnet.
The fix was tested locally by Vadim.
Now we need to deploy it on the testnet.

I assume that test is not the right word. @phahulin let’s rename it to fix.

[phahulin]

I have already explained it here Bugfix test on Sokol testnet (step 1)