From 9c1e5852f64daccf2bfbd7c18d401c9d282946e2 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+shaspitz@users.noreply.github.com> Date: Sat, 4 Nov 2023 23:24:03 -0700 Subject: [PATCH 1/5] Period -> PeriodMs --- consensus/clique/clique.go | 8 +++++--- consensus/clique/snapshot_test.go | 4 ++-- miner/miner_test.go | 6 +++--- miner/stress/clique/main.go | 2 +- miner/worker.go | 4 ++-- miner/worker_test.go | 6 +++--- params/config.go | 10 +++++----- 7 files changed, 21 insertions(+), 19 deletions(-) diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index f708050abd..11cb60d830 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -329,7 +329,7 @@ func (c *Clique) verifyCascadingFields(chain consensus.ChainHeaderReader, header if parent == nil || parent.Number.Uint64() != number-1 || parent.Hash() != header.ParentHash { return consensus.ErrUnknownAncestor } - if parent.Time+c.config.Period > header.Time { + if parent.Time+c.config.PeriodMs/1000 > header.Time { return errInvalidTimestamp } // Verify that the gasUsed is <= gasLimit @@ -558,7 +558,9 @@ func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header if parent == nil { return consensus.ErrUnknownAncestor } - header.Time = parent.Time + c.config.Period + // TODO: Need to handle that sub 1 second periods will never increment header.Time. + // Or maybe next line handles that? + header.Time = parent.Time + c.config.PeriodMs/1000 if header.Time < uint64(time.Now().Unix()) { header.Time = uint64(time.Now().Unix()) } @@ -608,7 +610,7 @@ func (c *Clique) Seal(chain consensus.ChainHeaderReader, block *types.Block, res return errUnknownBlock } // For 0-period chains, refuse to seal empty blocks (no reward but would spin sealing) - if c.config.Period == 0 && len(block.Transactions()) == 0 { + if c.config.PeriodMs == 0 && len(block.Transactions()) == 0 { return errors.New("sealing paused while waiting for transactions") } // Don't hold the signer fields for the entire sealing procedure diff --git a/consensus/clique/snapshot_test.go b/consensus/clique/snapshot_test.go index 26cebe008a..370b8f9b54 100644 --- a/consensus/clique/snapshot_test.go +++ b/consensus/clique/snapshot_test.go @@ -414,8 +414,8 @@ func (tt *cliqueTest) run(t *testing.T) { // Assemble a chain of headers from the cast votes config := *params.TestChainConfig config.Clique = ¶ms.CliqueConfig{ - Period: 1, - Epoch: tt.epoch, + PeriodMs: 1000, + Epoch: tt.epoch, } genesis.Config = &config diff --git a/miner/miner_test.go b/miner/miner_test.go index 36d5166c6d..56cb5ec6f8 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -261,8 +261,8 @@ func waitForMiningState(t *testing.T, m *Miner, mining bool) { func minerTestGenesisBlock(period uint64, gasLimit uint64, faucet common.Address) *core.Genesis { config := *params.AllCliqueProtocolChanges config.Clique = ¶ms.CliqueConfig{ - Period: period, - Epoch: config.Clique.Epoch, + PeriodMs: period, + Epoch: config.Clique.Epoch, } // Assemble and return the genesis with the precompiles and faucet pre-funded @@ -294,7 +294,7 @@ func createMiner(t *testing.T) (*Miner, *event.TypeMux, func(skipMiner bool)) { // Create chainConfig chainDB := rawdb.NewMemoryDatabase() triedb := trie.NewDatabase(chainDB, nil) - genesis := minerTestGenesisBlock(15, 11_500_000, common.HexToAddress("12345")) + genesis := minerTestGenesisBlock(15000, 11_500_000, common.HexToAddress("12345")) chainConfig, _, err := core.SetupGenesisBlock(chainDB, triedb, genesis) if err != nil { t.Fatalf("can't create new chain config: %v", err) diff --git a/miner/stress/clique/main.go b/miner/stress/clique/main.go index 7b29e63dfc..b6a0e997ca 100644 --- a/miner/stress/clique/main.go +++ b/miner/stress/clique/main.go @@ -152,7 +152,7 @@ func makeGenesis(faucets []*ecdsa.PrivateKey, sealers []*ecdsa.PrivateKey) *core genesis.GasLimit = 25000000 genesis.Config.ChainID = big.NewInt(18) - genesis.Config.Clique.Period = 1 + genesis.Config.Clique.PeriodMs = 1000 genesis.Alloc = core.GenesisAlloc{} for _, faucet := range faucets { diff --git a/miner/worker.go b/miner/worker.go index f680702814..fd4708ddb0 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -461,7 +461,7 @@ func (w *worker) newWorkLoop(recommit time.Duration) { case <-timer.C: // If sealing is running resubmit a new work cycle periodically to pull in // higher priced transactions. Disable this overhead for pending blocks. - if w.isRunning() && (w.chainConfig.Clique == nil || w.chainConfig.Clique.Period > 0) { + if w.isRunning() && (w.chainConfig.Clique == nil || w.chainConfig.Clique.PeriodMs > 0) { // Short circuit if no new transaction arrives. if w.newTxs.Load() == 0 { timer.Reset(recommit) @@ -565,7 +565,7 @@ func (w *worker) mainLoop() { // Special case, if the consensus engine is 0 period clique(dev mode), // submit sealing work here since all empty submission will be rejected // by clique. Of course the advance sealing(empty submission) is disabled. - if w.chainConfig.Clique != nil && w.chainConfig.Clique.Period == 0 { + if w.chainConfig.Clique != nil && w.chainConfig.Clique.PeriodMs == 0 { w.commitWork(nil, time.Now().Unix()) } } diff --git a/miner/worker_test.go b/miner/worker_test.go index 9c4694c0e2..6dc5cdb612 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -80,8 +80,8 @@ func init() { cliqueChainConfig = new(params.ChainConfig) *cliqueChainConfig = *params.TestChainConfig cliqueChainConfig.Clique = ¶ms.CliqueConfig{ - Period: 10, - Epoch: 30000, + PeriodMs: 10000, + Epoch: 30000, } signer := types.LatestSigner(params.TestChainConfig) @@ -171,7 +171,7 @@ func TestGenerateAndImportBlock(t *testing.T) { db = rawdb.NewMemoryDatabase() config = *params.AllCliqueProtocolChanges ) - config.Clique = ¶ms.CliqueConfig{Period: 1, Epoch: 30000} + config.Clique = ¶ms.CliqueConfig{PeriodMs: 1000, Epoch: 30000} engine := clique.New(config.Clique, db) w, b := newTestWorker(t, &config, engine, db, 0) diff --git a/params/config.go b/params/config.go index 88ff772a1d..e5d5cf6fab 100644 --- a/params/config.go +++ b/params/config.go @@ -128,8 +128,8 @@ var ( TerminalTotalDifficultyPassed: true, ShanghaiTime: newUint64(1678832736), Clique: &CliqueConfig{ - Period: 15, - Epoch: 30000, + PeriodMs: 15000, + Epoch: 30000, }, } // AllEthashProtocolChanges contains every protocol change (EIPs) introduced @@ -210,7 +210,7 @@ var ( TerminalTotalDifficulty: nil, TerminalTotalDifficultyPassed: false, Ethash: nil, - Clique: &CliqueConfig{Period: 0, Epoch: 30000}, + Clique: &CliqueConfig{PeriodMs: 0, Epoch: 30000}, } // TestChainConfig contains every protocol change (EIPs) introduced @@ -344,8 +344,8 @@ func (c *EthashConfig) String() string { // CliqueConfig is the consensus engine configs for proof-of-authority based sealing. type CliqueConfig struct { - Period uint64 `json:"period"` // Number of seconds between blocks to enforce - Epoch uint64 `json:"epoch"` // Epoch length to reset votes and checkpoint + PeriodMs uint64 `json:"period"` // Number of ms between blocks to enforce + Epoch uint64 `json:"epoch"` // Epoch length to reset votes and checkpoint } // String implements the stringer interface, returning the consensus engine details. From 01664986e2fcb9463a7911c8a963005c5da9e43f Mon Sep 17 00:00:00 2001 From: Shawn <44221603+shaspitz@users.noreply.github.com> Date: Sun, 5 Nov 2023 01:25:37 -0700 Subject: [PATCH 2/5] logs + handle delay being shorter than period --- consensus/clique/clique.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index 11cb60d830..ed4e95bdc4 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -500,6 +500,7 @@ func (c *Clique) verifySeal(snap *Snapshot, header *types.Header, parents []*typ // Prepare implements consensus.Engine, preparing all the consensus fields of the // header for running the transactions on top. func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error { + log.Trace("start of prepare") // If the block isn't a checkpoint, cast a random vote (good enough for now) header.Coinbase = common.Address{} header.Nonce = types.BlockNonce{} @@ -602,6 +603,7 @@ func (c *Clique) Authorize(signer common.Address, signFn SignerFn) { // Seal implements consensus.Engine, attempting to create a sealed block using // the local signing credentials. func (c *Clique) Seal(chain consensus.ChainHeaderReader, block *types.Block, results chan<- *types.Block, stop <-chan struct{}) error { + log.Trace("start of seal") header := block.Header() // Sealing the genesis block is not supported @@ -650,6 +652,14 @@ func (c *Clique) Seal(chain consensus.ChainHeaderReader, block *types.Block, res return err } copy(header.Extra[len(header.Extra)-extraSeal:], sighash) + + // Don't let delay be longer than target block period + if delay.Milliseconds() > int64(c.config.PeriodMs) { + log.Trace("old delay", "delay", common.PrettyDuration(delay)) + delay = time.Millisecond*time.Duration(c.config.PeriodMs) - 20*time.Millisecond + log.Trace("new delay", "delay", common.PrettyDuration(delay)) + } + // Wait until sealing is terminated or delay timeout. log.Trace("Waiting for slot to sign and propagate", "delay", common.PrettyDuration(delay)) go func() { From 9bb1a7f0034002e79c4a91406ea3828ad3e4627f Mon Sep 17 00:00:00 2001 From: Shawn <44221603+shaspitz@users.noreply.github.com> Date: Sun, 5 Nov 2023 01:59:39 -0700 Subject: [PATCH 3/5] lets try hardcoding delay --- consensus/clique/clique.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index ed4e95bdc4..db13d1f602 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -654,11 +654,13 @@ func (c *Clique) Seal(chain consensus.ChainHeaderReader, block *types.Block, res copy(header.Extra[len(header.Extra)-extraSeal:], sighash) // Don't let delay be longer than target block period - if delay.Milliseconds() > int64(c.config.PeriodMs) { - log.Trace("old delay", "delay", common.PrettyDuration(delay)) - delay = time.Millisecond*time.Duration(c.config.PeriodMs) - 20*time.Millisecond - log.Trace("new delay", "delay", common.PrettyDuration(delay)) - } + // if delay.Milliseconds() > int64(c.config.PeriodMs) { + // log.Trace("old delay", "delay", common.PrettyDuration(delay)) + // delay = time.Millisecond*time.Duration(c.config.PeriodMs) - 20*time.Millisecond + // log.Trace("new delay", "delay", common.PrettyDuration(delay)) + // } + + delay = time.Millisecond * (time.Duration(c.config.PeriodMs - 20)) // Wait until sealing is terminated or delay timeout. log.Trace("Waiting for slot to sign and propagate", "delay", common.PrettyDuration(delay)) From 402e5d2039ad595b3cc77ef2f35b022bdc3d7402 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+shaspitz@users.noreply.github.com> Date: Wed, 15 Nov 2023 12:17:16 -0800 Subject: [PATCH 4/5] cleans --- consensus/clique/clique.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index db13d1f602..512e9dceec 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -559,8 +559,6 @@ func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header if parent == nil { return consensus.ErrUnknownAncestor } - // TODO: Need to handle that sub 1 second periods will never increment header.Time. - // Or maybe next line handles that? header.Time = parent.Time + c.config.PeriodMs/1000 if header.Time < uint64(time.Now().Unix()) { header.Time = uint64(time.Now().Unix()) @@ -603,7 +601,6 @@ func (c *Clique) Authorize(signer common.Address, signFn SignerFn) { // Seal implements consensus.Engine, attempting to create a sealed block using // the local signing credentials. func (c *Clique) Seal(chain consensus.ChainHeaderReader, block *types.Block, results chan<- *types.Block, stop <-chan struct{}) error { - log.Trace("start of seal") header := block.Header() // Sealing the genesis block is not supported @@ -653,13 +650,8 @@ func (c *Clique) Seal(chain consensus.ChainHeaderReader, block *types.Block, res } copy(header.Extra[len(header.Extra)-extraSeal:], sighash) - // Don't let delay be longer than target block period - // if delay.Milliseconds() > int64(c.config.PeriodMs) { - // log.Trace("old delay", "delay", common.PrettyDuration(delay)) - // delay = time.Millisecond*time.Duration(c.config.PeriodMs) - 20*time.Millisecond - // log.Trace("new delay", "delay", common.PrettyDuration(delay)) - // } - + // For now, hard code delay to 20ms less than block period. + // This may cause deadlocks with certain numbers of nodes, more testing is needed. delay = time.Millisecond * (time.Duration(c.config.PeriodMs - 20)) // Wait until sealing is terminated or delay timeout. From 539e85345569f9cbe0f14b7a7e3dadb33e5b0f12 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+shaspitz@users.noreply.github.com> Date: Wed, 15 Nov 2023 12:18:04 -0800 Subject: [PATCH 5/5] Update clique.go --- consensus/clique/clique.go | 1 - 1 file changed, 1 deletion(-) diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index 512e9dceec..85cd2009d2 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -500,7 +500,6 @@ func (c *Clique) verifySeal(snap *Snapshot, header *types.Header, parents []*typ // Prepare implements consensus.Engine, preparing all the consensus fields of the // header for running the transactions on top. func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error { - log.Trace("start of prepare") // If the block isn't a checkpoint, cast a random vote (good enough for now) header.Coinbase = common.Address{} header.Nonce = types.BlockNonce{}