all: fix comments and add error checking logic (#5)

* all: fix comments and add error checking logic

* core: fix uiHash related precompiled to be able to refund gas

cmd/geth/accountcmd_test.go

@@ -181,33 +181,6 @@ Repeat password: {{.InputLine "foobar2"}}
 `)
 }
 
-func TestWalletImport(t *testing.T) {
-	t.Parallel()
-	geth := runGeth(t, "wallet", "import", "--lightkdf", "testdata/guswallet.json")
-	defer geth.ExpectExit()
-	geth.Expect(`
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "foo"}}
-Address: {d4584b5f6229b7be90727b0fc8c6b91bb427821f}
-`)
-
-	files, err := os.ReadDir(filepath.Join(geth.Datadir, "keystore"))
-	if len(files) != 1 {
-		t.Errorf("expected one key file in keystore directory, found %d files (error: %v)", len(files), err)
-	}
-}
-
-func TestWalletImportBadPassword(t *testing.T) {
-	t.Parallel()
-	geth := runGeth(t, "wallet", "import", "--lightkdf", "testdata/guswallet.json")
-	defer geth.ExpectExit()
-	geth.Expect(`
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "wrong"}}
-Fatal: could not decrypt key with given password
-`)
-}
-
 func TestUnlockFlag(t *testing.T) {
 	t.Parallel()
 	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),

core/state_transition.go

@@ -433,7 +433,7 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
 		if msg.To != nil || msg.Value.Sign() != 0 {
 			return nil, ErrInvalidRestoration
 		}
-		if msg.Data == nil {
+		if len(msg.Data) == 0 {
 			return nil, ErrEmptyRestorationProof
 		}
 	}
@@ -479,7 +479,7 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
 	} else {
 		fee := new(big.Int).SetUint64(st.gasUsed())
 		if rules.IsLondon {
-			// BaseFee is burned and sent to the foundation's treasury
+			// BaseFee is sent to the foundation's treasury
 			burn := new(big.Int).Mul(fee, st.evm.Context.BaseFee)
 			st.state.AddBalance(params.FoundationTreasuryAddress, burn)
 		}

core/txpool/legacypool/list.go

@@ -349,7 +349,7 @@ func (l *list) Forward(threshold uint32) types.Transactions {
 }
 
 // Filter removes all transactions from the list with a cost or gas limit higher
-// than the provided thresholds or with a epochCoverage outdated. Every removed transaction
+// than the provided thresholds or with an outdated epochCoverage. Every removed transaction
 // is returned for any post-removal maintenance. Strict-mode invalidated transactions are
 // also returned.
 //

core/txpool/validation.go

@@ -94,7 +94,7 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types
 		return core.ErrTipAboveFeeCap
 	}
 	// Ensure to and value is nil when restoreData is non-nil.
-	if tx.IsRestoration() && (tx.To() != nil || tx.Value().Sign() != 0) {
+	if tx.IsRestoration() && (tx.To() != nil || tx.Value().Sign() != 0 || len(tx.Data()) == 0) {
 		return core.ErrInvalidRestoration
 	}
 	// Make sure the transaction is signed properly

core/vm/contracts.go

@@ -93,8 +93,8 @@ var PrecompiledContractsBerlin = map[common.Address]PrecompiledContract{
 	common.BytesToAddress([]byte{9}): &blake2F{},
 }
 
-// PrecompiledContractsBerlin contains the default set of pre-compiled Ethereum
-// contracts used in the Berlin release.
+// PrecompiledContractsAlpaca contains the default set of pre-compiled Ethereum
+// contracts used in the Alpaca release.
 var PrecompiledContractsAlpaca = map[common.Address]PrecompiledContract{
 	common.BytesToAddress([]byte{1}): &ecrecover{},
 	common.BytesToAddress([]byte{2}): &sha256hash{},
@@ -693,10 +693,17 @@ func (c *createContractWithUiHash) RequiredGas(input []byte) uint64 {
 func (c *createContractWithUiHash) Run(input []byte, caller ContractRef, value *big.Int, suppliedGas uint64, evm *EVM) ([]byte, uint64, error) {
 	res, addr, returnGas, suberr := evm.CreateWithUiHash(caller, input, suppliedGas, value)
 	if suberr != nil {
-		return nil, 0, suberr
+		// return ErrExecutionReverted for remaining gas to be refunded
+		return nil, returnGas, ErrExecutionReverted
+	}
+	addressType, err := abi.NewType("address", "", nil)
+	if err != nil {
+		return nil, 0, err
+	}
+	bytesType, err := abi.NewType("bytes", "", nil)
+	if err != nil {
+		return nil, 0, err
 	}
-	addressType, _ := abi.NewType("address", "", nil)
-	bytesType, _ := abi.NewType("bytes", "", nil)
 	arguments := abi.Arguments{
 		{Type: bytesType},
 		{Type: addressType},
@@ -725,10 +732,17 @@ func (c *create2ContractWithUiHash) Run(input []byte, caller ContractRef, value
 	}
 	res, addr, returnGas, suberr := evm.Create2WithUiHash(caller, input, suppliedGas, value)
 	if suberr != nil {
-		return nil, 0, suberr
+		// return ErrExecutionReverted for remaining gas to be refunded
+		return nil, returnGas, ErrExecutionReverted
+	}
+	addressType, err := abi.NewType("address", "", nil)
+	if err != nil {
+		return nil, 0, err
+	}
+	bytesType, err := abi.NewType("bytes", "", nil)
+	if err != nil {
+		return nil, 0, err
 	}
-	addressType, _ := abi.NewType("address", "", nil)
-	bytesType, _ := abi.NewType("bytes", "", nil)
 	arguments := abi.Arguments{
 		{Type: bytesType},
 		{Type: addressType},

core/vm/errors.go

@@ -41,12 +41,12 @@ var (
 	ErrCreate2NotAvailable      = errors.New("create2 is not available")
 	ErrCreate2EOA               = errors.New("EOA can not create contract with create2")
 
-	ErrInsufficientFee     = errors.New("insufficient balance for restoration fee")
 	ErrZeroEpochCoverage   = errors.New("epochCoverage of account is already zero")
 	ErrEpochProofMismatch  = errors.New("mismatch between the epoch to restore and proofs")
 	ErrInvalidSourceEpoch  = errors.New("invalid source epoch")
 	ErrInvalidTargetEpoch  = errors.New("invalid target epoch")
 	ErrContractRestoration = errors.New("restoration of contract account")
+	ErrHeaderIsNil         = errors.New("header is nil")
 
 	// errStopToken is an internal token indicating interpreter loop termination,
 	// never returned to outside callers.

core/vm/evm.go

@@ -649,8 +649,7 @@ func (evm *EVM) createWithUi(caller ContractRef, codeAndUiHash *codeAndUiHash, g
 	contract := NewContract(caller, AccountRef(address), value, gas)
 	contract.SetCodeOptionalHash(&address, codeAndUiHash.codeAndHash)
 
-	debug := evm.Config.Tracer != nil
-	if debug {
+	if evm.Config.Tracer != nil {
 		if evm.depth == 0 {
 			evm.Config.Tracer.CaptureStart(evm, caller.Address(), address, true, codeAndUiHash.input, gas, value)
 		} else {
@@ -683,7 +682,9 @@ func (evm *EVM) createWithUi(caller ContractRef, codeAndUiHash *codeAndUiHash, g
 		}
 	}
 
-	evm.StateDB.SetUiHash(address, codeAndUiHash.uiHash)
+	if evm.StateDB.Exist(address) {
+		evm.StateDB.SetUiHash(address, codeAndUiHash.uiHash)
+	}
 
 	// When an error was returned by the EVM or when setting the creation code
 	// above we revert to the snapshot and consume any gas remaining. Additionally
@@ -695,7 +696,7 @@ func (evm *EVM) createWithUi(caller ContractRef, codeAndUiHash *codeAndUiHash, g
 		}
 	}
 
-	if debug {
+	if evm.Config.Tracer != nil {
 		if evm.depth == 0 {
 			evm.Config.Tracer.CaptureEnd(ret, gas-contract.Gas, err)
 		} else {
@@ -876,8 +877,11 @@ func (evm *EVM) verifyRestorationProof(target common.Address, targetEpoch uint32
 			return 0, 0, nil, ErrZeroEpochCoverage
 		}
 
-		rootHash := evm.Context.GetHeaderByNumber(lastCkptBn).Root
-		leafNode, err := trie.VerifyProof(rootHash, targetKey, proofDB)
+		header := evm.Context.GetHeaderByNumber(lastCkptBn)
+		if header == nil {
+			return 0, 0, nil, ErrHeaderIsNil
+		}
+		leafNode, err := trie.VerifyProof(header.Root, targetKey, proofDB)
 		if err != nil {
 			return 0, 0, nil, fmt.Errorf("merkle proof verification failed: %v", err)
 		}

eth/protocols/snap/sync.go

@@ -786,7 +786,7 @@ func (s *Syncer) loadSyncStatus() {
 				task.genTrie = trie.NewStackTrie(options)
 				for accountHash, subtasks := range task.SubTasks {
 					if checkpoint {
-						// Removes tasks for accounts that are already existed in current state
+						// Removes tasks for accounts that exist in current state.
 						// This can happen when the pivot changed.
 						if rawdb.ReadAccountSnapshot(s.db, syncEpoch+1, accountHash) != nil {
 							// task.res can be nil if the task is already done
@@ -1960,7 +1960,7 @@ func (s *Syncer) processAccountResponse(res *accountResponse) {
 	res.task.pend = 0
 	for i, account := range res.accounts {
 		if checkpoint {
-			// Don't add tasks for accounts that are already existed in current state
+			// Don't add tasks for accounts that exist in current state
 			if rawdb.ReadAccountSnapshot(s.db, syncEpoch+1, res.hashes[i]) != nil {
 				continue
 			}
@@ -3137,6 +3137,7 @@ func (s *Syncer) reportSyncProgress(force bool) {
 	}
 	// Don't report anything until we have a meaningful progress
 	synced := s.accountBytes + s.bytecodeBytes + s.storageBytes
+	s.logTime = time.Now()
 	estBytes := s.estimateStateBytes()
 	if estBytes == 0 {
 		return
@@ -3187,7 +3188,6 @@ func (s *Syncer) estimateStateBytes() float64 {
 	if accountFills.BitLen() == 0 {
 		return 0
 	}
-	s.logTime = time.Now()
 	estBytes := float64(new(big.Int).Div(
 		new(big.Int).Mul(new(big.Int).SetUint64(uint64(synced)), hashSpace),
 		accountFills,

eth/state_accessor.go

@@ -169,8 +169,8 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u
 		// to prevent accumulating too many nodes in memory.
 		triedb.Reference(root, common.Hash{})
 		if parent != (common.Hash{}) {
-			epoch := eth.blockchain.Config().CalcEpoch(current.NumberU64() - 1)
-			triedb.Dereference(epoch, parent)
+			parentEpoch := eth.blockchain.Config().CalcEpoch(current.NumberU64() - 1)
+			triedb.Dereference(parentEpoch, parent)
 		}
 		parent = root
 	}

internal/ethapi/api.go

@@ -1019,6 +1019,7 @@ func (diff *StateOverride) Apply(state *state.StateDB) error {
 		if account.Balance != nil {
 			state.SetBalance(addr, (*big.Int)(*account.Balance))
 		}
+		// Override account storage count.
 		if account.StorageCount != nil {
 			state.SetStorageCount(addr, uint64(*account.StorageCount))
 		}

trie/trie_id.go

@@ -21,9 +21,9 @@ import "github.com/ethereum/go-ethereum/common"
 // ID is the identifier for uniquely identifying a trie.
 type ID struct {
 	StateRoot common.Hash // The root of the corresponding state(block.root)
+	Epoch     uint32      // The epoch number of the trie
 	Owner     common.Hash // The contract address hash which the trie belongs to
 	Root      common.Hash // The root hash of trie
-	Epoch     uint32
 }
 
 // StateTrieID constructs an identifier for state trie with the provided state root.
@@ -52,8 +52,8 @@ func StorageTrieID(stateRoot common.Hash, epoch uint32, owner common.Hash, root
 func TrieID(root common.Hash) *ID {
 	return &ID{
 		StateRoot: root,
+		Epoch:     0,
 		Owner:     common.Hash{},
 		Root:      root,
-		Epoch:     0,
 	}
 }

trie/triedb/pathdb/database.go

@@ -448,23 +448,26 @@ func (db *Database) Recoverable(epoch uint32, root, ckptRoot common.Hash) bool {
 		return false
 	}
 
-	// If epoch to recover is same as the epoch of the disk layer,
-	// Recovery is started from the disk layer.
-	// Else if epoch to recover is less than the epoch of the disk layer,
-	// Recovery is started from last state of the epoch to recover.
+	// Recoverable state must below the disk layer. The epoch of
+	// recoverable state must be the same as the disk layer's epoch.
+	// The recoverable state only refers the state that is currently
+	// not available, but can be restored by applying state history.
 	dl := db.tree.bottom()
-	restoreStateId := dl.stateID()
-	if *id >= restoreStateId {
+	if *id >= dl.stateID() {
 		return false
 	}
 	if dl.epochNumber() != epoch {
 		return false
 	}
+	ckptDl := db.tree.ckptBottom()
+	if ckptDl != nil && ckptDl.rootHash() != types.TrieRootHash(ckptRoot) {
+		return false
+	}
 
 	// Ensure the requested state is a canonical state and all state
 	// histories in range [id+1, disklayer.ID] are present and complete.
 	parent := root
-	return checkHistories(db.freezer, *id+1, restoreStateId-*id, func(m *meta) error {
+	return checkHistories(db.freezer, *id+1, dl.stateID()-*id, func(m *meta) error {
 		if m.parent != parent {
 			return errors.New("unexpected state history")
 		}

trie/triedb/pathdb/layertree.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/trie/trienode"
 	"github.com/ethereum/go-ethereum/trie/triestate"
 )
@@ -251,6 +252,7 @@ func (tree *layerTree) ckptBottom() *ckptDiskLayer {
 	defer tree.lock.RUnlock()
 
 	if tree.diskLayer == nil {
+		log.Error("disk layer is empty")
 		return nil // Shouldn't happen, empty tree
 	}
 	return tree.diskLayer.ckptLayer