Collection Malleability

engine/access/rest/websockets/data_providers/blocks_provider_test.go

@@ -61,9 +61,9 @@ func (s *BlocksProviderSuite) SetupTest() {
 	for i := 0; i < blockCount; i++ {
 		block := unittest.BlockWithParentFixture(parent)
 		transaction := unittest.TransactionFixture()
-		col := flow.CollectionFromTransactions([]*flow.Transaction{&transaction})
-		guarantee := col.Guarantee()
-		block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(&guarantee)))
+		col := unittest.CollectionFromTransactions([]*flow.Transaction{&transaction})
+		guarantee := &flow.CollectionGuarantee{CollectionID: col.ID()}
+		block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(guarantee)))
 		// update for next iteration
 		parent = block.Header
 		s.blocks = append(s.blocks, block)

engine/access/rpc/backend/backend_stream_transactions_test.go

@@ -349,9 +349,9 @@ func (s *TransactionStatusSuite) mockTransactionResult(transactionID *flow.Ident
 }
 
 func (s *TransactionStatusSuite) addBlockWithTransaction(transaction *flow.Transaction) {
-	col := flow.CollectionFromTransactions([]*flow.Transaction{transaction})
+	col := unittest.CollectionFromTransactions([]*flow.Transaction{transaction})
 	colID := col.ID()
-	guarantee := col.Guarantee()
+	guarantee := flow.CollectionGuarantee{CollectionID: colID}
 	light := col.Light()
 	s.sealedBlock = s.finalizedBlock
 	s.addNewFinalizedBlock(s.sealedBlock.Header, true, func(block *flow.Block) {

engine/access/rpc/backend/backend_transactions_test.go

@@ -73,7 +73,7 @@ func (suite *Suite) TestGetTransactionResultReturnsUnknown() {
 		tx := unittest.TransactionFixture()
 		tx.TransactionBody = tbody
 
-		coll := flow.CollectionFromTransactions([]*flow.Transaction{&tx})
+		coll := unittest.CollectionFromTransactions([]*flow.Transaction{&tx})
 		suite.state.On("AtBlockID", block.ID()).Return(snap, nil).Once()
 
 		suite.transactions.
@@ -106,7 +106,7 @@ func (suite *Suite) TestGetTransactionResultReturnsTransactionError() {
 		tx := unittest.TransactionFixture()
 		tx.TransactionBody = tbody
 
-		coll := flow.CollectionFromTransactions([]*flow.Transaction{&tx})
+		coll := unittest.CollectionFromTransactions([]*flow.Transaction{&tx})
 
 		suite.transactions.
 			On("ByID", tx.ID()).
@@ -144,7 +144,7 @@ func (suite *Suite) TestGetTransactionResultReturnsValidTransactionResultFromHis
 		tx := unittest.TransactionFixture()
 		tx.TransactionBody = tbody
 
-		coll := flow.CollectionFromTransactions([]*flow.Transaction{&tx})
+		coll := unittest.CollectionFromTransactions([]*flow.Transaction{&tx})
 
 		suite.transactions.
 			On("ByID", tx.ID()).
@@ -218,7 +218,7 @@ func (suite *Suite) TestGetTransactionResultFromCache() {
 		backend, err := New(params)
 		suite.Require().NoError(err)
 
-		coll := flow.CollectionFromTransactions([]*flow.Transaction{tx})
+		coll := unittest.CollectionFromTransactions([]*flow.Transaction{tx})
 
 		resp, err := backend.GetTransactionResult(
 			context.Background(),
@@ -261,7 +261,7 @@ func (suite *Suite) TestGetTransactionResultCacheNonExistent() {
 		backend, err := New(params)
 		suite.Require().NoError(err)
 
-		coll := flow.CollectionFromTransactions([]*flow.Transaction{tx})
+		coll := unittest.CollectionFromTransactions([]*flow.Transaction{tx})
 
 		resp, err := backend.GetTransactionResult(
 			context.Background(),
@@ -302,7 +302,7 @@ func (suite *Suite) TestGetTransactionResultUnknownFromCache() {
 		backend, err := New(params)
 		suite.Require().NoError(err)
 
-		coll := flow.CollectionFromTransactions([]*flow.Transaction{tx})
+		coll := unittest.CollectionFromTransactions([]*flow.Transaction{tx})
 
 		resp, err := backend.GetTransactionResult(
 			context.Background(),
@@ -1269,9 +1269,9 @@ func (suite *Suite) TestTransactionResultFromStorage() {
 	// Create fixtures for block, transaction, and collection
 	block := unittest.BlockFixture()
 	transaction := unittest.TransactionFixture()
-	col := flow.CollectionFromTransactions([]*flow.Transaction{&transaction})
-	guarantee := col.Guarantee()
-	block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(&guarantee)))
+	col := unittest.CollectionFromTransactions([]*flow.Transaction{&transaction})
+	guarantee := &flow.CollectionGuarantee{CollectionID: col.ID()}
+	block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(guarantee)))
 	txId := transaction.ID()
 	blockId := block.ID()
 
@@ -1359,9 +1359,9 @@ func (suite *Suite) TestTransactionByIndexFromStorage() {
 	// Create fixtures for block, transaction, and collection
 	block := unittest.BlockFixture()
 	transaction := unittest.TransactionFixture()
-	col := flow.CollectionFromTransactions([]*flow.Transaction{&transaction})
-	guarantee := col.Guarantee()
-	block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(&guarantee)))
+	col := unittest.CollectionFromTransactions([]*flow.Transaction{&transaction})
+	guarantee := &flow.CollectionGuarantee{CollectionID: col.ID()}
+	block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(guarantee)))
 	blockId := block.ID()
 	txId := transaction.ID()
 	txIndex := rand.Uint32()
@@ -1445,8 +1445,8 @@ func (suite *Suite) TestTransactionResultsByBlockIDFromStorage() {
 	// Create fixtures for the block and collection
 	block := unittest.BlockFixture()
 	col := unittest.CollectionFixture(2)
-	guarantee := col.Guarantee()
-	block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(&guarantee)))
+	guarantee := &flow.CollectionGuarantee{CollectionID: col.ID()}
+	block.SetPayload(unittest.PayloadFixture(unittest.WithGuarantees(guarantee)))
 	blockId := block.ID()
 
 	// Mock the behavior of the blocks, collections and light transaction results objects

model/cluster/payload.go

@@ -1,7 +1,6 @@
 package cluster
 
 import (
-	"github.com/onflow/flow-go/model/fingerprint"
 	"github.com/onflow/flow-go/model/flow"
 )
 
@@ -58,13 +57,3 @@ func PayloadFromTransactions(refID flow.Identifier, transactions ...*flow.Transa
 func (p Payload) Hash() flow.Identifier {
 	return flow.MakeID(p)
 }
-
-func (p Payload) Fingerprint() []byte {
-	return fingerprint.Fingerprint(struct {
-		Collection       []byte
-		ReferenceBlockID flow.Identifier
-	}{
-		Collection:       p.Collection.Fingerprint(),
-		ReferenceBlockID: p.ReferenceBlockID,
-	})
-}

model/flow/collection.go

@@ -1,23 +1,13 @@
 package flow
 
-import "github.com/onflow/flow-go/model/fingerprint"
-
-// Collection is set of transactions.
+// Collection is an ordered list of transactions.
+// Collections form a part of the payload of cluster blocks, produced by Collection Nodes.
+// Every Collection maps 1-1 to a Chunk, which is used for transaction execution.
 type Collection struct {
 	Transactions []*TransactionBody
 }
 
-// CollectionFromTransactions creates a new collection from the list of
-// transactions.
-func CollectionFromTransactions(transactions []*Transaction) Collection {
-	coll := Collection{Transactions: make([]*TransactionBody, 0, len(transactions))}
-	for _, tx := range transactions {
-		coll.Transactions = append(coll.Transactions, &tx.TransactionBody)
-	}
-	return coll
-}
-
-// Light returns the light, reference-only version of the collection.
+// Light returns a LightCollection, which contains only the list of transaction IDs from the Collection.
 func (c Collection) Light() LightCollection {
 	lc := LightCollection{Transactions: make([]Identifier, 0, len(c.Transactions))}
 	for _, tx := range c.Transactions {
@@ -26,94 +16,44 @@ func (c Collection) Light() LightCollection {
 	return lc
 }
 
-// Guarantee returns a collection guarantee for this collection.
-func (c *Collection) Guarantee() CollectionGuarantee {
-	return CollectionGuarantee{
-		CollectionID: c.ID(),
-	}
-}
-
+// ID returns a cryptographic commitment to the Collection.
+// The ID of a Collection is equivalent to the ID of its corresponding LightCollection.
 func (c Collection) ID() Identifier {
 	return c.Light().ID()
 }
 
-func (c Collection) Len() int {
-	return len(c.Transactions)
-}
-
+// Checksum returns the collection's ID.
+// Deprecated: This is needed temporarily until further malleability is done, because many components assume Collection implements Entity
+// TODO(malleability): remove this function
 func (c Collection) Checksum() Identifier {
-	return c.Light().Checksum()
+	return c.ID()
 }
 
-func (c Collection) Fingerprint() []byte {
-	var txs []byte
-	for _, tx := range c.Transactions {
-		txs = append(txs, tx.Fingerprint()...)
-	}
-
-	return fingerprint.Fingerprint(struct {
-		Transactions []byte
-	}{
-		Transactions: txs,
-	})
+// Len returns the number of transactions in the collection.
+func (c Collection) Len() int {
+	return len(c.Transactions)
 }
 
-// LightCollection is a collection containing references to the constituent
-// transactions rather than full transaction bodies. It is used for indexing
-// transactions by collection and for computing the collection fingerprint.
+// LightCollection contains cryptographic commitments to the constituent transactions instead of transaction bodies.
+// It is used for indexing transactions by collection and for computing the collection fingerprint.
 type LightCollection struct {
 	Transactions []Identifier
 }
 
+// ID returns a cryptographic commitment to the LightCollection.
+// The ID of a LightCollection is equivalent to the ID for its corresponding Collection.
 func (lc LightCollection) ID() Identifier {
 	return MakeID(lc)
 }
 
-func (lc LightCollection) Checksum() Identifier {
-	return MakeID(lc)
+// Checksum returns the collection's ID.
+// Deprecated: This is needed temporarily until further malleability is done, because many components assume Collection implements Entity
+// TODO(malleability): remove this function
+func (c LightCollection) Checksum() Identifier {
+	return c.ID()
 }
 
+// Len returns the number of transactions in the collection.
 func (lc LightCollection) Len() int {
 	return len(lc.Transactions)
 }
-
-func (lc LightCollection) Has(txID Identifier) bool {
-	for _, id := range lc.Transactions {
-		if txID == id {
-			return true
-		}
-	}
-	return false
-}
-
-// Note that this is the basic version of the List, we need to substitute it with something like Merkle tree at some point
-type CollectionList struct {
-	collections []*Collection
-}
-
-func (cl *CollectionList) Fingerprint() Identifier {
-	return MerkleRoot(GetIDs(cl.collections)...)
-}
-
-func (cl *CollectionList) Insert(ch *Collection) {
-	cl.collections = append(cl.collections, ch)
-}
-
-func (cl *CollectionList) Items() []*Collection {
-	return cl.collections
-}
-
-// ByChecksum returns an entity from the list by entity fingerprint
-func (cl *CollectionList) ByChecksum(cs Identifier) (*Collection, bool) {
-	for _, coll := range cl.collections {
-		if coll.Checksum() == cs {
-			return coll, true
-		}
-	}
-	return nil, false
-}
-
-// ByIndex returns an entity from the list by index
-func (cl *CollectionList) ByIndex(i uint64) *Collection {
-	return cl.collections[i]
-}

model/flow/collection_test.go

@@ -3,29 +3,23 @@ package flow_test
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
-	"github.com/onflow/flow-go/model/encoding/rlp"
-	"github.com/onflow/flow-go/model/fingerprint"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/unittest"
 )
 
-func TestLightCollectionFingerprint(t *testing.T) {
-	col := unittest.CollectionFixture(2)
-	colID := col.ID()
-	data := fingerprint.Fingerprint(col.Light())
-	var decoded flow.LightCollection
-	rlp.NewMarshaler().MustUnmarshal(data, &decoded)
-	decodedID := decoded.ID()
-	assert.Equal(t, colID, decodedID)
-	assert.Equal(t, col.Light(), decoded)
-}
-
 // TestLightCollectionID_Malleability confirms that the LightCollection struct, which implements
 // the [flow.IDEntity] interface, is resistant to tampering.
 func TestLightCollectionID_Malleability(t *testing.T) {
 	unittest.RequireEntityNonMalleable(t, &flow.LightCollection{
 		Transactions: unittest.IdentifierListFixture(5),
 	})
 }
+
+// TestCollectionID_Malleability confirms that the Collection struct, which implements
+// the [flow.IDEntity] interface, is resistant to tampering.
+func TestCollectionID_Malleability(t *testing.T) {
+	collection := unittest.CollectionFixture(5)
+	unittest.RequireEntityNonMalleable(t, &collection, unittest.WithTypeGenerator[flow.TransactionBody](func() flow.TransactionBody {
+		return unittest.TransactionBodyFixture()
+	}))
+}

model/flow/transaction.go

@@ -2,11 +2,15 @@ package flow
 
 import (
 	"fmt"
+	"io"
 
+	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/onflow/crypto"
 	"github.com/onflow/crypto/hash"
 	"golang.org/x/exp/slices"
 
+	flowrlp "github.com/onflow/flow-go/model/encoding/rlp"
+
 	"github.com/onflow/flow-go/model/fingerprint"
 )
 
@@ -53,16 +57,26 @@ func NewTransactionBody() *TransactionBody {
 	return &TransactionBody{}
 }
 
+// Fingerprint returns the canonical, unique byte representation for the TransactionBody.
+// As RLP encoding logic for TransactionBody is over-ridden by EncodeRLP below, this is
+// equivalent to directly RLP encoding the TransactionBody.
+// This public function is retained primarily for backward compatibility.
 func (tb TransactionBody) Fingerprint() []byte {
-	return fingerprint.Fingerprint(struct {
+	return flowrlp.NewMarshaler().MustMarshal(tb)
+}
+
+// EncodeRLP defines RLP encoding behaviour for TransactionBody.
+func (tb TransactionBody) EncodeRLP(w io.Writer) error {
+	encodingCanonicalForm := struct {
 		Payload            interface{}
 		PayloadSignatures  interface{}
 		EnvelopeSignatures interface{}
 	}{
 		Payload:            tb.payloadCanonicalForm(),
 		PayloadSignatures:  signaturesList(tb.PayloadSignatures).canonicalForm(),
 		EnvelopeSignatures: signaturesList(tb.EnvelopeSignatures).canonicalForm(),
-	})
+	}
+	return rlp.Encode(w, encodingCanonicalForm)
 }
 
 func (tb TransactionBody) ByteSize() uint {

model/flow/transaction_test.go

@@ -3,9 +3,11 @@ package flow_test
 import (
 	"testing"
 
+	"github.com/onflow/go-ethereum/rlp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/onflow/flow-go/model/fingerprint"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/unittest"
 )
@@ -58,3 +60,23 @@ func TestTransaction_Status(t *testing.T) {
 		assert.Equal(t, status.String(), value)
 	}
 }
+
+// TestTransactionBodyID_Malleability provides basic validation that [flow.TransactionBody] is not malleable.
+func TestTransactionBodyID_Malleability(t *testing.T) {
+	txbody := unittest.TransactionBodyFixture()
+	unittest.RequireEntityNonMalleable(t, &txbody, unittest.WithTypeGenerator[flow.TransactionSignature](func() flow.TransactionSignature {
+		return unittest.TransactionSignatureFixture()
+	}))
+}
+
+// TestTransactionBody_Fingerprint provides basic validation that the [TransactionBody] fingerprint
+// is equivalent to its canonical RLP encoding.
+func TestTransactionBody_Fingerprint(t *testing.T) {
+	txbody := unittest.TransactionBodyFixture()
+	fp1 := txbody.Fingerprint()
+	fp2 := fingerprint.Fingerprint(txbody)
+	fp3, err := rlp.EncodeToBytes(txbody)
+	require.NoError(t, err)
+	assert.Equal(t, fp1, fp2)
+	assert.Equal(t, fp2, fp3)
+}