core: refactor (*DAO).StoreAsTransaction

Squash (*DAO).StoreAsTransaction and
(*DAO).StoreConflictingTransactions. It's better to keep them this way,
because StoreAsTransaction is always followed by
StoreConflictingTransactions, so it's an atomic operation.

The logic wasn't changed.

pkg/core/blockchain.go

@@ -732,13 +732,6 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error
 			}
 
 			writeBuf.Reset()
-			if bc.config.P2PSigExtensions {
-				err := kvcache.StoreConflictingTransactions(tx, block.Index, writeBuf)
-				if err != nil {
-					blockdone <- err
-					return
-				}
-			}
 		}
 		if bc.config.RemoveUntraceableBlocks {
 			var start, stop uint32

pkg/core/blockchain_test.go

@@ -792,9 +792,16 @@ func TestVerifyTx(t *testing.T) {
 				t.Run("dummy on-chain conflict", func(t *testing.T) {
 					tx := bc.newTestTx(h, testScript)
 					require.NoError(t, accs[0].SignTx(netmode.UnitTestNet, tx))
-					dummyTx := transaction.NewTrimmedTX(tx.Hash())
+					conflicting := transaction.New([]byte{byte(opcode.RET)}, 1)
-					dummyTx.Version = transaction.DummyVersion
+					conflicting.Attributes = []transaction.Attribute{
-					require.NoError(t, bc.dao.StoreAsTransaction(dummyTx, bc.blockHeight, nil))
+						{
+							Type: transaction.ConflictsT,
+							Value: &transaction.Conflicts{
+								Hash: tx.Hash(),
+							},
+						},
+					}
+					require.NoError(t, bc.dao.StoreAsTransaction(conflicting, bc.blockHeight, nil))
 					require.True(t, errors.Is(bc.VerifyTx(tx), ErrHasConflicts))
 				})
 				t.Run("attribute on-chain conflict", func(t *testing.T) {

pkg/core/dao/dao.go

@@ -68,7 +68,6 @@ type DAO interface {
 	StoreAsBlock(block *block.Block, buf *io.BufBinWriter) error
 	StoreAsCurrentBlock(block *block.Block, buf *io.BufBinWriter) error
 	StoreAsTransaction(tx *transaction.Transaction, index uint32, buf *io.BufBinWriter) error
-	StoreConflictingTransactions(tx *transaction.Transaction, index uint32, buf *io.BufBinWriter) error
 	putNEP17TransferInfo(acc util.Uint160, bs *state.NEP17TransferInfo, buf *io.BufBinWriter) error
 }
 
@@ -618,7 +617,8 @@ func (dao *Simple) StoreAsCurrentBlock(block *block.Block, buf *io.BufBinWriter)
 	return dao.Store.Put(storage.SYSCurrentBlock.Bytes(), buf.Bytes())
 }
 
-// StoreAsTransaction stores given TX as DataTransaction. It can reuse given
+// StoreAsTransaction stores given TX as DataTransaction. It also stores transactions
+// given tx has conflicts with as DataTransaction with dummy version. It can reuse given
 // buffer for the purpose of value serialization.
 func (dao *Simple) StoreAsTransaction(tx *transaction.Transaction, index uint32, buf *io.BufBinWriter) error {
 	key := storage.AppendPrefix(storage.DataTransaction, tx.Hash().BytesBE())
@@ -626,32 +626,30 @@ func (dao *Simple) StoreAsTransaction(tx *transaction.Transaction, index uint32,
 		buf = io.NewBufBinWriter()
 	}
 	buf.WriteU32LE(index)
-	if tx.Version == transaction.DummyVersion {
+	tx.EncodeBinary(buf.BinWriter)
-		buf.BinWriter.WriteB(tx.Version)
-	} else {
-		tx.EncodeBinary(buf.BinWriter)
-	}
 	if buf.Err != nil {
 		return buf.Err
 	}
-	return dao.Store.Put(key, buf.Bytes())
+	err := dao.Store.Put(key, buf.Bytes())
-}
+	if err != nil {
-
+		return err
-// StoreConflictingTransactions stores transactions given tx has conflicts with
-// as DataTransaction with dummy version. It can reuse given buffer for the
-// purpose of value serialization.
-func (dao *Simple) StoreConflictingTransactions(tx *transaction.Transaction, index uint32, buf *io.BufBinWriter) error {
-	if buf == nil {
-		buf = io.NewBufBinWriter()
 	}
-	for _, attr := range tx.GetAttributes(transaction.ConflictsT) {
+	if dao.p2pSigExtensions {
-		hash := attr.Value.(*transaction.Conflicts).Hash
+		var value []byte
-		dummyTx := transaction.NewTrimmedTX(hash)
+		for _, attr := range tx.GetAttributes(transaction.ConflictsT) {
-		dummyTx.Version = transaction.DummyVersion
+			hash := attr.Value.(*transaction.Conflicts).Hash
-		if err := dao.StoreAsTransaction(dummyTx, index, buf); err != nil {
+			copy(key[1:], hash.BytesBE())
-			return fmt.Errorf("failed to store conflicting transaction %s for transaction %s: %w", hash.StringLE(), tx.Hash().StringLE(), err)
+			if value == nil {
+				buf.Reset()
+				buf.WriteU32LE(index)
+				buf.BinWriter.WriteB(transaction.DummyVersion)
+				value = buf.Bytes()
+			}
+			err = dao.Store.Put(key, value)
+			if err != nil {
+				return fmt.Errorf("failed to store conflicting transaction %s for transaction %s: %w", hash.StringLE(), tx.Hash().StringLE(), err)
+			}
 		}
-		buf.Reset()
 	}
 	return nil
 }

pkg/core/dao/dao_test.go

@@ -2,6 +2,7 @@ package dao
 
 import (
 	"encoding/binary"
+	"errors"
 	"testing"
 
 	"github.com/nspcc-dev/neo-go/internal/random"
@@ -11,6 +12,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/io"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger"
+	"github.com/nspcc-dev/neo-go/pkg/util"
 	"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
 	"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
 	"github.com/stretchr/testify/require"
@@ -150,13 +152,69 @@ func TestGetCurrentHeaderHeight_Store(t *testing.T) {
 }
 
 func TestStoreAsTransaction(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false, false)
+	t.Run("P2PSigExtensions off", func(t *testing.T) {
+		dao := NewSimple(storage.NewMemoryStore(), false, false)
+		tx := transaction.New([]byte{byte(opcode.PUSH1)}, 1)
+		hash := tx.Hash()
+		err := dao.StoreAsTransaction(tx, 0, nil)
+		require.NoError(t, err)
+		err = dao.HasTransaction(hash)
+		require.NotNil(t, err)
+	})
+
+	t.Run("P2PSigExtensions on", func(t *testing.T) {
+		dao := NewSimple(storage.NewMemoryStore(), false, true)
+		conflictsH := util.Uint256{1, 2, 3}
+		tx := transaction.New([]byte{byte(opcode.PUSH1)}, 1)
+		tx.Attributes = []transaction.Attribute{
+			{
+				Type:  transaction.ConflictsT,
+				Value: &transaction.Conflicts{Hash: conflictsH},
+			},
+		}
+		hash := tx.Hash()
+		err := dao.StoreAsTransaction(tx, 0, nil)
+		require.NoError(t, err)
+		err = dao.HasTransaction(hash)
+		require.True(t, errors.Is(err, ErrAlreadyExists))
+		err = dao.HasTransaction(conflictsH)
+		require.True(t, errors.Is(err, ErrHasConflicts))
+	})
+}
+
+func BenchmarkStoreAsTransaction(b *testing.B) {
+	dao := NewSimple(storage.NewMemoryStore(), false, true)
 	tx := transaction.New([]byte{byte(opcode.PUSH1)}, 1)
-	hash := tx.Hash()
+	tx.Attributes = []transaction.Attribute{
-	err := dao.StoreAsTransaction(tx, 0, nil)
+		{
-	require.NoError(t, err)
+			Type: transaction.ConflictsT,
-	err = dao.HasTransaction(hash)
+			Value: &transaction.Conflicts{
-	require.NotNil(t, err)
+				Hash: util.Uint256{1, 2, 3},
+			},
+		},
+		{
+			Type: transaction.ConflictsT,
+			Value: &transaction.Conflicts{
+				Hash: util.Uint256{4, 5, 6},
+			},
+		},
+		{
+			Type: transaction.ConflictsT,
+			Value: &transaction.Conflicts{
+				Hash: util.Uint256{7, 8, 9},
+			},
+		},
+	}
+	_ = tx.Hash()
+
+	b.ResetTimer()
+	b.ReportAllocs()
+	for n := 0; n < b.N; n++ {
+		err := dao.StoreAsTransaction(tx, 1, nil)
+		if err != nil {
+			b.FailNow()
+		}
+	}
 }
 
 func TestMakeStorageItemKey(t *testing.T) {