fix to resolve comments

Signed-off-by: cryyl <1226241521@qq.com>

Author: cryyl

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, full secure mode", "insecure: state verification by verify node but insecure mode", "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

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
 		},
 	}

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

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)
 	}

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 {

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
+	// taskTerminalTimeout is the timeout time for a task termination
+	taskTerminalTimeout = 30 * time.Second
 )
 
 var (
@@ -110,23 +112,21 @@ func (vm *remoteVerifyManager) mainLoop() {
 		case hash := <-vm.verifyCh:
 			vm.cacheBlockVerified(hash)
 			vm.taskLock.Lock()
-			if task, ok := vm.tasks[hash]; ok {
-				delete(vm.tasks, hash)
-				verifyTaskCounter.Dec(1)
+			if _, ok := vm.tasks[hash]; ok {
+				vm.DeleteAndCloseTask(task)
 				verifyTaskSucceedMeter.Mark(1)
+				verifyTaskCounter.Dec(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()
+					verifyTaskCounter.Dec(1)
 				}
 			}
 			vm.taskLock.Unlock()
@@ -180,7 +180,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 +224,13 @@ func (vm *remoteVerifyManager) AncestorVerified(header *types.Header) bool {
 	vm.taskLock.RLock()
 	task, exist := vm.tasks[hash]
 	vm.taskLock.RUnlock()
+	timeout := time.After(taskTerminalTimeout)
 	if exist {
-		<-task.terminalCh
+		select {
+		case <-task.terminalCh:
+		case <-timeout:
+			return false
+		}
 	}
 
 	_, exist = vm.verifiedCache.Get(hash)
@@ -238,6 +242,11 @@ func (vm *remoteVerifyManager) HandleRootResponse(vr *VerifyResult, pid string)
 	return nil
 }
 
+func (vm *remoteVerifyManager) DeleteAndCloseTask(task *verifyTask) {
+	delete(vm.tasks, task.blockHeader.Hash())
+	task.Close()
+}
+
 type VerifyResult struct {
 	Status      types.VerifyStatus
 	BlockNumber uint64
@@ -335,7 +344,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 +361,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{}{}
 	}

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)
 	}

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()
 	}

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"

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,