Merge pull request #2134 from nspcc-dev/remove-untraceable-conflicts

core: remove untraceable dummy conflicting transactions

pkg/compiler/interop_test.go

@@ -193,7 +193,7 @@ func TestAppCall(t *testing.T) {
 	}
 
 	fc := fakechain.NewFakeChain()
-	ic := interop.NewContext(trigger.Application, fc, dao.NewSimple(storage.NewMemoryStore(), false), contractGetter, nil, nil, nil, zaptest.NewLogger(t))
+	ic := interop.NewContext(trigger.Application, fc, dao.NewSimple(storage.NewMemoryStore(), false, false), contractGetter, nil, nil, nil, zaptest.NewLogger(t))
 
 	t.Run("valid script", func(t *testing.T) {
 		src := getAppCallScript(fmt.Sprintf("%#v", ih.BytesBE()))

pkg/core/blockchain.go

@@ -235,8 +235,8 @@ func NewBlockchain(s storage.Store, cfg config.ProtocolConfiguration, log *zap.L
 	}
 	bc := &Blockchain{
 		config:      cfg,
-		dao:         dao.NewSimple(s, cfg.StateRootInHeader),
+		dao:         dao.NewSimple(s, cfg.StateRootInHeader, cfg.P2PSigExtensions),
-		persistent:  dao.NewSimple(s, cfg.StateRootInHeader),
+		persistent:  dao.NewSimple(s, cfg.StateRootInHeader, cfg.P2PSigExtensions),
 		stopCh:      make(chan struct{}),
 		runToExitCh: make(chan struct{}),
 		memPool:     mempool.New(cfg.MemPoolSize, 0, false),
@@ -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
 }
 
@@ -77,12 +76,14 @@ type Simple struct {
 	Store *storage.MemCachedStore
 	// stateRootInHeader specifies if block header contains state root.
 	stateRootInHeader bool
+	// p2pSigExtensions denotes whether P2PSignatureExtensions are enabled.
+	p2pSigExtensions bool
 }
 
 // NewSimple creates new simple dao using provided backend store.
-func NewSimple(backend storage.Store, stateRootInHeader bool) *Simple {
+func NewSimple(backend storage.Store, stateRootInHeader bool, p2pSigExtensions bool) *Simple {
 	st := storage.NewMemCachedStore(backend)
-	return &Simple{Store: st, stateRootInHeader: stateRootInHeader}
+	return &Simple{Store: st, stateRootInHeader: stateRootInHeader, p2pSigExtensions: p2pSigExtensions}
 }
 
 // GetBatch returns currently accumulated DB changeset.
@@ -93,7 +94,7 @@ func (dao *Simple) GetBatch() *storage.MemBatch {
 // GetWrapped returns new DAO instance with another layer of wrapped
 // MemCachedStore around the current DAO Store.
 func (dao *Simple) GetWrapped() DAO {
-	d := NewSimple(dao.Store, dao.stateRootInHeader)
+	d := NewSimple(dao.Store, dao.stateRootInHeader, dao.p2pSigExtensions)
 	return d
 }
 
@@ -585,6 +586,13 @@ func (dao *Simple) DeleteBlock(h util.Uint256, w *io.BufBinWriter) error {
 	for _, tx := range b.Transactions {
 		copy(key[1:], tx.Hash().BytesBE())
 		batch.Delete(key)
+		if dao.p2pSigExtensions {
+			for _, attr := range tx.GetAttributes(transaction.ConflictsT) {
+				hash := attr.Value.(*transaction.Conflicts).Hash
+				copy(key[1:], hash.BytesBE())
+				batch.Delete(key)
+			}
+		}
 		key[0] = byte(storage.STNotification)
 		batch.Delete(key)
 	}
@@ -609,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())
@@ -617,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,13 +12,14 @@ 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"
 )
 
 func TestPutGetAndDecode(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	serializable := &TestSerializable{field: random.String(4)}
 	hash := []byte{1}
 	err := dao.Put(serializable, hash)
@@ -42,7 +44,7 @@ func (t *TestSerializable) DecodeBinary(reader *io.BinReader) {
 }
 
 func TestPutGetAppExecResult(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	hash := random.Uint256()
 	appExecResult := &state.AppExecResult{
 		Container: hash,
@@ -60,7 +62,7 @@ func TestPutGetAppExecResult(t *testing.T) {
 }
 
 func TestPutGetStorageItem(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	id := int32(random.Int(0, 1024))
 	key := []byte{0}
 	storageItem := state.StorageItem{}
@@ -71,7 +73,7 @@ func TestPutGetStorageItem(t *testing.T) {
 }
 
 func TestDeleteStorageItem(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	id := int32(random.Int(0, 1024))
 	key := []byte{0}
 	storageItem := state.StorageItem{}
@@ -84,7 +86,7 @@ func TestDeleteStorageItem(t *testing.T) {
 }
 
 func TestGetBlock_NotExists(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	hash := random.Uint256()
 	block, err := dao.GetBlock(hash)
 	require.Error(t, err)
@@ -92,7 +94,7 @@ func TestGetBlock_NotExists(t *testing.T) {
 }
 
 func TestPutGetBlock(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	b := &block.Block{
 		Header: block.Header{
 			Script: transaction.Witness{
@@ -110,14 +112,14 @@ func TestPutGetBlock(t *testing.T) {
 }
 
 func TestGetVersion_NoVersion(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	version, err := dao.GetVersion()
 	require.Error(t, err)
 	require.Equal(t, "", version)
 }
 
 func TestGetVersion(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	err := dao.PutVersion("testVersion")
 	require.NoError(t, err)
 	version, err := dao.GetVersion()
@@ -126,14 +128,14 @@ func TestGetVersion(t *testing.T) {
 }
 
 func TestGetCurrentHeaderHeight_NoHeader(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	height, err := dao.GetCurrentBlockHeight()
 	require.Error(t, err)
 	require.Equal(t, uint32(0), height)
 }
 
 func TestGetCurrentHeaderHeight_Store(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), false)
+	dao := NewSimple(storage.NewMemoryStore(), false, false)
 	b := &block.Block{
 		Header: block.Header{
 			Script: transaction.Witness{
@@ -150,13 +152,69 @@ func TestGetCurrentHeaderHeight_Store(t *testing.T) {
 }
 
 func TestStoreAsTransaction(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), 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) {
@@ -173,7 +231,7 @@ func TestMakeStorageItemKey(t *testing.T) {
 }
 
 func TestPutGetStateSyncPoint(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), true)
+	dao := NewSimple(storage.NewMemoryStore(), true, false)
 
 	// empty store
 	_, err := dao.GetStateSyncPoint()
@@ -188,7 +246,7 @@ func TestPutGetStateSyncPoint(t *testing.T) {
 }
 
 func TestPutGetStateSyncCurrentBlockHeight(t *testing.T) {
-	dao := NewSimple(storage.NewMemoryStore(), true)
+	dao := NewSimple(storage.NewMemoryStore(), true, false)
 
 	// empty store
 	_, err := dao.GetStateSyncCurrentBlockHeight()

pkg/core/interop/crypto/ecdsa_test.go

@@ -71,7 +71,7 @@ func initCheckMultisigVMNoArgs(container *transaction.Transaction) *vm.VM {
 	ic := interop.NewContext(
 		trigger.Verification,
 		fakechain.NewFakeChain(),
-		dao.NewSimple(storage.NewMemoryStore(), false),
+		dao.NewSimple(storage.NewMemoryStore(), false, false),
 		nil, nil, nil,
 		container,
 		nil)
@@ -177,7 +177,7 @@ func TestCheckSig(t *testing.T) {
 	require.NoError(t, err)
 
 	verifyFunc := ECDSASecp256r1CheckSig
-	d := dao.NewSimple(storage.NewMemoryStore(), false)
+	d := dao.NewSimple(storage.NewMemoryStore(), false, false)
 	ic := &interop.Context{Network: uint32(netmode.UnitTestNet), DAO: d}
 	runCase := func(t *testing.T, isErr bool, result interface{}, args ...interface{}) {
 		ic.SpawnVM()

pkg/core/interop_system_test.go

@@ -552,7 +552,7 @@ func TestStorageFind(t *testing.T) {
 func createVM(t *testing.T) (*vm.VM, *interop.Context, *Blockchain) {
 	chain := newTestChain(t)
 	context := chain.newInteropContext(trigger.Application,
-		dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader), nil, nil)
+		dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader, chain.config.P2PSigExtensions), nil, nil)
 	v := context.SpawnVM()
 	return v, context, chain
 }
@@ -572,7 +572,7 @@ func createVMAndContractState(t testing.TB) (*vm.VM, *state.Contract, *interop.C
 	}
 
 	chain := newTestChain(t)
-	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader)
+	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader, chain.config.P2PSigExtensions)
 	context := chain.newInteropContext(trigger.Application, d, nil, nil)
 	v := context.SpawnVM()
 	return v, contractState, context, chain
@@ -584,7 +584,7 @@ func createVMAndTX(t *testing.T) (*vm.VM, *transaction.Transaction, *interop.Con
 	tx.Signers = []transaction.Signer{{Account: util.Uint160{1, 2, 3, 4}}}
 	tx.Scripts = []transaction.Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	chain := newTestChain(t)
-	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader)
+	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader, chain.config.P2PSigExtensions)
 	context := chain.newInteropContext(trigger.Application, d, nil, tx)
 	v := context.SpawnVM()
 	return v, tx, context, chain

pkg/core/interops_test.go

@@ -17,7 +17,7 @@ func testNonInterop(t *testing.T, value interface{}, f func(*interop.Context) er
 	v := vm.New()
 	v.Estack().PushVal(value)
 	chain := newTestChain(t)
-	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader)
+	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader, chain.config.P2PSigExtensions)
 	context := chain.newInteropContext(trigger.Application, d, nil, nil)
 	context.VM = v
 	require.Error(t, f(context))

pkg/core/native/management_test.go

@@ -17,7 +17,7 @@ import (
 
 func TestDeployGetUpdateDestroyContract(t *testing.T) {
 	mgmt := newManagement()
-	d := dao.NewSimple(storage.NewMemoryStore(), false)
+	d := dao.NewSimple(storage.NewMemoryStore(), false, false)
 	err := mgmt.Initialize(&interop.Context{DAO: d})
 	require.NoError(t, err)
 	script := []byte{byte(opcode.RET)}
@@ -72,12 +72,12 @@ func TestDeployGetUpdateDestroyContract(t *testing.T) {
 
 func TestManagement_Initialize(t *testing.T) {
 	t.Run("good", func(t *testing.T) {
-		d := dao.NewSimple(storage.NewMemoryStore(), false)
+		d := dao.NewSimple(storage.NewMemoryStore(), false, false)
 		mgmt := newManagement()
 		require.NoError(t, mgmt.InitializeCache(d))
 	})
 	t.Run("invalid contract state", func(t *testing.T) {
-		d := dao.NewSimple(storage.NewMemoryStore(), false)
+		d := dao.NewSimple(storage.NewMemoryStore(), false, false)
 		mgmt := newManagement()
 		require.NoError(t, d.PutStorageItem(mgmt.ID, []byte{prefixContract}, state.StorageItem{0xFF}))
 		require.Error(t, mgmt.InitializeCache(d))
@@ -86,7 +86,7 @@ func TestManagement_Initialize(t *testing.T) {
 
 func TestManagement_GetNEP17Contracts(t *testing.T) {
 	mgmt := newManagement()
-	d := dao.NewSimple(storage.NewMemoryStore(), false)
+	d := dao.NewSimple(storage.NewMemoryStore(), false, false)
 	err := mgmt.Initialize(&interop.Context{DAO: d})
 	require.NoError(t, err)
 

pkg/core/native_contract_test.go

@@ -227,7 +227,7 @@ func TestNativeContract_InvokeInternal(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader)
+	d := dao.NewSimple(storage.NewMemoryStore(), chain.config.StateRootInHeader, chain.config.P2PSigExtensions)
 	ic := chain.newInteropContext(trigger.Application, d, nil, nil)
 
 	sumOffset := 0