feat!: Implement BlobTx (#893)

* chore!: remove original tx from the abci.TxResult * chore!: remove the original hash from malleated txs * fix: remove remaining original hash * feat!: add wrapper funcs for blob transactions * chore!: track blob txs in the mempool * chore!: add stronger protections against false positives while decoding by adding a typeID * fix: test malleated tx indexing * fix: comment * fix: use more accurate arg name in wrap malleated * Apply suggestions from code review Co-authored-by: Rootul P <[email protected]> * chore: proto-format and proto-gen to add comments * chore: docs Co-authored-by: Rootul P <[email protected]> * chore: rename unwrapped to unmarshal * fix: remaining renames * fix!: pass the entire blob transaction to the application Co-authored-by: Callum Waters <[email protected]> * feat!: modify tx.Key and tx.Hash to unwrap the tx before calculating the hash * Revert "chore!: track blob txs in the mempool" This reverts commit a6e28ae. * fix: clean up revert * chore: docs Co-authored-by: Rootul P <[email protected]> * chore: add docs and optimize unmarshalling of transactions when calculating the hash or the key * chore!: wrap/unwrap -> marshal/unmarshal * fix: update test * fix: use existing transaction to calculate len Co-authored-by: Rootul P <[email protected]> Co-authored-by: Callum Waters <[email protected]>
celestiaorg · Dec 8, 2022 · 74d234e · 74d234e
1 parent 7bbc4a7
commit 74d234e
Show file tree

Hide file tree

Showing 18 changed files with 838 additions and 535 deletions.
diff --git a/abci/types/types.pb.go b/abci/types/types.pb.go
diff --git a/mempool/v0/clist_mempool.go b/mempool/v0/clist_mempool.go
@@ -328,6 +328,9 @@ func (mem *CListMempool) removeTx(tx types.Tx, elem *clist.CElement, removeFromC
 	mem.txs.Remove(elem)
 	elem.DetachPrev()
 	mem.txsMap.Delete(tx.Key())
+	if memtx, ok := elem.Value.(*mempoolTx); ok {
+		tx = memtx.tx
+	}
 	atomic.AddInt64(&mem.txsBytes, int64(-len(tx)))
 
 	if removeFromCache {
@@ -617,15 +620,6 @@ func (mem *CListMempool) Update(
 		// https://github.com/tendermint/tendermint/issues/3322.
 		if e, ok := mem.txsMap.Load(tx.Key()); ok {
 			mem.removeTx(tx, e.(*clist.CElement), false)
-			// see if the transaction is a malleated transaction of a some parent
-			// transaction that exists in the mempool
-		} else if malleatedTx, isMalleated := types.UnwrapMalleatedTx(tx); isMalleated {
-			var parentKey [types.TxKeySize]byte
-			copy(parentKey[:], malleatedTx.OriginalTxHash)
-			err := mem.RemoveTxByKey(parentKey)
-			if err != nil {
-				return err
-			}
 		}
 	}
 

diff --git a/mempool/v0/clist_mempool_test.go b/mempool/v0/clist_mempool_test.go
@@ -25,6 +25,7 @@ import (
 	tmrand "github.com/tendermint/tendermint/libs/rand"
 	"github.com/tendermint/tendermint/libs/service"
 	"github.com/tendermint/tendermint/mempool"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	"github.com/tendermint/tendermint/proxy"
 	"github.com/tendermint/tendermint/types"
 )
@@ -680,6 +681,38 @@ func TestMempoolRemoteAppConcurrency(t *testing.T) {
 	require.NoError(t, mp.FlushAppConn())
 }
 
+func TestRemoveBlobTx(t *testing.T) {
+	app := kvstore.NewApplication()
+	cc := proxy.NewLocalClientCreator(app)
+
+	cfg := config.ResetTestRoot("mempool_test")
+
+	cfg.Mempool.MaxTxsBytes = 1000
+	mp, cleanup := newMempoolWithAppAndConfig(cc, cfg)
+	defer cleanup()
+
+	originalTx := []byte{1, 2, 3, 4}
+	malleatedTx, err := types.MarshalMalleatedTx(100, originalTx)
+	require.NoError(t, err)
+
+	// create the blobTx
+	b := tmproto.Blob{
+		NamespaceId:  []byte{1, 2, 3, 4, 5, 6, 7, 8},
+		Data:         []byte{1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+		ShareVersion: 0,
+	}
+	bTx, err := types.MarshalBlobTx(originalTx, &b)
+	require.NoError(t, err)
+
+	err = mp.CheckTx(bTx, nil, mempool.TxInfo{})
+	require.NoError(t, err)
+
+	err = mp.Update(1, []types.Tx{malleatedTx}, abciResponses(1, abci.CodeTypeOK), nil, nil)
+	require.NoError(t, err)
+	assert.EqualValues(t, 0, mp.Size())
+	assert.EqualValues(t, 0, mp.SizeBytes())
+}
+
 // caller must close server
 func newRemoteApp(t *testing.T, addr string, app abci.Application) (abciclient.Client, service.Service) {
 	clientCreator, err := abciclient.NewClient(addr, "socket", true)

diff --git a/mempool/v1/mempool.go b/mempool/v1/mempool.go
@@ -402,26 +402,17 @@ func (txmp *TxMempool) Update(
 
 	txmp.metrics.SuccessfulTxs.Add(float64(len(blockTxs)))
 	for i, tx := range blockTxs {
-		// The tx hash corresponding to the originating transaction. Set to the
-		// current tx hash initially, but can change in case of a malleated tx.
-		originalKey := tx.Key()
-
-		// Regardless of outcome, remove the transaction from the mempool.
-		if err := txmp.removeTxByKey(originalKey); err != nil {
-			if malleatedTx, isMalleated := types.UnwrapMalleatedTx(tx); isMalleated {
-				copy(originalKey[:], malleatedTx.OriginalTxHash)
-				_ = txmp.removeTxByKey(originalKey)
-			}
-		}
-
 		// Add successful committed transactions to the cache (if they are not
 		// already present).  Transactions that failed to commit are removed from
 		// the cache unless the operator has explicitly requested we keep them.
 		if deliverTxResponses[i].Code == abci.CodeTypeOK {
 			_ = txmp.cache.Push(tx)
 		} else if !txmp.config.KeepInvalidTxsInCache {
-			txmp.cache.RemoveTxByKey(originalKey)
+			txmp.cache.Remove(tx)
 		}
+
+		// Regardless of success, remove the transaction from the mempool.
+		_ = txmp.removeTxByKey(tx.Key())
 	}
 
 	txmp.purgeExpiredTxs(blockHeight)

diff --git a/mempool/v1/mempool_test.go b/mempool/v1/mempool_test.go
@@ -2,7 +2,6 @@ package v1
 
 import (
 	"bytes"
-	"crypto/sha256"
 	"errors"
 	"fmt"
 	"math/rand"
@@ -14,6 +13,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/tendermint/tendermint/abci/example/code"
@@ -22,6 +22,7 @@ import (
 	"github.com/tendermint/tendermint/config"
 	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/mempool"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	"github.com/tendermint/tendermint/proxy"
 	"github.com/tendermint/tendermint/types"
 )
@@ -654,23 +655,29 @@ func TestTxMempool_CheckTxPostCheckError(t *testing.T) {
 	}
 }
 
-func TestMalleatedTxRemoval(t *testing.T) {
+func TestRemoveBlobTx(t *testing.T) {
 	txmp := setup(t, 500)
+
 	originalTx := []byte{1, 2, 3, 4}
-	malleatedTx := []byte{1, 2}
-	originalHash := sha256.Sum256(originalTx)
+	malleatedTx, err := types.MarshalMalleatedTx(100, originalTx)
+	require.NoError(t, err)
 
-	// create the wrapped child transaction
-	wTx, err := types.WrapMalleatedTx(originalHash[:], 0, malleatedTx)
+	// create the blobTx
+	b := tmproto.Blob{
+		NamespaceId:  []byte{1, 2, 3, 4, 5, 6, 7, 8},
+		Data:         []byte{1, 2, 3, 4, 5, 6, 7, 8, 1, 2, 3, 4, 5, 6, 7, 8, 9},
+		ShareVersion: 0,
+	}
+	bTx, err := types.MarshalBlobTx(originalTx, &b)
 	require.NoError(t, err)
 
-	// add the parent transaction to the mempool
-	err = txmp.CheckTx(originalTx, nil, mempool.TxInfo{})
+	err = txmp.CheckTx(bTx, nil, mempool.TxInfo{})
 	require.NoError(t, err)
 
-	// remove the parent from the mempool using the wrapped child tx
-	err = txmp.Update(1, []types.Tx{wTx}, abciResponses(1, abci.CodeTypeOK), nil, nil)
+	err = txmp.Update(1, []types.Tx{malleatedTx}, abciResponses(1, abci.CodeTypeOK), nil, nil)
 	require.NoError(t, err)
+	assert.EqualValues(t, 0, txmp.Size())
+	assert.EqualValues(t, 0, txmp.SizeBytes())
 }
 
 func abciResponses(n int, code uint32) []*abci.ResponseDeliverTx {

diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go
@@ -11,6 +11,14 @@ const (
 	// tx inclusion proof via the ABCI Query method. The desired transaction
 	// index must be formatted into the path.
 	TxInclusionProofQueryPath = "custom/txInclusionProof/%d"
+
+	// ProtoBlobTxTypeID is included in each encoded BlobTx to help prevent
+	// decoding binaries that are not actually BlobTxs.
+	ProtoBlobTxTypeID = "BLOB"
+
+	// ProtoMalleatedTxTypeID is included in each encoded MalleatedTx to help prevent
+	// decoding binaries that are not actually MalleatedTxs.
+	ProtoMalleatedTxTypeID = "MLTD"
 )
 
 var (

diff --git a/proto/tendermint/abci/types.proto b/proto/tendermint/abci/types.proto
@@ -362,11 +362,10 @@ message EventAttribute {
 //
 // One usage is indexing transaction results.
 message TxResult {
-  int64             height        = 1;
-  uint32            index         = 2;
-  bytes             tx            = 3;
-  ResponseDeliverTx result        = 4 [(gogoproto.nullable) = false];
-  bytes             original_hash = 5;
+  int64             height = 1;
+  uint32            index  = 2;
+  bytes             tx     = 3;
+  ResponseDeliverTx result = 4 [(gogoproto.nullable) = false];
 }
 
 //----------------------------------------

diff --git a/proto/tendermint/types/block.proto b/proto/tendermint/types/block.proto
@@ -8,8 +8,8 @@ import "tendermint/types/types.proto";
 import "tendermint/types/evidence.proto";
 
 message Block {
-  Header header      = 1 [(gogoproto.nullable) = false];
-  Data   data        = 2 [(gogoproto.nullable) = false];
+  Header                        header      = 1 [(gogoproto.nullable) = false];
+  Data                          data        = 2 [(gogoproto.nullable) = false];
   tendermint.types.EvidenceList evidence    = 3 [(gogoproto.nullable) = false];
-  Commit last_commit = 4;
+  Commit                        last_commit = 4;
 }
diff --git a/proto/tendermint/types/evidence.proto b/proto/tendermint/types/evidence.proto
@@ -17,20 +17,20 @@ message Evidence {
 
 // DuplicateVoteEvidence contains evidence of a validator signed two conflicting votes.
 message DuplicateVoteEvidence {
-  tendermint.types.Vote       vote_a = 1;
-  tendermint.types.Vote       vote_b = 2;
-  int64                       total_voting_power = 3;
-  int64                       validator_power = 4;
-  google.protobuf.Timestamp   timestamp = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
+  tendermint.types.Vote     vote_a             = 1;
+  tendermint.types.Vote     vote_b             = 2;
+  int64                     total_voting_power = 3;
+  int64                     validator_power    = 4;
+  google.protobuf.Timestamp timestamp          = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
 }
 
 // LightClientAttackEvidence contains evidence of a set of validators attempting to mislead a light client.
 message LightClientAttackEvidence {
-  tendermint.types.LightBlock         conflicting_block = 1;
-  int64                               common_height     = 2;
+  tendermint.types.LightBlock conflicting_block            = 1;
+  int64                       common_height                = 2;
   repeated tendermint.types.Validator byzantine_validators = 3;
-  int64                               total_voting_power = 4;
-  google.protobuf.Timestamp           timestamp = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
+  int64                               total_voting_power   = 4;
+  google.protobuf.Timestamp           timestamp            = 5 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
 }
 
 message EvidenceList {