search

AntelopeIO / leap

C++ implementation of the Antelope protocol
Other
116 stars 68 forks source link

IF: port tests using qc_chain and chain_pacemaker to work with the new implementation #2147

Closed linh2931 closed 8 months ago

linh2931 commented 9 months ago

Depends on https://github.com/AntelopeIO/leap/issues/2193.

https://github.com/AntelopeIO/leap/issues/2123 removes qc_chain and chain_pacemaker related code.

There are a large number of comprehensive tests using qc_chain and chain_pacemaker. They should be reviewed; those appropriate should be ported to work with the new implementation when appropriate.

linh2931 commented 9 months ago

Review results

3 files in hotstuff test directory contain tests: test_hotstuff.cpp, hotstuff_tools.cpp, and test_hotstuff_state.cpp.

  1. hotstuff_tools.cpp tests votes and is kept.
  2. test_hotstuff_state.cpp was intended to test safety file; it is unfinished. When new finalizer safety file work (https://github.com/AntelopeIO/leap/issues/2070) is finished, new tests should be added.
  3. test_hotstuff.cpp has 9 tests:
    • hotstuff_1 -- test optimistic responsiveness (3 confirmations per block): precommit, precommitQC, commit all come within one block. Not applicable.
    • hotstuff_2 -- test slower network (1 confirmation per block): precommit comes in one block a time. Existing tests cover this. But we want to also properly test slower networks where some blocks do not have a QC and finality advances slowly. https://github.com/AntelopeIO/leap/pull/2234
    • hotstuff_3 -- test leader rotation: Covered by existing tests (both integration and Boost tests).
    • hotstuff_4 -- test loss and recovery of liveness: https://github.com/AntelopeIO/leap/pull/2223
    • hotstuff_5 -- test finality violation: This is covered explicitly in a later issue: https://github.com/AntelopeIO/leap/pull/2223
    • no hotstuff_6
    • hotstuff_7 -- test leader rotation with a non-complete connection graph: Covered by existing integration test (the one that brings down a bridge and then brings it back up).
    • hotstuff_8 -- same as hotstuff_1 with duplicate vote messages. We already test duplicate vote messages in separate Boost tests. Also in https://github.com/AntelopeIO/leap/pull/2234.
    • hotstuff_9 -- test leader rotation with a star topology: Covered by existing integration test.
    • hotstuff_10 -- test leader rotation with a ring topology: Covered by existing integration test.
greg7mdp commented 8 months ago

Re-enable weak vote related tests which were disabled in order to merge #2135

linh2931 commented 8 months ago

Previous hotstuff tests did not have tests for weak votes. Each of the 9 tests has either a similar test now or not applicable. Close this and reopen https://github.com/AntelopeIO/leap/issues/2193 for weak related tests (which cannot be done by simply changing vote messages in the middle of transmission).