Merge pull request #640 from nspcc-dev/forbid-encoding-bad-txes

Forbid encoding incomplete transactions

pkg/consensus/consensus_test.go

@@ -49,7 +49,7 @@ func TestService_GetVerified(t *testing.T) {
 
 	p := new(Payload)
 	p.SetType(payload.PrepareRequestType)
-	p.SetPayload(&prepareRequest{transactionHashes: hashes})
+	p.SetPayload(&prepareRequest{transactionHashes: hashes, minerTx: *newMinerTx(999)})
 	p.SetValidatorIndex(1)
 
 	priv, _ := getTestValidator(1)

pkg/consensus/payload.go

@@ -241,6 +241,9 @@ func (p *Payload) DecodeBinaryUnsigned(r *io.BinReader) {
 func (p *Payload) Hash() util.Uint256 {
 	w := io.NewBufBinWriter()
 	p.EncodeBinaryUnsigned(w.BinWriter)
+	if w.Err != nil {
+		panic("failed to hash payload")
+	}
 
 	return hash.DoubleSha256(w.Bytes())
 }

pkg/core/block/block_test.go

@@ -86,8 +86,8 @@ func newDumbBlock() *Block {
 			},
 		},
 		Transactions: []*transaction.Transaction{
-			{Type: transaction.MinerType},
+			{Type: transaction.MinerType, Data: &transaction.MinerTX{}},
-			{Type: transaction.IssueType},
+			{Type: transaction.IssueType, Data: &transaction.IssueTX{}},
 		},
 	}
 }
@@ -105,22 +105,22 @@ func TestBlockVerify(t *testing.T) {
 	assert.Nil(t, block.Verify())
 
 	block.Transactions = []*transaction.Transaction{
-		{Type: transaction.IssueType},
+		{Type: transaction.IssueType, Data: &transaction.IssueTX{}},
-		{Type: transaction.MinerType},
+		{Type: transaction.MinerType, Data: &transaction.MinerTX{}},
 	}
 	assert.NoError(t, block.RebuildMerkleRoot())
 	assert.NotNil(t, block.Verify())
 
 	block.Transactions = []*transaction.Transaction{
-		{Type: transaction.MinerType},
+		{Type: transaction.IssueType, Data: &transaction.IssueTX{}},
-		{Type: transaction.MinerType},
+		{Type: transaction.MinerType, Data: &transaction.MinerTX{}},
 	}
 	assert.NoError(t, block.RebuildMerkleRoot())
 	assert.NotNil(t, block.Verify())
 	block.Transactions = []*transaction.Transaction{
-		{Type: transaction.MinerType},
+		{Type: transaction.MinerType, Data: &transaction.MinerTX{}},
-		{Type: transaction.IssueType},
+		{Type: transaction.IssueType, Data: &transaction.IssueTX{}},
-		{Type: transaction.IssueType},
+		{Type: transaction.IssueType, Data: &transaction.IssueTX{}},
 	}
 	assert.NotNil(t, block.Verify())
 }

pkg/core/dao_test.go

@@ -319,7 +319,7 @@ func TestGetCurrentHeaderHeight_Store(t *testing.T) {
 
 func TestStoreAsTransaction(t *testing.T) {
 	dao := newDao(storage.NewMemoryStore())
-	tx := &transaction.Transaction{}
+	tx := &transaction.Transaction{Type: transaction.IssueType, Data: &transaction.IssueTX{}}
 	hash := tx.Hash()
 	err := dao.StoreAsTransaction(tx, 0)
 	require.NoError(t, err)

pkg/core/transaction/transaction.go

@@ -1,6 +1,7 @@
 package transaction
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/CityOfZion/neo-go/pkg/crypto/hash"
@@ -151,13 +152,15 @@ func (t *Transaction) EncodeBinary(bw *io.BinWriter) {
 // encodeHashableFields encodes the fields that are not used for
 // signing the transaction, which are all fields except the scripts.
 func (t *Transaction) encodeHashableFields(bw *io.BinWriter) {
+	if t.Data == nil {
+		bw.Err = errors.New("transaction has no data")
+		return
+	}
 	bw.WriteB(byte(t.Type))
 	bw.WriteB(byte(t.Version))
 
 	// Underlying TXer.
-	if t.Data != nil {
 	t.Data.EncodeBinary(bw)
-	}
 
 	// Attributes
 	bw.WriteArray(t.Attributes)

pkg/core/transaction/transaction_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/CityOfZion/neo-go/pkg/smartcontract"
 	"github.com/CityOfZion/neo-go/pkg/util"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestWitnessEncodeDecode(t *testing.T) {
@@ -167,3 +168,10 @@ func TestDecodePublishTX(t *testing.T) {
 	assert.Nil(t, buf.Err)
 	assert.Equal(t, rawPublishTX, hex.EncodeToString(buf.Bytes()))
 }
+
+func TestEncodingTXWithNoData(t *testing.T) {
+	buf := io.NewBufBinWriter()
+	tx := &Transaction{}
+	tx.EncodeBinary(buf.BinWriter)
+	require.Error(t, buf.Err)
+}