Make AccountCreation do determined amount of operations and Move .Put out of .View (#402)

* move .Put out of .View and remove deletion of all tombstones when delete acc

* move .Put out of .View and remove deletion of all tombstones when delete acc

cmd/restapi/apis/storage_tombstone_api.go

@@ -35,9 +35,8 @@ func (e *Env) FindStorageTombstone(c *gin.Context) {
 }
 
 type StorageTombsResponse struct {
-	Prefix               string `json:"prefix"`
-	DontOverlapOtherTomb bool   `json:"dontOverlapOtherTomb"`
-	HideStorage          bool   `json:"hideStorage"`
+	Prefix      string `json:"prefix"`
+	HideStorage bool   `json:"hideStorage"`
 }
 
 func findStorageTombstoneByPrefix(prefixS string, remoteDB *remote.DB) ([]*StorageTombsResponse, error) {
@@ -46,7 +45,6 @@ func findStorageTombstoneByPrefix(prefixS string, remoteDB *remote.DB) ([]*Stora
 	if err := remoteDB.View(context.TODO(), func(tx *remote.Tx) error {
 		interBucket := tx.Bucket(dbutils.IntermediateTrieHashBucket)
 		c := interBucket.Cursor(remote.DefaultCursorOpts.PrefetchValues(true))
-		cOverlap := interBucket.Cursor(remote.DefaultCursorOpts.PrefetchValues(false).PrefetchSize(1))
 		storage := tx.Bucket(dbutils.StorageBucket).Cursor(remote.DefaultCursorOpts.PrefetchValues(false).PrefetchSize(1))
 
 		for k, v, err := c.Seek(prefix); k != nil; k, v, err = c.Next() {
@@ -61,25 +59,6 @@ func findStorageTombstoneByPrefix(prefixS string, remoteDB *remote.DB) ([]*Stora
 				continue
 			}
 
-			// 1 prefix must be covered only by 1 tombstone
-			overlap := false
-			from := append(k, []byte{0, 0}...)
-			for overlapK, v, err := cOverlap.Seek(from); overlapK != nil; overlapK, v, err = cOverlap.Next() {
-				if err != nil {
-					return err
-				}
-				if !bytes.HasPrefix(overlapK, from) {
-					overlapK = nil
-				}
-				if len(v) > 0 {
-					continue
-				}
-
-				if bytes.HasPrefix(overlapK, k) {
-					overlap = true
-				}
-			}
-
 			// each tomb must cover storage
 			hideStorage := false
 			addrHash := common.CopyBytes(k[:common.HashLength])
@@ -108,16 +87,14 @@ func findStorageTombstoneByPrefix(prefixS string, remoteDB *remote.DB) ([]*Stora
 			}
 
 			results = append(results, &StorageTombsResponse{
-				Prefix:               fmt.Sprintf("%x\n", k),
-				DontOverlapOtherTomb: !overlap,
-				HideStorage:          hideStorage,
+				Prefix:      fmt.Sprintf("%x\n", k),
+				HideStorage: hideStorage,
 			})
 
 			if len(results) > 50 {
 				results = append(results, &StorageTombsResponse{
-					Prefix:               "too much results",
-					DontOverlapOtherTomb: true,
-					HideStorage:          true,
+					Prefix:      "too much results",
+					HideStorage: true,
 				})
 				return nil
 			}

core/state/database.go

@@ -277,6 +277,7 @@ func ClearTombstonesForReCreatedAccount(db ethdb.MinDatabase, addrHash common.Ha
 		return fmt.Errorf("only Bolt supported yet, given: %T", db)
 	}
 
+	var toPut [][]byte
 	if err := boltDb.Update(func(tx *bolt.Tx) error {
 		if debug.IntermediateTrieHashAssertDbIntegrity {
 			defer func() {
@@ -296,7 +297,6 @@ func ClearTombstonesForReCreatedAccount(db ethdb.MinDatabase, addrHash common.Ha
 		incarnation := dbutils.DecodeIncarnation(k[common.HashLength : common.HashLength+8])
 		for ; incarnation > 0; incarnation-- {
 			accWithInc := dbutils.GenerateStoragePrefix(addrHash, incarnation)
-
 			for k, _ = storage.Seek(accWithInc); k != nil; k, _ = storage.Next() {
 				if !bytes.HasPrefix(k, accWithInc) {
 					k = nil
@@ -307,16 +307,18 @@ func ClearTombstonesForReCreatedAccount(db ethdb.MinDatabase, addrHash common.Ha
 				}
 
 				kNoInc := dbutils.RemoveIncarnationFromKey(k)
-				if err := db.Put(dbutils.IntermediateTrieHashBucket, common.CopyBytes(kNoInc[:common.HashLength+1]), []byte{}); err != nil {
-					return err
-				}
+				toPut = append(toPut, common.CopyBytes(kNoInc[:common.HashLength+1]))
 			}
 		}
-
 		return nil
 	}); err != nil {
 		return err
 	}
+	for _, k := range toPut {
+		if err := db.Put(dbutils.IntermediateTrieHashBucket, k, []byte{}); err != nil {
+			return err
+		}
+	}
 
 	if err := db.Delete(dbutils.IntermediateTrieHashBucket, addrHashBytes); err != nil {
 		return err
@@ -340,7 +342,8 @@ func PutTombstoneForDeletedAccount(db ethdb.MinDatabase, addrHash []byte) error
 	buf := pool.GetBuffer(64)
 	defer pool.PutBuffer(buf)
 
-	return boltDb.View(func(tx *bolt.Tx) error {
+	hasStorage := false
+	if err := boltDb.View(func(tx *bolt.Tx) error {
 		if debug.IntermediateTrieHashAssertDbIntegrity {
 			defer func() {
 				if err := StorageTombstonesIntegrityDBCheck(tx); err != nil {
@@ -349,28 +352,7 @@ func PutTombstoneForDeletedAccount(db ethdb.MinDatabase, addrHash []byte) error
 			}()
 		}
 
-		// cleanup all previous under given account
-		interBucket := tx.Bucket(dbutils.IntermediateTrieHashBucket)
-		c := interBucket.Cursor()
-		for k, v := c.Seek(addrHash); k != nil; k, v = c.Next() {
-			if !bytes.HasPrefix(k, addrHash) {
-				k = nil
-			}
-			if k == nil {
-				break
-			}
-
-			if len(v) > 0 {
-				continue
-			}
-
-			if err := db.Delete(dbutils.IntermediateTrieHashBucket, common.CopyBytes(k)); err != nil {
-				return err
-			}
-		}
-
 		// place 1 tombstone to account if it has storage
-		hasStorage := false
 		storage := tx.Bucket(dbutils.StorageBucket).Cursor()
 		k, _ := storage.Seek(addrHash)
 		if !bytes.HasPrefix(k, addrHash) {
@@ -381,12 +363,16 @@ func PutTombstoneForDeletedAccount(db ethdb.MinDatabase, addrHash []byte) error
 			hasStorage = true
 		}
 
-		if !hasStorage {
-			return nil
-		}
+		return nil
+	}); err != nil {
+		return err
+	}
 
-		return db.Put(dbutils.IntermediateTrieHashBucket, common.CopyBytes(addrHash), []byte{})
-	})
+	if !hasStorage {
+		return nil
+	}
+
+	return db.Put(dbutils.IntermediateTrieHashBucket, common.CopyBytes(addrHash), []byte{})
 }
 
 func ClearTombstonesForNewStorage(db ethdb.MinDatabase, storageKeyNoInc []byte) error {
@@ -397,6 +383,9 @@ func ClearTombstonesForNewStorage(db ethdb.MinDatabase, storageKeyNoInc []byte)
 		return fmt.Errorf("only Bolt supported yet, given: %T", db)
 	}
 	addrHashBytes := common.CopyBytes(storageKeyNoInc[:common.HashLength])
+
+	var toPut [][]byte
+	var toDelete [][]byte
 	if err := boltDb.View(func(tx *bolt.Tx) error {
 		if debug.IntermediateTrieHashAssertDbIntegrity {
 			defer func() {
@@ -442,14 +431,9 @@ func ClearTombstonesForNewStorage(db ethdb.MinDatabase, storageKeyNoInc []byte)
 					continue
 				}
 
-				k := dbutils.RemoveIncarnationFromKey(storageK[:i+1+8])
-				if err := db.Put(dbutils.IntermediateTrieHashBucket, k, []byte{}); err != nil {
-					return err
-				}
-			}
-			if err := db.Delete(dbutils.IntermediateTrieHashBucket, common.CopyBytes(storageKeyNoInc[:i])); err != nil {
-				return err
+				toPut = append(toPut, dbutils.RemoveIncarnationFromKey(storageK[:i+1+8]))
 			}
+			toDelete = append(toDelete, common.CopyBytes(storageKeyNoInc[:i]))
 			break
 		}
 		return nil
@@ -457,6 +441,18 @@ func ClearTombstonesForNewStorage(db ethdb.MinDatabase, storageKeyNoInc []byte)
 		return err
 	}
 
+	for _, k := range toPut {
+		if err := db.Put(dbutils.IntermediateTrieHashBucket, k, []byte{}); err != nil {
+			return err
+		}
+	}
+
+	for _, k := range toDelete {
+		if err := db.Delete(dbutils.IntermediateTrieHashBucket, k); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

core/state/database_test.go

@@ -1179,7 +1179,6 @@ func TestClearTombstonesForReCreatedAccount(t *testing.T) {
 	checkProps := func() {
 		if err := db.KV().View(func(tx *bolt.Tx) error {
 			inter := tx.Bucket(dbutils.IntermediateTrieHashBucket).Cursor()
-			cOverlap := tx.Bucket(dbutils.IntermediateTrieHashBucket).Cursor()
 			storage := tx.Bucket(dbutils.StorageBucket).Cursor()
 
 			for k, v := inter.First(); k != nil; k, v = inter.Next() {
@@ -1187,21 +1186,7 @@ func TestClearTombstonesForReCreatedAccount(t *testing.T) {
 					continue
 				}
 
-				// 1 prefix must be covered only by 1 tombstone
-				from := append(k, []byte{0, 0}...)
-				for overlapK, overlapV := cOverlap.Seek(from); overlapK != nil; overlapK, overlapV = cOverlap.Next() {
-					if !bytes.HasPrefix(overlapK, from) {
-						overlapK = nil
-					}
-					if len(overlapV) > 0 {
-						continue
-					}
-
-					if bytes.HasPrefix(overlapK, k) {
-						panic(fmt.Sprintf("%x is prefix of %x\n", overlapK, k))
-					}
-				}
-
+				// each tomb must cover storage
 				addrHash := common.CopyBytes(k[:common.HashLength])
 				storageK, _ := storage.Seek(addrHash)
 				if !bytes.HasPrefix(storageK, addrHash) {
@@ -1358,10 +1343,12 @@ func TestClearTombstonesForReCreatedAccount(t *testing.T) {
 	checkProps()
 
 	checks = map[string]bool{
-		accKey:          true,
-		untouchedAcc:    false,
-		accKey + "2233": false, // was true on previous step
+		accKey:       true,
+		untouchedAcc: false,
 
+		// accKey + "2233" was true on previous step, don't delete this tombstone even one with shorter prefix exists.
+		// Because account creation must do predictable amount of operations.
+		accKey + "2233": true,
 	}
 
 	for k, expect := range checks {

debug-web-ui/src/components/LookupStorageTombstonesForm.js

@@ -41,7 +41,6 @@ const Details = ({hashes}) => (
                 <thead>
                 <tr>
                     <th><strong>Prefix</strong></th>
-                    <th><strong>Don't overlap other tomb</strong></th>
                     <th><strong>Hide storage</strong></th>
                 </tr>
                 </thead>
@@ -54,16 +53,13 @@ const Details = ({hashes}) => (
 );
 
 const TableRow = ({item}) => {
-    const {prefix, dontOverlapOtherTomb, hideStorage} = item
+    const {prefix,  hideStorage} = item
 
     return (
         <tr>
             <td className="text-monospace">
                 {prefix}
             </td>
-            <td className={dontOverlapOtherTomb ? '' : 'bg-danger'}>
-                {dontOverlapOtherTomb ? 'yes' : 'no'}
-            </td>
             <td className={hideStorage ? '' : 'bg-danger'}>
                 {hideStorage ? 'yes' : 'no'}
             </td>