poanetwork / hbbft

An implementation of the paper "Honey Badger of BFT Protocols" in Rust. This is a modular library of consensus.
Other
357 stars 96 forks source link

Test `drop_and_re_add` fails. #390

Closed afck closed 5 years ago

afck commented 5 years ago

The drop_and_re_add test fails with the following net_dynamic_hb.proptest_regressions file:

xs 3766095200 4099363815 241143382 3155674404 # shrinks to cfg = TestConfig { dimension: NetworkDimension { size: 10, faulty: 3 }, total_txs: 27, batch_size: 12, contribution_size: 7, seed: [73, 242, 153, 97, 138, 68, 78, 181, 145, 37, 155, 185, 206, 103, 141, 115] }

Node 5's DynamicHoneyBadger never receives the first batch after getting re-added. The Subset instances for the subsequent batches complete, but the node is stalled waiting for the one in epoch 5.

The reason is that the Broadcast instances with nodes 3 and 7 as proposers don't terminate (the corresponding BinaryAgreement instances both returned true). I haven't found out why yet:

vkomenda commented 5 years ago

Would it make sense trying older revisions of net_dynamic_hb with this seed? Or the seed wouldn't lead to the same behaviour there?

afck commented 5 years ago

It probably wouldn't lead to the same behaviour: The test has changed quite a bit recently, I think?

Anyway, I suspect now that the second point is the problem: Node 7's sender queue actually removes node 5 after handling batch 4, in which node 5 was added back.

It might be an unrealistic scenario, but I still think we should handle it: The sender queue should keep track of re-added nodes, even if they are in the last_epochs map.