From 1630d41d86b5dfd2339d932d53a6ad1abb0e1aca Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 14:09:57 +0100 Subject: [PATCH 01/10] lfg --- block/produce.go | 41 ++++++++++------------------------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/block/produce.go b/block/produce.go index 700394753..c9e99b51b 100644 --- a/block/produce.go +++ b/block/produce.go @@ -19,43 +19,19 @@ import ( func (m *Manager) ProduceBlockLoop(ctx context.Context) { m.logger.Debug("Started produce loop") - // Setup variables for empty block production - var ( - produceEmptyBlock = true // Allow the initial block to be empty - resetEmptyBlocksFn = func(bool) {} - nextEmptyBlock = time.Time{} - ) - - // Setup ticker for empty blocks if enabled - // if disabled, produceEmptyBlock will remain true and the block will be produced every block time - if 0 < m.Conf.MaxIdleTime { - //define a reset function to reset the timer - resetEmptyBlocksFn = func(proofRequired bool) { - produceEmptyBlock = false - if proofRequired { - nextEmptyBlock = time.Now().Add(m.Conf.MaxProofTime) - } else { - nextEmptyBlock = time.Now().Add(m.Conf.MaxIdleTime) - } - } - resetEmptyBlocksFn(false) - } - - // Main ticker for block production ticker := time.NewTicker(m.Conf.BlockTime) defer ticker.Stop() + var nextEmptyBlock time.Time + for { select { case <-ctx.Done(): return case <-ticker.C: - //check if we can produce a block even if there are no transactions - //if the time has passed, we will produce an empty block - if !nextEmptyBlock.IsZero() && nextEmptyBlock.Before(time.Now()) { - m.logger.Debug("no transactions, producing empty block") - produceEmptyBlock = true - } + + // if empty blocks are configured to be enabled, and one is scheduled... + produceEmptyBlock := 0 < m.Conf.MaxProofTime && nextEmptyBlock.Before(time.Now()) block, commit, err := m.ProduceAndGossipBlock(ctx, produceEmptyBlock) if errors.Is(err, context.Canceled) { @@ -73,11 +49,14 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { m.logger.Error("produce and gossip: uncategorized, assuming recoverable", "error", err) continue } + // If IBC transactions are present, set proof required to true // This will set a shorter timer for the next block // currently we set it for all txs as we don't have a way to determine if an IBC tx is present (https://github.com/dymensionxyz/dymint/issues/709) - proofRequired := len(block.Data.Txs) != 0 - resetEmptyBlocksFn(proofRequired) + nextEmptyBlock = time.Now().Add(m.Conf.MaxIdleTime) + if 0 < len(block.Data.Txs) { + nextEmptyBlock = time.Now().Add(m.Conf.MaxProofTime) + } // Send the size to the accumulated size channel // This will block in case the submitter is too slow and it's buffer is full From 73faed93b2469ac6be7b1f22a15b31da8a68fc83 Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 14:12:56 +0100 Subject: [PATCH 02/10] added a produced empty block log --- block/produce.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/produce.go b/block/produce.go index c9e99b51b..7d906e874 100644 --- a/block/produce.go +++ b/block/produce.go @@ -56,6 +56,8 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { nextEmptyBlock = time.Now().Add(m.Conf.MaxIdleTime) if 0 < len(block.Data.Txs) { nextEmptyBlock = time.Now().Add(m.Conf.MaxProofTime) + } else { + m.logger.Info("produced empty block") } // Send the size to the accumulated size channel From 8c4c50552b55e0c21e99414e7a05c15afc5d429d Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 14:22:18 +0100 Subject: [PATCH 03/10] allow empty on first --- block/produce.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/produce.go b/block/produce.go index 7d906e874..88104781f 100644 --- a/block/produce.go +++ b/block/produce.go @@ -23,6 +23,7 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { defer ticker.Stop() var nextEmptyBlock time.Time + firstBlock := true for { select { @@ -31,7 +32,8 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { case <-ticker.C: // if empty blocks are configured to be enabled, and one is scheduled... - produceEmptyBlock := 0 < m.Conf.MaxProofTime && nextEmptyBlock.Before(time.Now()) + produceEmptyBlock := firstBlock || 0 < m.Conf.MaxProofTime && nextEmptyBlock.Before(time.Now()) + firstBlock = false block, commit, err := m.ProduceAndGossipBlock(ctx, produceEmptyBlock) if errors.Is(err, context.Canceled) { From bb9f5cebe9bf2a05260b9b2dc47af9c56aa0de3c Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 14:49:36 +0100 Subject: [PATCH 04/10] fix logical brace --- block/produce.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/produce.go b/block/produce.go index 88104781f..a1cf59544 100644 --- a/block/produce.go +++ b/block/produce.go @@ -32,7 +32,7 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { case <-ticker.C: // if empty blocks are configured to be enabled, and one is scheduled... - produceEmptyBlock := firstBlock || 0 < m.Conf.MaxProofTime && nextEmptyBlock.Before(time.Now()) + produceEmptyBlock := firstBlock || (0 < m.Conf.MaxProofTime && nextEmptyBlock.Before(time.Now())) firstBlock = false block, commit, err := m.ProduceAndGossipBlock(ctx, produceEmptyBlock) From 56592178bacedf44a8b28dbcb6c1d90ff5d44e31 Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 14:55:36 +0100 Subject: [PATCH 05/10] fix flipped production test --- block/production_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block/production_test.go b/block/production_test.go index fa11a781c..8b28c9ab0 100644 --- a/block/production_test.go +++ b/block/production_test.go @@ -40,15 +40,15 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { // Init manager with empty blocks feature disabled managerConfigCreatesEmptyBlocks := testutil.GetManagerConfig() managerConfigCreatesEmptyBlocks.BlockTime = blockTime - managerConfigCreatesEmptyBlocks.MaxIdleTime = 0 * time.Second + managerConfigCreatesEmptyBlocks.MaxIdleTime = MaxIdleTime + managerConfigCreatesEmptyBlocks.MaxProofTime = MaxIdleTime managerWithEmptyBlocks, err := testutil.GetManager(managerConfigCreatesEmptyBlocks, nil, nil, 1, 1, 0, proxyApp, nil) require.NoError(err) // Init manager with empty blocks feature enabled managerConfig := testutil.GetManagerConfig() managerConfig.BlockTime = blockTime - managerConfig.MaxIdleTime = MaxIdleTime - managerConfig.MaxProofTime = MaxIdleTime + managerConfig.MaxIdleTime = 0 manager, err := testutil.GetManager(managerConfig, nil, nil, 1, 1, 0, proxyApp, nil) require.NoError(err) @@ -61,8 +61,8 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { go manager.ProduceBlockLoop(mCtx) go managerWithEmptyBlocks.ProduceBlockLoop(mCtx) - buf1 := make(chan struct{}, 100) //dummy to avoid unhealthy event - buf2 := make(chan struct{}, 100) //dummy to avoid unhealthy event + buf1 := make(chan struct{}, 100) // dummy to avoid unhealthy event + buf2 := make(chan struct{}, 100) // dummy to avoid unhealthy event go manager.AccumulatedDataLoop(mCtx, buf1) go managerWithEmptyBlocks.AccumulatedDataLoop(mCtx, buf2) <-mCtx.Done() From 47ca5928d9aaab53327cc71ba5758be809f0a73c Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 15:05:17 +0100 Subject: [PATCH 06/10] fix the production test, but something is blocked --- block/production_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/block/production_test.go b/block/production_test.go index 8b28c9ab0..02ebffd93 100644 --- a/block/production_test.go +++ b/block/production_test.go @@ -25,7 +25,7 @@ import ( func TestCreateEmptyBlocksEnableDisable(t *testing.T) { const blockTime = 200 * time.Millisecond - const MaxIdleTime = blockTime * 10 + const maxIdleTime = blockTime * 10 const runTime = 10 * time.Second assert := assert.New(t) @@ -37,15 +37,15 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { err := proxyApp.Start() require.NoError(err) - // Init manager with empty blocks feature disabled + // Init manager with empty blocks feature enabled managerConfigCreatesEmptyBlocks := testutil.GetManagerConfig() managerConfigCreatesEmptyBlocks.BlockTime = blockTime - managerConfigCreatesEmptyBlocks.MaxIdleTime = MaxIdleTime - managerConfigCreatesEmptyBlocks.MaxProofTime = MaxIdleTime + managerConfigCreatesEmptyBlocks.MaxIdleTime = maxIdleTime + managerConfigCreatesEmptyBlocks.MaxProofTime = maxIdleTime managerWithEmptyBlocks, err := testutil.GetManager(managerConfigCreatesEmptyBlocks, nil, nil, 1, 1, 0, proxyApp, nil) require.NoError(err) - // Init manager with empty blocks feature enabled + // Init manager with empty blocks feature disabled managerConfig := testutil.GetManagerConfig() managerConfig.BlockTime = blockTime managerConfig.MaxIdleTime = 0 @@ -69,11 +69,11 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { require.Greater(manager.Store.Height(), initialHeight) require.Greater(managerWithEmptyBlocks.Store.Height(), initialHeight) - assert.Greater(managerWithEmptyBlocks.Store.Height(), manager.Store.Height()) + assert.GreaterOrEqual(managerWithEmptyBlocks.Store.Height(), manager.Store.Height()) // Check that blocks are created with empty blocks feature disabled - assert.LessOrEqual(manager.Store.Height(), uint64(runTime/MaxIdleTime)) - assert.LessOrEqual(managerWithEmptyBlocks.Store.Height(), uint64(runTime/blockTime)) + assert.LessOrEqual(manager.Store.Height(), uint64(runTime/blockTime)) + assert.LessOrEqual(managerWithEmptyBlocks.Store.Height(), uint64(runTime/maxIdleTime)) for i := uint64(2); i < managerWithEmptyBlocks.Store.Height(); i++ { prevBlock, err := managerWithEmptyBlocks.Store.LoadBlock(i - 1) @@ -84,8 +84,7 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { assert.NotZero(block.Header.Time) diff := time.Unix(0, int64(block.Header.Time)).Sub(time.Unix(0, int64(prevBlock.Header.Time))) - assert.Greater(diff, blockTime-blockTime/10) - assert.Less(diff, blockTime+blockTime/10) + assert.Greater(diff, manager.Conf.MaxIdleTime) } for i := uint64(2); i < manager.Store.Height(); i++ { @@ -97,7 +96,8 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { assert.NotZero(block.Header.Time) diff := time.Unix(0, int64(block.Header.Time)).Sub(time.Unix(0, int64(prevBlock.Header.Time))) - assert.Greater(diff, manager.Conf.MaxIdleTime) + assert.Greater(diff, blockTime-blockTime/10) + assert.Less(diff, blockTime+blockTime/10) } } From b5bfac6d893d6da6428948c52cb4492cdbdafc11 Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 15:09:25 +0100 Subject: [PATCH 07/10] fix if --- block/produce.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/produce.go b/block/produce.go index a1cf59544..04c254e26 100644 --- a/block/produce.go +++ b/block/produce.go @@ -32,7 +32,7 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { case <-ticker.C: // if empty blocks are configured to be enabled, and one is scheduled... - produceEmptyBlock := firstBlock || (0 < m.Conf.MaxProofTime && nextEmptyBlock.Before(time.Now())) + produceEmptyBlock := firstBlock || (0 < m.Conf.MaxIdleTime && nextEmptyBlock.Before(time.Now())) firstBlock = false block, commit, err := m.ProduceAndGossipBlock(ctx, produceEmptyBlock) From bbad26f7f2bbd7f596654741cb562726ee8e9480 Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 15:20:22 +0100 Subject: [PATCH 08/10] Revert "fix the production test, but something is blocked" This reverts commit 47ca5928d9aaab53327cc71ba5758be809f0a73c. --- block/production_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/block/production_test.go b/block/production_test.go index 02ebffd93..8b28c9ab0 100644 --- a/block/production_test.go +++ b/block/production_test.go @@ -25,7 +25,7 @@ import ( func TestCreateEmptyBlocksEnableDisable(t *testing.T) { const blockTime = 200 * time.Millisecond - const maxIdleTime = blockTime * 10 + const MaxIdleTime = blockTime * 10 const runTime = 10 * time.Second assert := assert.New(t) @@ -37,15 +37,15 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { err := proxyApp.Start() require.NoError(err) - // Init manager with empty blocks feature enabled + // Init manager with empty blocks feature disabled managerConfigCreatesEmptyBlocks := testutil.GetManagerConfig() managerConfigCreatesEmptyBlocks.BlockTime = blockTime - managerConfigCreatesEmptyBlocks.MaxIdleTime = maxIdleTime - managerConfigCreatesEmptyBlocks.MaxProofTime = maxIdleTime + managerConfigCreatesEmptyBlocks.MaxIdleTime = MaxIdleTime + managerConfigCreatesEmptyBlocks.MaxProofTime = MaxIdleTime managerWithEmptyBlocks, err := testutil.GetManager(managerConfigCreatesEmptyBlocks, nil, nil, 1, 1, 0, proxyApp, nil) require.NoError(err) - // Init manager with empty blocks feature disabled + // Init manager with empty blocks feature enabled managerConfig := testutil.GetManagerConfig() managerConfig.BlockTime = blockTime managerConfig.MaxIdleTime = 0 @@ -69,11 +69,11 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { require.Greater(manager.Store.Height(), initialHeight) require.Greater(managerWithEmptyBlocks.Store.Height(), initialHeight) - assert.GreaterOrEqual(managerWithEmptyBlocks.Store.Height(), manager.Store.Height()) + assert.Greater(managerWithEmptyBlocks.Store.Height(), manager.Store.Height()) // Check that blocks are created with empty blocks feature disabled - assert.LessOrEqual(manager.Store.Height(), uint64(runTime/blockTime)) - assert.LessOrEqual(managerWithEmptyBlocks.Store.Height(), uint64(runTime/maxIdleTime)) + assert.LessOrEqual(manager.Store.Height(), uint64(runTime/MaxIdleTime)) + assert.LessOrEqual(managerWithEmptyBlocks.Store.Height(), uint64(runTime/blockTime)) for i := uint64(2); i < managerWithEmptyBlocks.Store.Height(); i++ { prevBlock, err := managerWithEmptyBlocks.Store.LoadBlock(i - 1) @@ -84,7 +84,8 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { assert.NotZero(block.Header.Time) diff := time.Unix(0, int64(block.Header.Time)).Sub(time.Unix(0, int64(prevBlock.Header.Time))) - assert.Greater(diff, manager.Conf.MaxIdleTime) + assert.Greater(diff, blockTime-blockTime/10) + assert.Less(diff, blockTime+blockTime/10) } for i := uint64(2); i < manager.Store.Height(); i++ { @@ -96,8 +97,7 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { assert.NotZero(block.Header.Time) diff := time.Unix(0, int64(block.Header.Time)).Sub(time.Unix(0, int64(prevBlock.Header.Time))) - assert.Greater(diff, blockTime-blockTime/10) - assert.Less(diff, blockTime+blockTime/10) + assert.Greater(diff, manager.Conf.MaxIdleTime) } } From 0fa595db54e50e582f3c632f1002bf5771622215 Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 15:20:29 +0100 Subject: [PATCH 09/10] Revert "fix flipped production test" This reverts commit 56592178bacedf44a8b28dbcb6c1d90ff5d44e31. --- block/production_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block/production_test.go b/block/production_test.go index 8b28c9ab0..fa11a781c 100644 --- a/block/production_test.go +++ b/block/production_test.go @@ -40,15 +40,15 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { // Init manager with empty blocks feature disabled managerConfigCreatesEmptyBlocks := testutil.GetManagerConfig() managerConfigCreatesEmptyBlocks.BlockTime = blockTime - managerConfigCreatesEmptyBlocks.MaxIdleTime = MaxIdleTime - managerConfigCreatesEmptyBlocks.MaxProofTime = MaxIdleTime + managerConfigCreatesEmptyBlocks.MaxIdleTime = 0 * time.Second managerWithEmptyBlocks, err := testutil.GetManager(managerConfigCreatesEmptyBlocks, nil, nil, 1, 1, 0, proxyApp, nil) require.NoError(err) // Init manager with empty blocks feature enabled managerConfig := testutil.GetManagerConfig() managerConfig.BlockTime = blockTime - managerConfig.MaxIdleTime = 0 + managerConfig.MaxIdleTime = MaxIdleTime + managerConfig.MaxProofTime = MaxIdleTime manager, err := testutil.GetManager(managerConfig, nil, nil, 1, 1, 0, proxyApp, nil) require.NoError(err) @@ -61,8 +61,8 @@ func TestCreateEmptyBlocksEnableDisable(t *testing.T) { go manager.ProduceBlockLoop(mCtx) go managerWithEmptyBlocks.ProduceBlockLoop(mCtx) - buf1 := make(chan struct{}, 100) // dummy to avoid unhealthy event - buf2 := make(chan struct{}, 100) // dummy to avoid unhealthy event + buf1 := make(chan struct{}, 100) //dummy to avoid unhealthy event + buf2 := make(chan struct{}, 100) //dummy to avoid unhealthy event go manager.AccumulatedDataLoop(mCtx, buf1) go managerWithEmptyBlocks.AccumulatedDataLoop(mCtx, buf2) <-mCtx.Done() From 6b93370774585e4d9ac54f956cb8f54500e31b2b Mon Sep 17 00:00:00 2001 From: danwt <30197399+danwt@users.noreply.github.com> Date: Thu, 9 May 2024 15:23:31 +0100 Subject: [PATCH 10/10] fix if --- block/produce.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/produce.go b/block/produce.go index 04c254e26..c3328392c 100644 --- a/block/produce.go +++ b/block/produce.go @@ -32,7 +32,7 @@ func (m *Manager) ProduceBlockLoop(ctx context.Context) { case <-ticker.C: // if empty blocks are configured to be enabled, and one is scheduled... - produceEmptyBlock := firstBlock || (0 < m.Conf.MaxIdleTime && nextEmptyBlock.Before(time.Now())) + produceEmptyBlock := firstBlock || 0 == m.Conf.MaxIdleTime || nextEmptyBlock.Before(time.Now()) firstBlock = false block, commit, err := m.ProduceAndGossipBlock(ctx, produceEmptyBlock)