core: move spent coin management out of the inner storeBlock loop

prevHash == input.PrevHash, so make less DB accesses and more real work. Fix
some bugs along the way:
 * spentCoins structure may already be present in the DB when persisting TX,
   there is nothing wrong with that and we shouldn't overwrite it
 * it's only used for NEO and only to check for claim validity. Thus, when
   processing claim tx the corresponding spentCoins should always be present
   in the DB

pkg/core/blockchain.go

@@ -440,6 +440,11 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 			if err != nil {
 				return err
 			}
+			spentCoin, err := cache.GetSpentCoinsOrNew(prevHash, prevTXHeight)
+			if err != nil {
+				return err
+			}
+			oldSpentCoinLen := len(spentCoin.items)
 			for _, input := range inputs {
 				unspent.states[input.PrevIndex] = state.CoinSpent
 				prevTXOutput := prevTX.Outputs[input.PrevIndex]
@@ -449,11 +454,7 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 				}
 
 				if prevTXOutput.AssetID.Equals(GoverningTokenID()) {
-					spentCoin := NewSpentCoinState(input.PrevHash, prevTXHeight)
 					spentCoin.items[input.PrevIndex] = block.Index
-					if err = cache.PutSpentCoinState(input.PrevHash, spentCoin); err != nil {
-						return err
-					}
 					if err = processTXWithValidatorsSubtract(&prevTXOutput, account, cache); err != nil {
 						return err
 					}
@@ -482,6 +483,11 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 			if err = cache.PutUnspentCoinState(prevHash, unspent); err != nil {
 				return err
 			}
+			if oldSpentCoinLen != len(spentCoin.items) {
+				if err = cache.PutSpentCoinState(prevHash, spentCoin); err != nil {
+					return err
+				}
+			}
 		}
 
 		// Process the underlying type of the TX.
@@ -517,18 +523,34 @@ func (bc *Blockchain) storeBlock(block *block.Block) error {
 			// Remove claimed NEO from spent coins making it unavalaible for
 			// additional claims.
 			for _, input := range t.Claims {
-				scs, err := cache.GetSpentCoinsOrNew(input.PrevHash)
+				scs, err := cache.GetSpentCoinState(input.PrevHash)
-				if err != nil {
+				if err == nil {
-					return err
+					_, ok := scs.items[input.PrevIndex]
+					if !ok {
+						err = errors.New("no spent coin state")
+					}
 				}
-				if scs.txHash == input.PrevHash {
+				if err != nil {
-					// Existing scs.
+					// We can't really do anything about it
-					delete(scs.items, input.PrevIndex)
+					// as it's a transaction in a signed block.
+					bc.log.Warn("DOUBLE CLAIM",
+						zap.String("PrevHash", input.PrevHash.StringLE()),
+						zap.Uint16("PrevIndex", input.PrevIndex),
+						zap.String("tx", tx.Hash().StringLE()),
+						zap.Uint32("block", block.Index),
+					)
+					// "Strict" mode.
+					if bc.config.VerifyTransactions {
+						return err
+					}
+					break
+				}
+				delete(scs.items, input.PrevIndex)
+				if len(scs.items) > 0 {
 					if err = cache.PutSpentCoinState(input.PrevHash, scs); err != nil {
 						return err
 					}
 				} else {
-					// Uninitialized, new, forget about it.
 					if err = cache.DeleteSpentCoinState(input.PrevHash); err != nil {
 						return err
 					}

pkg/core/dao.go

@@ -175,15 +175,13 @@ func (dao *dao) PutUnspentCoinState(hash util.Uint256, ucs *UnspentCoinState) er
 // -- start spent coins.
 
 // GetSpentCoinsOrNew returns spent coins from store.
-func (dao *dao) GetSpentCoinsOrNew(hash util.Uint256) (*SpentCoinState, error) {
+func (dao *dao) GetSpentCoinsOrNew(hash util.Uint256, height uint32) (*SpentCoinState, error) {
 	spent, err := dao.GetSpentCoinState(hash)
 	if err != nil {
 		if err != storage.ErrKeyNotFound {
 			return nil, err
 		}
-		spent = &SpentCoinState{
+		spent = NewSpentCoinState(hash, height)
-			items: make(map[uint16]uint32),
-		}
 	}
 	return spent, nil
 }

pkg/core/dao_test.go

@@ -124,7 +124,7 @@ func TestPutGetUnspentCoinState(t *testing.T) {
 func TestGetSpentCoinStateOrNew_New(t *testing.T) {
 	dao := newDao(storage.NewMemoryStore())
 	hash := random.Uint256()
-	spentCoinState, err := dao.GetSpentCoinsOrNew(hash)
+	spentCoinState, err := dao.GetSpentCoinsOrNew(hash, 1)
 	require.NoError(t, err)
 	require.NotNil(t, spentCoinState)
 }