From 88e2caaac4d65ca7f8d86a542c295c4f4eb47388 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 25 Nov 2020 09:46:40 +0100 Subject: [PATCH] core: change to one callback per batch of inserted headers + review concerns --- core/headerchain.go | 42 ++++++++++++++++------------------------ core/headerchain_test.go | 20 +++++++++---------- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/core/headerchain.go b/core/headerchain.go index e196564dc856..5ea478f5e42a 100644 --- a/core/headerchain.go +++ b/core/headerchain.go @@ -132,7 +132,6 @@ func (hc *HeaderChain) GetBlockNumber(hash common.Hash) *uint64 { type headerWriteResult struct { ignored int imported int - status WriteStatus lastHash common.Hash lastHeader *types.Header } @@ -155,12 +154,12 @@ func (hc *HeaderChain) WriteHeaders(headers []*types.Header, postWriteFn PostWri return &headerWriteResult{}, consensus.ErrUnknownAncestor } var ( - lastHeader *types.Header // Last successfully imported header - lastNumber uint64 // Last successfully imported number - lastHash common.Hash - externTd *big.Int // TD of successfully imported chain - inserted []numberHash // Ephemeral lookup of number/hash for the chain - firstInsertedIndex = -1 // Index of the first non-ignored header + lastHeader *types.Header // Last successfully imported header + lastNumber uint64 // Last successfully imported number + lastHash common.Hash + externTd *big.Int // TD of successfully imported chain + inserted []numberHash // Ephemeral lookup of number/hash for the chain + firstInserted = -1 // Index of the first non-ignored header ) lastHash, lastNumber = headers[0].ParentHash, headers[0].Number.Uint64()-1 // Already validated above batch := hc.chainDb.NewBatch() @@ -178,7 +177,7 @@ func (hc *HeaderChain) WriteHeaders(headers []*types.Header, postWriteFn PostWri } hash, number := header.Hash(), header.Number.Uint64() if header.ParentHash != lastHash { - log.Warn("Non-contiguous header insertion", "header.parent", header.ParentHash, "expected", hash, "number", number) + log.Warn("Non-contiguous header insertion", "parent", header.ParentHash, "expected", lastHash, "number", number) break } externTd = new(big.Int).Add(header.Difficulty, ptd) @@ -193,8 +192,8 @@ func (hc *HeaderChain) WriteHeaders(headers []*types.Header, postWriteFn PostWri inserted = append(inserted, numberHash{number, hash}) hc.headerCache.Add(hash, header) hc.numberCache.Add(hash, number) - if firstInsertedIndex < 0 { - firstInsertedIndex = i + if firstInserted < 0 { + firstInserted = i } } lastHeader, lastHash, lastNumber, ptd = header, hash, number, externTd @@ -253,7 +252,7 @@ func (hc *HeaderChain) WriteHeaders(headers []*types.Header, postWriteFn PostWri // during this import batch, then we need to write that now // Further down, we continue writing the staus for the ones that // were not already known - for i := 0; i < firstInsertedIndex; i++ { + for i := 0; i < firstInserted; i++ { hash := headers[i].Hash() num := headers[i].Number.Uint64() rawdb.WriteCanonicalHash(markerBatch, hash, num) @@ -279,30 +278,23 @@ func (hc *HeaderChain) WriteHeaders(headers []*types.Header, postWriteFn PostWri // Execute any post-write callback function // - unless we're exiting // - and unless we ignored everything - if postWriteFn != nil && !hc.procInterrupt() && firstInsertedIndex >= 0 { - // TODO: Is it really necessary to invoke this N times, instead of just - // invoking it for the last header? - // It is only used for lightchain event aggregation - for _, header := range headers[firstInsertedIndex:] { - if header.Number.Uint64() > lastNumber { - break - } - postWriteFn(header, status) - } + if postWriteFn != nil && !hc.procInterrupt() && len(inserted) > 0 { + // The postwrite function is called only for the last imported header. + // It is only used for lightchain event aggregation. + postWriteFn(lastHeader, status) } return &headerWriteResult{ ignored: len(headers) - len(inserted), imported: len(inserted), - status: status, lastHash: lastHash, lastHeader: lastHeader, }, nil } // PostWriteCallback is a callback function for inserting headers, -// which is called after each header is inserted. -// The reson for having it is: -// In light-chain mode, status should be processed and light chain events sent, +// which is called once, with the last successfully imported header in the batch. +// The raeson for having it is: +// In light-chain mode, status should be processed and light chain events sent, // whereas in a non-light mode this is not necessary since chain events are sent after inserting blocks. type PostWriteCallback func(header *types.Header, status WriteStatus) diff --git a/core/headerchain_test.go b/core/headerchain_test.go index 9bd068bc6449..d55c54050ced 100644 --- a/core/headerchain_test.go +++ b/core/headerchain_test.go @@ -98,23 +98,23 @@ func TestHeaderInsertion(t *testing.T) { log.Root().SetHandler(log.StdoutHandler) // Inserting 64 headers on an empty chain, expecting - // 64 callbacks, 64 canon-status, 0 sidestatus, - if err := testInsert(t, hc, chainA[:64], 64, 64, 0); err != nil { + // 1 callbacks, 1 canon-status, 0 sidestatus, + if err := testInsert(t, hc, chainA[:64], 1, 1, 0); err != nil { t.Fatal(err) } - // Inserting 64 inentical headers, expecting + // Inserting 64 identical headers, expecting // 0 callbacks, 0 canon-status, 0 sidestatus, if err := testInsert(t, hc, chainA[:64], 0, 0, 0); err != nil { t.Fatal(err) } // Inserting the same some old, some new headers - // 32 callbacks, 32 canon, 0 side - if err := testInsert(t, hc, chainA[32:96], 32, 32, 0); err != nil { + // 1 callbacks, 1 canon, 0 side + if err := testInsert(t, hc, chainA[32:96], 1, 1, 0); err != nil { t.Fatal(err) } // Inserting side blocks, but not overtaking the canon chain - if err := testInsert(t, hc, chainB[0:32], 32, 0, 32); err != nil { + if err := testInsert(t, hc, chainB[0:32], 1, 0, 1); err != nil { t.Fatal(err) } // Inserting more side blocks, but we don't have the parent @@ -122,19 +122,19 @@ func TestHeaderInsertion(t *testing.T) { t.Fatal(fmt.Errorf("Expected %v, got %v", consensus.ErrUnknownAncestor, err)) } // Inserting more sideblocks, overtaking the canon chain - if err := testInsert(t, hc, chainB[32:97], 65, 65, 0); err != nil { + if err := testInsert(t, hc, chainB[32:97], 1, 1, 0); err != nil { t.Fatal(err) } // Inserting more A-headers, taking back the canonicality - if err := testInsert(t, hc, chainA[90:100], 4, 4, 0); err != nil { + if err := testInsert(t, hc, chainA[90:100], 1, 1, 0); err != nil { t.Fatal(err) } // And B becomes canon again - if err := testInsert(t, hc, chainB[97:107], 10, 10, 0); err != nil { + if err := testInsert(t, hc, chainB[97:107], 1, 1, 0); err != nil { t.Fatal(err) } // And B becomes even longer - if err := testInsert(t, hc, chainB[107:128], 21, 21, 0); err != nil { + if err := testInsert(t, hc, chainB[107:128], 1, 1, 0); err != nil { t.Fatal(err) } }