Merge pull request #399 from OffchainLabs/opt-destroy-no-storage

Optimize destroying an account with no storage in snapshot diffToDisk

Author: tsahee

core/state/snapshot/disklayer.go

@@ -76,30 +76,28 @@ func (dl *diskLayer) Stale() bool {
 
 // Account directly retrieves the account associated with a particular hash in
 // the snapshot slim data format.
-func (dl *diskLayer) Account(hash common.Hash) (*types.SlimAccount, error) {
-	data, err := dl.AccountRLP(hash)
+func (dl *diskLayer) account(hash common.Hash, evenIfStale bool) (*types.SlimAccount, error) {
+	data, err := dl.accountRLP(hash, evenIfStale)
 	if err != nil {
 		return nil, err
 	}
 	if len(data) == 0 { // can be both nil and []byte{}
 		return nil, nil
 	}
 	account := new(types.SlimAccount)
-	if err := rlp.DecodeBytes(data, account); err != nil {
-		panic(err)
-	}
-	return account, nil
+	err = rlp.DecodeBytes(data, account)
+	return account, err
 }
 
 // AccountRLP directly retrieves the account RLP associated with a particular
 // hash in the snapshot slim data format.
-func (dl *diskLayer) AccountRLP(hash common.Hash) ([]byte, error) {
+func (dl *diskLayer) accountRLP(hash common.Hash, evenIfStale bool) ([]byte, error) {
 	dl.lock.RLock()
 	defer dl.lock.RUnlock()
 
 	// If the layer was flattened into, consider it invalid (any live reference to
 	// the original should be marked as unusable).
-	if dl.stale {
+	if dl.stale && !evenIfStale {
 		return nil, ErrSnapshotStale
 	}
 	// If the layer is being generated, ensure the requested hash has already been
@@ -121,7 +119,9 @@ func (dl *diskLayer) AccountRLP(hash common.Hash) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	dl.cache.Set(hash[:], blob)
+	if !dl.stale {
+		dl.cache.Set(hash[:], blob)
+	}
 
 	snapshotCleanAccountMissMeter.Mark(1)
 	if n := len(blob); n > 0 {
@@ -132,6 +132,14 @@ func (dl *diskLayer) AccountRLP(hash common.Hash) ([]byte, error) {
 	return blob, nil
 }
 
+func (dl *diskLayer) Account(hash common.Hash) (*types.SlimAccount, error) {
+	return dl.account(hash, false)
+}
+
+func (dl *diskLayer) AccountRLP(hash common.Hash) ([]byte, error) {
+	return dl.accountRLP(hash, false)
+}
+
 // Storage directly retrieves the storage data associated with a particular hash,
 // within a particular account.
 func (dl *diskLayer) Storage(accountHash, storageHash common.Hash) ([]byte, error) {

core/state/snapshot/snapshot.go

@@ -564,10 +564,23 @@ func diffToDisk(bottom *diffLayer) *diskLayer {
 		if base.genMarker != nil && bytes.Compare(hash[:], base.genMarker) > 0 {
 			continue
 		}
-		// Remove all storage slots
+
+		oldAccount, err := base.account(hash, true)
+
+		// Delete the account snapshot
 		rawdb.DeleteAccountSnapshot(batch, hash)
 		base.cache.Set(hash[:], nil)
 
+		// Try to check if there's any associated storage to delete from the snapshot
+		if err != nil {
+			log.Warn("Failed to get destructed account from snapshot", "err", err)
+			// Fall through to deleting storage in case this account has any
+		} else if oldAccount == nil || oldAccount.Root == nil {
+			// There's no storage associated with this account to delete
+			continue
+		}
+
+		// Remove all storage slots
 		it := rawdb.IterateStorageSnapshots(base.diskdb, hash)
 		for it.Next() {
 			key := it.Key()