Use a 2/3 instead of 1/2 majority in Aura.

[afck]

Closes #108.

[phahulin]

Also probably in this test https://github.com/poanetwork/parity-ethereum/blob/b5097c67155debd9aee2c5e499456b4a1f187b26/ethcore/src/snapshot/tests/proof_of_authority.rs#L186

[afck]

Good catch! Unfortunately the tests passed with the * 2 and now fail with the * 3 > … * 2. I'll try to fix them.

[afck]

That's another test that uses an old validator set contract ABI, and (according to the docs) this implementation: https://gist.github.com/anonymous/2a43783647e0f0dfcc359bd6fd81d6d9 @vkomenda: Is that the one you already created an updated version of?

[vkomenda]

That's another test that uses an old validator set contract ABI

Well spotted! We could use the updated version or we might store this file alongside the tests, possibly having updated to Solidity v0.5. My version doesn't emit ValidatorsChanged.

[afck]

Yes, maybe we should try to make one test implementation work for all tests, and add ValidatorsChanged to it.

[vkomenda]

I can't find where ValidatorsChanged is received in Parity. Maybe it's unused?

[afck]

Possibly… if it's only in the test contract, maybe it's only used in that test?

[vkomenda]

I don't think the event is ever used at all. We can try without it to see if the test still passes.

[varasev]

Since we need 2/3 of the majority not only for POSDAO but also for our existing networks (Core/Sokol/Dai), let's do the same changes made in this PR above the original Parity code (which is not modified for POSDAO) and try to launch Parity's original unit tests.

[afck]

I added a quorum_2_3_transition option to the Aura configuration.

[varasev]

I added a quorum_2_3_transition option to the Aura configuration.

The corresponding parameter in spec.json is called quorum23Transition, right?

[afck]

Ah, right, it's camel-cased. :+1:

[varasev]

Can we add some line to the logs which would tell us that the quorum23Transition is applied?

[afck]

I added a message—is trace enough or should it be a higher log level?

[varasev]

Let's display it on the info level like here: https://github.com/paritytech/parity-ethereum/blob/e91eb337c9d01ff339565e682bcd2b4b7608df6d/ethcore/src/engines/authority_round/mod.rs#L1429 so that it would be shown in the logs even when logging = "engine=trace" is not defined in toml config file.

Can we display it starting from the block when it is applied? Like https://github.com/paritytech/parity-ethereum/blob/e91eb337c9d01ff339565e682bcd2b4b7608df6d/ethcore/src/engines/authority_round/mod.rs#L1429.

[afck]

I updated the last commit accordingly. :+1:

[varasev]

Thanks, I will try to test that :+1:

[varasev]

Please do the same for vk-stable-mulmod branch: https://github.com/poanetwork/parity-ethereum/pull/109/commits/aa7dce71ae26e0c03765d09f4de3d31d3fedace2

[afck]

Done. (@vkomenda: I took the liberty of directly pushing the log statement onto vk-stable-mulmod.

[varasev]

I updated the last commit accordingly. 👍

I see two lines in the logs for some reason:

And for the detailed logs (another, non-validator, node):

[afck]

I put it into on_close_block—another method that seems to be called multiple times for some reason, despite #103…

[vkomenda]

I looked at the callers of on_close_block and it looks like import_verified_blocks is called multiple times. This can happen if the block queue is flushed multiple times.