From 747741213b9cfd81c4e752252df2dc26825ce4f2 Mon Sep 17 00:00:00 2001 From: SeongChan Lee Date: Thu, 12 Sep 2019 04:24:48 +0900 Subject: [PATCH] Fix possible regression issue caused by `ProposeWaitEmptyBlockTimer` Possible scenario: * Schedule timeout for `ProposeWaitEmptyBlockTimer` on height: n * On timeout, timer loop queues a call to `Tendermint::on_timeout` on the timer worker * Blocks are imported at the same time with the timeout. * Block importer holds lock for Tendermint, calls `new_blocks` * The worker checks whether it is cancelled, but it is not cancelled yet. * The worker waits for the lock. * several `move_to_height`, `move_to_step` are called by new_blocks * height is not n anymore. * `move_to_step` clears timer, but the check is already passed and the worker waits for the lock. * It finally releases the lock. * The waiting worker calls Tendermint::on_timeout and the timeout for `ProposeWaitEmptyBlockTimer` calls `move_to_step(Prevote)` * The timeout was set for the height n, but the code in the timeout reads changed height. --- core/src/consensus/tendermint/worker.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/core/src/consensus/tendermint/worker.rs b/core/src/consensus/tendermint/worker.rs index 78df782f68..a71b069ac0 100644 --- a/core/src/consensus/tendermint/worker.rs +++ b/core/src/consensus/tendermint/worker.rs @@ -1232,8 +1232,15 @@ impl Worker { TendermintState::ProposeWaitEmptyBlockTimer { block, } => { - cdebug!(ENGINE, "Empty proposal timer is finished, go to the prevote step and broadcast the block"); - self.submit_proposal_block(block.as_ref()); + if self.height == block.header().number() { + cdebug!( + ENGINE, + "Empty proposal timer is finished, go to the prevote step and broadcast the block" + ); + self.submit_proposal_block(block.as_ref()); + } else { + cwarn!(ENGINE, "Empty proposal timer was for previous height."); + } } _ => { cwarn!(ENGINE, "Empty proposal timer was not cleared.");