Message inclusion check (#216)

* convert PreprocessTxs to PrepareProposal * add message inclusion check to ProcessProposal * use the malleated tx decoder to decode malleated transactions * use constant for pay for message URL * switch from PayForMessage to PayForData * fix integration test * address #226 * change missed var name Co-authored-by: Ismail Khoffi <[email protected]> * get missed pfm vs pfd Co-authored-by: Ismail Khoffi <[email protected]> * pfm -> pfd Co-authored-by: John Adler <[email protected]> * better wording Co-authored-by: John Adler <[email protected]> * better wording Co-authored-by: John Adler <[email protected]> * message -> data Co-authored-by: John Adler <[email protected]> * pfm -> pfd Co-authored-by: Ismail Khoffi <[email protected]> * remaining pfms -> pfds * get rid of empty space Co-authored-by: John Adler <[email protected]> * get rid of empty space Co-authored-by: John Adler <[email protected]> * log URL type mismatch * add todo for refactoring to check for subtree roots Co-authored-by: Ismail Khoffi <[email protected]> Co-authored-by: John Adler <[email protected]>
celestiaorg · May 5, 2022 · b2617e4 · b2617e4
1 parent d7bb393
commit b2617e4
Show file tree

Hide file tree

Showing 5 changed files with 270 additions and 4 deletions.
diff --git a/app/abci.go → app/prepare_proposal.go b/app/abci.go → app/prepare_proposal.go
diff --git a/app/process_proposal.go b/app/process_proposal.go
@@ -0,0 +1,88 @@
+package app
+
+import (
+	"github.com/celestiaorg/celestia-app/x/payment/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+const (
+	rejectedPropBlockLog = "Rejected proposal block:"
+)
+
+func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponseProcessProposal {
+	// Check for message inclusion:
+	//  - each MsgPayForData included in a block should have a corresponding data also in the block body
+	//  - the commitment in each PFD should match that of its corresponding data
+	//  - there should be no unpaid-for data
+
+	// extract the commitments from any MsgPayForDatas in the block
+	commitments := make(map[string]struct{})
+	// we have a separate counter so that identical data also get counted
+	// also see https://github.com/celestiaorg/celestia-app/issues/226
+	commitmentCounter := 0
+	for _, rawTx := range req.BlockData.Txs {
+		tx, err := MalleatedTxDecoder(app.txConfig.TxDecoder())(rawTx)
+		if err != nil {
+			continue
+		}
+
+		for _, msg := range tx.GetMsgs() {
+			if sdk.MsgTypeURL(msg) != types.URLMsgPayForData {
+				continue
+			}
+
+			pfd, ok := msg.(*types.MsgPayForData)
+			if !ok {
+				app.Logger().Error("Msg type does not match MsgPayForData URL")
+				continue
+			}
+
+			commitments[string(pfd.MessageShareCommitment)] = struct{}{}
+			commitmentCounter++
+		}
+	}
+
+	// quickly compare the number of PFDs and messages, if they aren't
+	// identical, then  we already know this block is invalid
+	if commitmentCounter != len(req.BlockData.Messages.MessagesList) {
+		app.Logger().Error(
+			rejectedPropBlockLog,
+			"reason",
+			"varying number of messages and payForData txs in the same block",
+		)
+		return abci.ResponseProcessProposal{
+			Result: abci.ResponseProcessProposal_REJECT,
+		}
+	}
+
+	// iterate through all of the messages and ensure that a PFD with the exact
+	// commitment exists
+	for _, msg := range req.BlockData.Messages.MessagesList {
+		commit, err := types.CreateCommitment(app.SquareSize(), msg.NamespaceId, msg.Data)
+		if err != nil {
+			app.Logger().Error(
+				rejectedPropBlockLog,
+				"reason",
+				"failure to create commitment for included message",
+				"error",
+				err.Error(),
+			)
+			return abci.ResponseProcessProposal{
+				Result: abci.ResponseProcessProposal_REJECT,
+			}
+		}
+
+		// TODO: refactor to actually check for subtree roots instead of simply inclusion see issues #382 and #383
+		if _, has := commitments[string(commit)]; !has {
+			app.Logger().Info(rejectedPropBlockLog, "reason", "missing MsgPayForData for included message")
+			return abci.ResponseProcessProposal{
+				Result: abci.ResponseProcessProposal_REJECT,
+			}
+		}
+	}
+
+	return abci.ResponseProcessProposal{
+		Result: abci.ResponseProcessProposal_ACCEPT,
+	}
+}
diff --git a/app/test/abci_test.go → app/test/prepare_proposal_test.go b/app/test/abci_test.go → app/test/prepare_proposal_test.go
diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go
@@ -0,0 +1,177 @@
+package app_test
+
+import (
+	"crypto/rand"
+	"testing"
+
+	"github.com/celestiaorg/celestia-app/app"
+	"github.com/celestiaorg/celestia-app/testutil"
+	"github.com/celestiaorg/celestia-app/x/payment/types"
+	"github.com/cosmos/cosmos-sdk/client"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/spm/cosmoscmd"
+	abci "github.com/tendermint/tendermint/abci/types"
+	tmrand "github.com/tendermint/tendermint/libs/rand"
+	"github.com/tendermint/tendermint/pkg/consts"
+	core "github.com/tendermint/tendermint/proto/tendermint/types"
+)
+
+func TestMessageInclusionCheck(t *testing.T) {
+	signer := testutil.GenerateKeyringSigner(t, testAccName)
+	info := signer.GetSignerInfo()
+
+	testApp := testutil.SetupTestApp(t, info.GetAddress())
+
+	encConf := cosmoscmd.MakeEncodingConfig(app.ModuleBasics)
+
+	firstValidPFD, msg1 := genRandMsgPayForData(t, signer)
+	secondValidPFD, msg2 := genRandMsgPayForData(t, signer)
+
+	invalidCommitmentPFD, msg3 := genRandMsgPayForData(t, signer)
+	invalidCommitmentPFD.MessageShareCommitment = tmrand.Bytes(32)
+
+	// block with all messages included
+	validData := core.Data{
+		Txs: [][]byte{
+			buildTx(t, signer, encConf.TxConfig, firstValidPFD),
+			buildTx(t, signer, encConf.TxConfig, secondValidPFD),
+		},
+		Messages: core.Messages{
+			MessagesList: []*core.Message{
+				{
+					NamespaceId: firstValidPFD.MessageNamespaceId,
+					Data:        msg1,
+				},
+				{
+					NamespaceId: secondValidPFD.MessageNamespaceId,
+					Data:        msg2,
+				},
+			},
+		},
+	}
+
+	// block with a missing message
+	missingMessageData := core.Data{
+		Txs: [][]byte{
+			buildTx(t, signer, encConf.TxConfig, firstValidPFD),
+			buildTx(t, signer, encConf.TxConfig, secondValidPFD),
+		},
+		Messages: core.Messages{
+			MessagesList: []*core.Message{
+				{
+					NamespaceId: firstValidPFD.MessageNamespaceId,
+					Data:        msg1,
+				},
+			},
+		},
+	}
+
+	// block with all messages included, but the commitment is changed
+	invalidData := core.Data{
+		Txs: [][]byte{
+			buildTx(t, signer, encConf.TxConfig, firstValidPFD),
+			buildTx(t, signer, encConf.TxConfig, secondValidPFD),
+		},
+		Messages: core.Messages{
+			MessagesList: []*core.Message{
+				{
+					NamespaceId: firstValidPFD.MessageNamespaceId,
+					Data:        msg1,
+				},
+				{
+					NamespaceId: invalidCommitmentPFD.MessageNamespaceId,
+					Data:        msg3,
+				},
+			},
+		},
+	}
+
+	// block with all messages included
+	extraMessageData := core.Data{
+		Txs: [][]byte{
+			buildTx(t, signer, encConf.TxConfig, firstValidPFD),
+		},
+		Messages: core.Messages{
+			MessagesList: []*core.Message{
+				{
+					NamespaceId: firstValidPFD.MessageNamespaceId,
+					Data:        msg1,
+				},
+				{
+					NamespaceId: secondValidPFD.MessageNamespaceId,
+					Data:        msg2,
+				},
+			},
+		},
+	}
+
+	type test struct {
+		input          abci.RequestProcessProposal
+		expectedResult abci.ResponseProcessProposal_Result
+	}
+
+	tests := []test{
+		{
+			input: abci.RequestProcessProposal{
+				BlockData: &validData,
+			},
+			expectedResult: abci.ResponseProcessProposal_ACCEPT,
+		},
+		{
+			input: abci.RequestProcessProposal{
+				BlockData: &missingMessageData,
+			},
+			expectedResult: abci.ResponseProcessProposal_REJECT,
+		},
+		{
+			input: abci.RequestProcessProposal{
+				BlockData: &invalidData,
+			},
+			expectedResult: abci.ResponseProcessProposal_REJECT,
+		},
+		{
+			input: abci.RequestProcessProposal{
+				BlockData: &extraMessageData,
+			},
+			expectedResult: abci.ResponseProcessProposal_REJECT,
+		},
+	}
+
+	for _, tt := range tests {
+		res := testApp.ProcessProposal(tt.input)
+		assert.Equal(t, tt.expectedResult, res.Result)
+	}
+
+}
+
+func genRandMsgPayForData(t *testing.T, signer *types.KeyringSigner) (*types.MsgPayForData, []byte) {
+	ns := make([]byte, consts.NamespaceSize)
+	_, err := rand.Read(ns)
+	require.NoError(t, err)
+
+	message := make([]byte, tmrand.Intn(3000))
+	_, err = rand.Read(message)
+	require.NoError(t, err)
+
+	commit, err := types.CreateCommitment(consts.MaxSquareSize, ns, message)
+	require.NoError(t, err)
+
+	pfd := types.MsgPayForData{
+		MessageShareCommitment: commit,
+		MessageNamespaceId:     ns,
+	}
+
+	return &pfd, message
+}
+
+func buildTx(t *testing.T, signer *types.KeyringSigner, txCfg client.TxConfig, msg sdk.Msg) []byte {
+	tx, err := signer.BuildSignedTx(signer.NewTxBuilder(), msg)
+	require.NoError(t, err)
+
+	rawTx, err := txCfg.TxEncoder()(tx)
+	require.NoError(t, err)
+
+	return rawTx
+}
diff --git a/x/payment/payfordata.go b/x/payment/payfordata.go
@@ -2,6 +2,7 @@ package payment
 
 import (
 	"context"
+
 	"google.golang.org/grpc"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -60,7 +61,7 @@ func BuildPayForData(
 	message []byte,
 	gasLim uint64,
 ) (*types.MsgWirePayForData, error) {
-	// create the raw WirePayForMessage transaction
+	// create the raw WirePayForData transaction
 	wpfd, err := types.NewWirePayForData(nID, message, shareSizes...)
 	if err != nil {
 		return nil, err
@@ -72,7 +73,7 @@ func BuildPayForData(
 		return nil, err
 	}
 
-	// generate the signatures for each `MsgPayForMessage` using the `KeyringSigner`,
+	// generate the signatures for each `MsgPayForData` using the `KeyringSigner`,
 	// then set the gas limit for the tx
 	gasLimOption := types.SetGasLimit(gasLim)
 	err = wpfd.SignShareCommitments(signer, gasLimOption)
@@ -89,8 +90,8 @@ func SignPayForData(
 	pfd *types.MsgWirePayForData,
 	opts ...types.TxBuilderOption,
 ) (signing.Tx, error) {
-	// Build and sign the final `WirePayForMessage` tx that now contains the signatures
-	// for potential `MsgPayForMessage`s
+	// Build and sign the final `WirePayForData` tx that now contains the signatures
+	// for potential `MsgPayForData`s
 	builder := signer.NewTxBuilder()
 	for _, opt := range opts {
 		opt(builder)