From a3f889a345661dd2afd2b7420b46e9ed2658dfb4 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Thu, 21 Oct 2021 12:20:08 +0200 Subject: [PATCH] go/consensus/tendermint: Only reset executor pool after emitting block Make sure to only reset the executor pool after any timeouts have been cleared (e.g. when an empty block is emitted) as otherwise there could be a stale timeout. --- .changelog/4323.bugfix.md | 5 +++++ .../tendermint/apps/roothash/roothash.go | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 .changelog/4323.bugfix.md diff --git a/.changelog/4323.bugfix.md b/.changelog/4323.bugfix.md new file mode 100644 index 00000000000..64ec50e637f --- /dev/null +++ b/.changelog/4323.bugfix.md @@ -0,0 +1,5 @@ +go/consensus/tendermint: Only reset executor pool after emitting block + +Make sure to only reset the executor pool after any timeouts have been +cleared (e.g. when an empty block is emitted) as otherwise there could be a +stale timeout. diff --git a/go/consensus/tendermint/apps/roothash/roothash.go b/go/consensus/tendermint/apps/roothash/roothash.go index 69a3fb520fc..859acba15a6 100644 --- a/go/consensus/tendermint/apps/roothash/roothash.go +++ b/go/consensus/tendermint/apps/roothash/roothash.go @@ -237,14 +237,16 @@ func (app *rootHashApplication) suspendUnpaidRuntime( return err } - rtState.Suspended = true - rtState.ExecutorPool = nil - // Emity an empty block signalling that the runtime was suspended. if err := app.emitEmptyBlock(ctx, rtState, block.Suspended); err != nil { return fmt.Errorf("failed to emit empty block: %w", err) } + // Make sure to only reset the executor pool after any timeouts have been cleared as otherwise + // the emitEmptyBlock method will forget to clear them. + rtState.Suspended = true + rtState.ExecutorPool = nil + return nil } @@ -516,6 +518,14 @@ func (app *rootHashApplication) processRoundTimeout(ctx *tmapi.Context, state *r return fmt.Errorf("failed to get runtime state: %w", err) } + if rtState.ExecutorPool == nil { + // This should NEVER happen as the timeout should be cleared before the pool is reset. + ctx.Logger().Error("no executor pool", + "runtime_id", runtimeID, + ) + return fmt.Errorf("no executor pool") + } + if !rtState.ExecutorPool.IsTimeout(ctx.BlockHeight()) { // This should NEVER happen. ctx.Logger().Error("no scheduled timeout",