From ae3bdea794402dd22d74a307bef07bf4d6850027 Mon Sep 17 00:00:00 2001 From: cryyl <1226241521@qq.com> Date: Thu, 5 May 2022 19:10:52 +0800 Subject: [PATCH] fix to resolve comments Signed-off-by: cryyl <1226241521@qq.com> --- cmd/utils/flags.go | 4 +-- core/block_validator.go | 1 - core/blockchain.go | 49 +++++++++++++++------------------ core/blockchain_diff_test.go | 2 +- core/blockchain_notries_test.go | 2 +- core/remote_state_verifier.go | 31 +++++++++++++-------- core/state/state_object.go | 4 ++- core/state/statedb.go | 12 ++------ eth/handler.go | 3 +- eth/protocols/trust/handler.go | 2 +- internal/ethapi/api.go | 4 +-- 11 files changed, 54 insertions(+), 60 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 79ad12dc33..f3c37b8f38 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -275,7 +275,7 @@ var ( defaultVerifyMode = ethconfig.Defaults.TriesVerifyMode TriesVerifyModeFlag = TextMarshalerFlag{ Name: "tries-verify-mode", - Usage: `tries verify mode: "local", "full", "insecure", "none"`, + Usage: `tries verify mode: "local: a normal full node", "full: state verification by verify node which has diffLayer of blocks", "insecure: state verification by verify node which has no diffLayer of blocks", "none: no state verification"`, Value: &defaultVerifyMode, } OverrideBerlinFlag = cli.Uint64Flag{ @@ -1688,7 +1688,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { } if ctx.GlobalIsSet(TriesVerifyModeFlag.Name) { cfg.TriesVerifyMode = *GlobalTextMarshaler(ctx, TriesVerifyModeFlag.Name).(*core.VerifyMode) - // If a node sets verify mode to full or light, it's a fast node and need + // If a node sets verify mode to full or insecure, it's a fast node and need // to verify blocks from verify nodes, then it should enable trust protocol. if cfg.TriesVerifyMode.NeedRemoteVerify() { cfg.EnableTrustProtocol = true diff --git a/core/block_validator.go b/core/block_validator.go index 876c6e43a2..85e016df84 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -110,7 +110,6 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if v.remoteValidator != nil && !v.remoteValidator.AncestorVerified(block.Header()) { return fmt.Errorf("%w, number: %s, hash: %s", ErrAncestorHasNotBeenVerified, block.Number(), block.Hash()) } - return nil }, } diff --git a/core/blockchain.go b/core/blockchain.go index 465235fdd2..a11d08b05d 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -28,9 +28,8 @@ import ( "sync/atomic" "time" - "golang.org/x/crypto/sha3" - lru "github.com/hashicorp/golang-lru" + "golang.org/x/crypto/sha3" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/mclock" @@ -509,31 +508,27 @@ func (bc *BlockChain) cacheReceipts(hash common.Hash, receipts types.Receipts) { bc.receiptsCache.Add(hash, receipts) } -func (bc *BlockChain) cacheDiffLayer(diffLayer *types.DiffLayer, sorted bool) { - if !sorted { - sort.SliceStable(diffLayer.Codes, func(i, j int) bool { - return diffLayer.Codes[i].Hash.Hex() < diffLayer.Codes[j].Hash.Hex() - }) - sort.SliceStable(diffLayer.Destructs, func(i, j int) bool { - return diffLayer.Destructs[i].Hex() < (diffLayer.Destructs[j].Hex()) - }) - sort.SliceStable(diffLayer.Accounts, func(i, j int) bool { - return diffLayer.Accounts[i].Account.Hex() < diffLayer.Accounts[j].Account.Hex() - }) - sort.SliceStable(diffLayer.Storages, func(i, j int) bool { - return diffLayer.Storages[i].Account.Hex() < diffLayer.Storages[j].Account.Hex() - }) - } +func (bc *BlockChain) cacheDiffLayer(diffLayer *types.DiffLayer, diffLayerCh chan struct{}) { + sort.SliceStable(diffLayer.Codes, func(i, j int) bool { + return diffLayer.Codes[i].Hash.Hex() < diffLayer.Codes[j].Hash.Hex() + }) + sort.SliceStable(diffLayer.Destructs, func(i, j int) bool { + return diffLayer.Destructs[i].Hex() < (diffLayer.Destructs[j].Hex()) + }) + sort.SliceStable(diffLayer.Accounts, func(i, j int) bool { + return diffLayer.Accounts[i].Account.Hex() < diffLayer.Accounts[j].Account.Hex() + }) + sort.SliceStable(diffLayer.Storages, func(i, j int) bool { + return diffLayer.Storages[i].Account.Hex() < diffLayer.Storages[j].Account.Hex() + }) if bc.diffLayerCache.Len() >= diffLayerCacheLimit { bc.diffLayerCache.RemoveOldest() } bc.diffLayerCache.Add(diffLayer.BlockHash, diffLayer) - if cached, ok := bc.diffLayerChanCache.Get(diffLayer.BlockHash); ok { - diffLayerCh := cached.(chan struct{}) - close(diffLayerCh) - } + close(diffLayerCh) + if bc.db.DiffStore() != nil { // push to priority queue before persisting bc.diffQueueBuffer <- diffLayer @@ -1840,7 +1835,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. } bc.diffLayerChanCache.Add(diffLayer.BlockHash, diffLayerCh) - go bc.cacheDiffLayer(diffLayer, false) + go bc.cacheDiffLayer(diffLayer, diffLayerCh) } wg.Wait() @@ -2156,7 +2151,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er return it.index, err } if statedb.NoTrie() { - statedb.SetCurrentRoot(block.Root()) + statedb.SetExpectedStateRoot(block.Root()) } bc.updateHighestVerifiedHeader(block.Header()) @@ -2833,10 +2828,10 @@ func (bc *BlockChain) HandleDiffLayer(diffLayer *types.DiffLayer, pid string, fu return nil } - diffHash := common.Hash{} - if diffLayer.DiffHash.Load() != nil { - diffHash = diffLayer.DiffHash.Load().(common.Hash) + if diffLayer.DiffHash.Load() == nil { + return fmt.Errorf("unexpected difflayer which diffHash is nil from peeer %s", pid) } + diffHash := diffLayer.DiffHash.Load().(common.Hash) bc.diffMux.Lock() defer bc.diffMux.Unlock() @@ -3178,7 +3173,7 @@ func EnableBlockValidator(chainConfig *params.ChainConfig, engine consensus.Engi } } -func (bc *BlockChain) GetRootByDiffHash(blockNumber uint64, blockHash common.Hash, diffHash common.Hash) *VerifyResult { +func (bc *BlockChain) GetVerifyResult(blockNumber uint64, blockHash common.Hash, diffHash common.Hash) *VerifyResult { var res VerifyResult res.BlockNumber = blockNumber res.BlockHash = blockHash diff --git a/core/blockchain_diff_test.go b/core/blockchain_diff_test.go index fad8fe353a..7e65ac6d79 100644 --- a/core/blockchain_diff_test.go +++ b/core/blockchain_diff_test.go @@ -613,7 +613,7 @@ func testGetRootByDiffHash(t *testing.T, chain1, chain2 *BlockChain, blockNumber chain1.diffLayerCache.Remove(block1.Hash()) } - result := chain1.GetRootByDiffHash(blockNumber, block2.Hash(), diffHash2) + result := chain1.GetVerifyResult(blockNumber, block2.Hash(), diffHash2) if result.Status != expect.Status { t.Fatalf("failed to verify block, number: %v, expect status: %v, real status: %v", blockNumber, expect.Status, result.Status) } diff --git a/core/blockchain_notries_test.go b/core/blockchain_notries_test.go index b4fa305888..e439fd709f 100644 --- a/core/blockchain_notries_test.go +++ b/core/blockchain_notries_test.go @@ -161,7 +161,7 @@ func makeTestBackendWithRemoteValidator(blocks int, mode VerifyMode, failed *ver peer.setCallBack(func(req *requestRoot) { if fastnode.validator != nil && fastnode.validator.RemoteVerifyManager() != nil { - resp := verifier.GetRootByDiffHash(req.blockNumber, req.blockHash, req.diffHash) + resp := verifier.GetVerifyResult(req.blockNumber, req.blockHash, req.diffHash) if failed != nil && req.blockNumber == failed.blockNumber { resp.Status = failed.status } else { diff --git a/core/remote_state_verifier.go b/core/remote_state_verifier.go index d23715bea1..3875a25cea 100644 --- a/core/remote_state_verifier.go +++ b/core/remote_state_verifier.go @@ -29,6 +29,8 @@ const ( resendInterval = 2 * time.Second // tryAllPeersTime is the time that a block has not been verified and then try all the valid verify peers. tryAllPeersTime = 15 * time.Second + // maxWaitVerifyResultTime is the max time of waiting for ancestor's verify result. + maxWaitVerifyResultTime = 30 * time.Second ) var ( @@ -111,22 +113,18 @@ func (vm *remoteVerifyManager) mainLoop() { vm.cacheBlockVerified(hash) vm.taskLock.Lock() if task, ok := vm.tasks[hash]; ok { - delete(vm.tasks, hash) - verifyTaskCounter.Dec(1) + vm.CloseTask(task) verifyTaskSucceedMeter.Mark(1) verifyTaskExecutionTimer.Update(time.Since(task.startAt)) - task.Close() } vm.taskLock.Unlock() case <-pruneTicker.C: vm.taskLock.Lock() - for hash, task := range vm.tasks { + for _, task := range vm.tasks { if vm.bc.insertStopped() || (vm.bc.CurrentHeader().Number.Cmp(task.blockHeader.Number) == 1 && vm.bc.CurrentHeader().Number.Uint64()-task.blockHeader.Number.Uint64() > pruneHeightDiff) { - delete(vm.tasks, hash) - verifyTaskCounter.Dec(1) + vm.CloseTask(task) verifyTaskFailedMeter.Mark(1) - task.Close() } } vm.taskLock.Unlock() @@ -180,7 +178,6 @@ func (vm *remoteVerifyManager) NewBlockVerifyTask(header *types.Header) { if cached, ok := vm.bc.diffLayerChanCache.Get(hash); ok { diffLayerCh := cached.(chan struct{}) <-diffLayerCh - vm.bc.diffLayerChanCache.Remove(hash) diffLayer = vm.bc.GetTrustedDiffLayer(hash) } // if this block has no diff, there is no need to verify it. @@ -225,8 +222,13 @@ func (vm *remoteVerifyManager) AncestorVerified(header *types.Header) bool { vm.taskLock.RLock() task, exist := vm.tasks[hash] vm.taskLock.RUnlock() + timeout := time.After(maxWaitVerifyResultTime) if exist { - <-task.terminalCh + select { + case <-task.terminalCh: + case <-timeout: + return false + } } _, exist = vm.verifiedCache.Get(hash) @@ -238,6 +240,12 @@ func (vm *remoteVerifyManager) HandleRootResponse(vr *VerifyResult, pid string) return nil } +func (vm *remoteVerifyManager) CloseTask(task *verifyTask) { + delete(vm.tasks, task.blockHeader.Hash()) + task.Close() + verifyTaskCounter.Dec(1) +} + type VerifyResult struct { Status types.VerifyStatus BlockNumber uint64 @@ -335,7 +343,7 @@ func (vt *verifyTask) sendVerifyRequest(n int) { // if has not valid peer, log warning. if len(validPeers) == 0 { log.Warn("there is no valid peer for block", "number", vt.blockHeader.Number) - vt.Close() + return } if n < len(validPeers) && n > 0 { @@ -352,9 +360,8 @@ func (vt *verifyTask) sendVerifyRequest(n int) { func (vt *verifyTask) compareRootHashAndMark(msg verifyMessage, verifyCh chan common.Hash) { if msg.verifyResult.Root == vt.blockHeader.Root { - blockhash := msg.verifyResult.BlockHash // write back to manager so that manager can cache the result and delete this task. - verifyCh <- blockhash + verifyCh <- msg.verifyResult.BlockHash } else { vt.badPeers[msg.peerId] = struct{}{} } diff --git a/core/state/state_object.go b/core/state/state_object.go index 0263d8d64b..b6b550de4d 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -32,6 +32,8 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) +const snapshotStaleRetryInterval = time.Millisecond * 10 + var emptyCodeHash = crypto.Keccak256(nil) type Code []byte @@ -284,7 +286,7 @@ func (s *StateObject) GetCommittedState(db Database, key common.Hash) common.Has // there is a small chance that the difflayer of the stale will be read while reading, // resulting in an empty array being returned here. // Therefore, noTrie mode must retry here, - // and add a time interval when retrying to avoid stacking too much and causing OOM. + // and add a time interval when retrying to avoid stacking too much and causing stack overflow. time.Sleep(snapshotStaleRetryInterval) return s.GetCommittedState(db, key) } diff --git a/core/state/statedb.go b/core/state/statedb.go index 4c2e12f0b6..3179a2c17c 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -39,10 +39,7 @@ import ( "github.com/ethereum/go-ethereum/trie" ) -const ( - defaultNumOfSlots = 100 - snapshotStaleRetryInterval = time.Millisecond * 100 -) +const defaultNumOfSlots = 100 type revision struct { id int @@ -81,7 +78,6 @@ type StateDB struct { prefetcherLock sync.Mutex prefetcher *triePrefetcher originalRoot common.Hash // The pre-state root, before any changes were made - currentRoot common.Hash // only used when noTrie is true expectedRoot common.Hash // The state root in the block header stateRoot common.Hash // The calculation result of IntermediateRoot @@ -276,10 +272,6 @@ func (s *StateDB) NoTrie() bool { return s.noTrie } -func (s *StateDB) SetCurrentRoot(root common.Hash) { - s.currentRoot = root -} - func (s *StateDB) Error() error { return s.dbErr } @@ -1184,7 +1176,7 @@ func (s *StateDB) StateIntermediateRoot() common.Hash { defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) } if s.noTrie { - return s.currentRoot + return s.expectedRoot } else { return s.trie.Hash() } diff --git a/eth/handler.go b/eth/handler.go index 82f7cd611e..8db505b343 100644 --- a/eth/handler.go +++ b/eth/handler.go @@ -24,8 +24,6 @@ import ( "sync/atomic" "time" - "github.com/ethereum/go-ethereum/eth/protocols/trust" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/forkid" @@ -35,6 +33,7 @@ import ( "github.com/ethereum/go-ethereum/eth/protocols/diff" "github.com/ethereum/go-ethereum/eth/protocols/eth" "github.com/ethereum/go-ethereum/eth/protocols/snap" + "github.com/ethereum/go-ethereum/eth/protocols/trust" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/log" diff --git a/eth/protocols/trust/handler.go b/eth/protocols/trust/handler.go index 9b93d4a228..f10aff5178 100644 --- a/eth/protocols/trust/handler.go +++ b/eth/protocols/trust/handler.go @@ -126,7 +126,7 @@ func handleRootRequest(backend Backend, msg Decoder, peer *Peer) error { return fmt.Errorf("%w: message %v: %v", errDecode, msg, err) } - res := backend.Chain().GetRootByDiffHash(req.BlockNumber, req.BlockHash, req.DiffHash) + res := backend.Chain().GetVerifyResult(req.BlockNumber, req.BlockHash, req.DiffHash) return p2p.Send(peer.rw, RespondRootMsg, RootResponsePacket{ RequestId: req.RequestId, Status: res.Status, diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index db16a05338..3309542581 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1287,8 +1287,8 @@ func (s *PublicBlockChainAPI) GetDiffAccountsWithScope(ctx context.Context, bloc return result, err } -func (s *PublicBlockChainAPI) GetRootByDiffHash(ctx context.Context, blockNr rpc.BlockNumber, blockHash common.Hash, diffHash common.Hash) *core.VerifyResult { - return s.b.Chain().GetRootByDiffHash(uint64(blockNr), blockHash, diffHash) +func (s *PublicBlockChainAPI) GetVerifyResult(ctx context.Context, blockNr rpc.BlockNumber, blockHash common.Hash, diffHash common.Hash) *core.VerifyResult { + return s.b.Chain().GetVerifyResult(uint64(blockNr), blockHash, diffHash) } // ExecutionResult groups all structured logs emitted by the EVM