diff --git a/.gitignore b/.gitignore index cf22a4a568..d1de802be9 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,3 @@ build coverage.txt tools-stamp .vscode -.idea diff --git a/app/process_proposal.go b/app/process_proposal.go index b48fdf120e..947814de3b 100644 --- a/app/process_proposal.go +++ b/app/process_proposal.go @@ -63,6 +63,19 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponsePr // iterate through all of the messages and ensure that a PFD with the exact // commitment exists for _, msg := range req.BlockData.Messages.MessagesList { + if err := types.ValidateMessageNamespaceID(msg.NamespaceId); err != nil { + app.Logger().Error( + rejectedPropBlockLog, + "reason", + "found a message that uses an invalid namespace id", + "error", + err.Error(), + ) + return abci.ResponseProcessProposal{ + Result: abci.ResponseProcessProposal_REJECT, + } + } + commit, err := types.CreateCommitment(req.BlockData.OriginalSquareSize, msg.NamespaceId, msg.Data) if err != nil { app.Logger().Error( diff --git a/app/split_shares.go b/app/split_shares.go index 4f1ce2716d..2eb106a0fe 100644 --- a/app/split_shares.go +++ b/app/split_shares.go @@ -60,11 +60,18 @@ func SplitShares(txConf client.TxConfig, squareSize uint64, data *core.Data) ([] } msg := authTx.GetMsgs()[0] + wireMsg, ok := msg.(*types.MsgWirePayForData) if !ok { continue } + // run basic validation on the message + err = wireMsg.ValidateBasic() + if err != nil { + continue + } + // run basic validation on the transaction (which include the wireMsg // above) err = authTx.ValidateBasic() diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index 236bd7dd40..f8593d85ab 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -8,12 +8,14 @@ import ( "github.com/celestiaorg/celestia-app/app/encoding" "github.com/celestiaorg/celestia-app/testutil" "github.com/celestiaorg/celestia-app/x/payment/types" + "github.com/celestiaorg/nmt/namespace" "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/pkg/consts" core "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -102,6 +104,39 @@ func TestPrepareProposal(t *testing.T) { } } +func TestPrepareMessagesWithReservedNamespaces(t *testing.T) { + testApp := testutil.SetupTestAppWithGenesisValSet(t) + encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) + + signer := testutil.GenerateKeyringSigner(t, testAccName) + + type test struct { + name string + namespace namespace.ID + expectedMessages int + } + + tests := []test{ + {"transaction namespace id for message", consts.TxNamespaceID, 0}, + {"evidence namespace id for message", consts.EvidenceNamespaceID, 0}, + {"tail padding namespace id for message", consts.TailPaddingNamespaceID, 0}, + {"parity shares namespace id for message", consts.ParitySharesNamespaceID, 0}, + {"reserved namespace id for message", namespace.ID{0, 0, 0, 0, 0, 0, 0, 200}, 0}, + {"valid namespace id for message", namespace.ID{3, 3, 2, 2, 2, 1, 1, 1}, 1}, + } + + for _, tt := range tests { + tx := generateRawTx(t, encCfg.TxConfig, tt.namespace, []byte{1}, signer, 2, 4, 8, 16) + input := abci.RequestPrepareProposal{ + BlockData: &core.Data{ + Txs: [][]byte{tx}, + }, + } + res := testApp.PrepareProposal(input) + assert.Equal(t, tt.expectedMessages, len(res.BlockData.Messages.MessagesList)) + } +} + func generateRawTx(t *testing.T, txConfig client.TxConfig, ns, message []byte, signer *types.KeyringSigner, ks ...uint64) (rawTx []byte) { coin := sdk.Coin{ Denom: app.BondDenom, diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index d2a46886ec..dd878b2ff6 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -9,6 +9,7 @@ import ( "github.com/celestiaorg/celestia-app/app/encoding" "github.com/celestiaorg/celestia-app/testutil" "github.com/celestiaorg/celestia-app/x/payment/types" + "github.com/celestiaorg/nmt/namespace" "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/assert" @@ -164,13 +165,99 @@ func TestMessageInclusionCheck(t *testing.T) { } } +func TestProcessMessagesWithReservedNamespaces(t *testing.T) { + testApp := testutil.SetupTestAppWithGenesisValSet(t) + encConf := encoding.MakeConfig(app.ModuleEncodingRegisters...) + + signer := testutil.GenerateKeyringSigner(t, testAccName) + + type test struct { + name string + namespace namespace.ID + expectedResult abci.ResponseProcessProposal_Result + } + + tests := []test{ + {"transaction namespace id for message", consts.TxNamespaceID, abci.ResponseProcessProposal_REJECT}, + {"evidence namespace id for message", consts.EvidenceNamespaceID, abci.ResponseProcessProposal_REJECT}, + {"tail padding namespace id for message", consts.TailPaddingNamespaceID, abci.ResponseProcessProposal_REJECT}, + {"namespace id 200 for message", namespace.ID{0, 0, 0, 0, 0, 0, 0, 200}, abci.ResponseProcessProposal_REJECT}, + {"correct namespace id for message", namespace.ID{3, 3, 2, 2, 2, 1, 1, 1}, abci.ResponseProcessProposal_ACCEPT}, + } + + for _, tt := range tests { + pfd, msg := genRandMsgPayForDataForNamespace(t, signer, 8, tt.namespace) + input := abci.RequestProcessProposal{ + BlockData: &core.Data{ + Txs: [][]byte{ + buildTx(t, signer, encConf.TxConfig, pfd), + }, + Messages: core.Messages{ + MessagesList: []*core.Message{ + { + NamespaceId: pfd.GetMessageNamespaceId(), + Data: msg, + }, + }, + }, + OriginalSquareSize: 8, + }, + } + data, err := coretypes.DataFromProto(input.BlockData) + require.NoError(t, err) + + shares, _, err := data.ComputeShares(input.BlockData.OriginalSquareSize) + require.NoError(t, err) + + rawShares := shares.RawShares() + + require.NoError(t, err) + eds, err := da.ExtendShares(input.BlockData.OriginalSquareSize, rawShares) + require.NoError(t, err) + dah := da.NewDataAvailabilityHeader(eds) + input.Header.DataHash = dah.Hash() + res := testApp.ProcessProposal(input) + assert.Equal(t, tt.expectedResult, res.Result) + } +} + +func TestProcessMessageWithParityShareNamespaces(t *testing.T) { + testApp := testutil.SetupTestAppWithGenesisValSet(t) + encConf := encoding.MakeConfig(app.ModuleEncodingRegisters...) + + signer := testutil.GenerateKeyringSigner(t, testAccName) + + pfd, msg := genRandMsgPayForDataForNamespace(t, signer, 8, consts.ParitySharesNamespaceID) + input := abci.RequestProcessProposal{ + BlockData: &core.Data{ + Txs: [][]byte{ + buildTx(t, signer, encConf.TxConfig, pfd), + }, + Messages: core.Messages{ + MessagesList: []*core.Message{ + { + NamespaceId: pfd.GetMessageNamespaceId(), + Data: msg, + }, + }, + }, + OriginalSquareSize: 8, + }, + } + res := testApp.ProcessProposal(input) + assert.Equal(t, abci.ResponseProcessProposal_REJECT, res.Result) +} + func genRandMsgPayForData(t *testing.T, signer *types.KeyringSigner, squareSize uint64) (*types.MsgPayForData, []byte) { ns := make([]byte, consts.NamespaceSize) _, err := rand.Read(ns) require.NoError(t, err) + return genRandMsgPayForDataForNamespace(t, signer, squareSize, ns) +} +func genRandMsgPayForDataForNamespace(t *testing.T, signer *types.KeyringSigner, squareSize uint64, ns namespace.ID) (*types.MsgPayForData, []byte) { message := make([]byte, randomInt(20)) - _, err = rand.Read(message) + _, err := rand.Read(message) require.NoError(t, err) commit, err := types.CreateCommitment(squareSize, ns, message) diff --git a/x/payment/types/errors.go b/x/payment/types/errors.go index baf4180880..ebc8cadc02 100644 --- a/x/payment/types/errors.go +++ b/x/payment/types/errors.go @@ -16,4 +16,6 @@ var ( ErrInvalidShareCommit = sdkerrors.Register(ModuleName, 11116, "invalid commit for share") ErrParitySharesNamespace = sdkerrors.Register(ModuleName, 11117, "cannot use parity shares namespace ID") ErrTailPaddingNamespace = sdkerrors.Register(ModuleName, 11118, "cannot use tail padding namespace ID") + ErrTxNamespace = sdkerrors.Register(ModuleName, 11119, "cannot use transaction namespace ID") + ErrEvidenceNamespace = sdkerrors.Register(ModuleName, 11120, "cannot use evidence namespace ID") ) diff --git a/x/payment/types/wirepayfordata.go b/x/payment/types/wirepayfordata.go index b1f75a98ee..7bc598aae7 100644 --- a/x/payment/types/wirepayfordata.go +++ b/x/payment/types/wirepayfordata.go @@ -5,6 +5,7 @@ import ( "errors" fmt "fmt" + "github.com/celestiaorg/nmt/namespace" sdkclient "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -84,12 +85,8 @@ func (msg *MsgWirePayForData) Route() string { return RouterKey } // commitments, signatures for those share commitments, and fulfills the sdk.Msg // interface. func (msg *MsgWirePayForData) ValidateBasic() error { - // ensure that the namespace id is of length == NamespaceIDSize - if nsLen := len(msg.GetMessageNameSpaceId()); nsLen != NamespaceIDSize { - return ErrInvalidNamespaceLen.Wrapf("got: %d want: %d", - nsLen, - NamespaceIDSize, - ) + if err := ValidateMessageNamespaceID(msg.GetMessageNameSpaceId()); err != nil { + return err } if _, err := sdk.AccAddressFromBech32(msg.Signer); err != nil { @@ -105,21 +102,6 @@ func (msg *MsgWirePayForData) ValidateBasic() error { ) } - // ensure that a reserved namespace is not used - if bytes.Compare(msg.GetMessageNameSpaceId(), consts.MaxReservedNamespace) < 1 { - return ErrReservedNamespace.Wrapf("got namespace: %x, want: > %x", msg.GetMessageNameSpaceId(), consts.MaxReservedNamespace) - } - - // ensure that ParitySharesNamespaceID is not used - if bytes.Equal(msg.GetMessageNameSpaceId(), consts.ParitySharesNamespaceID) { - return ErrParitySharesNamespace - } - - // ensure that TailPaddingNamespaceID is not used - if bytes.Equal(msg.GetMessageNameSpaceId(), consts.TailPaddingNamespaceID) { - return ErrTailPaddingNamespace - } - for idx, commit := range msg.MessageShareCommitment { // check that each commit is valid if !powerOf2(commit.K) { @@ -139,6 +121,33 @@ func (msg *MsgWirePayForData) ValidateBasic() error { return nil } +// ValidateMessageNamespaceID returns an error if the provided namespace.ID is an invalid or reserved namespace id. +func ValidateMessageNamespaceID(ns namespace.ID) error { + // ensure that the namespace id is of length == NamespaceIDSize + if nsLen := len(ns); nsLen != NamespaceIDSize { + return ErrInvalidNamespaceLen.Wrapf("got: %d want: %d", + nsLen, + NamespaceIDSize, + ) + } + // ensure that a reserved namespace is not used + if bytes.Compare(ns, consts.MaxReservedNamespace) < 1 { + return ErrReservedNamespace.Wrapf("got namespace: %x, want: > %x", ns, consts.MaxReservedNamespace) + } + + // ensure that ParitySharesNamespaceID is not used + if bytes.Equal(ns, consts.ParitySharesNamespaceID) { + return ErrParitySharesNamespace + } + + // ensure that TailPaddingNamespaceID is not used + if bytes.Equal(ns, consts.TailPaddingNamespaceID) { + return ErrTailPaddingNamespace + } + + return nil +} + // GetSigners returns the addresses of the message signers func (msg *MsgWirePayForData) GetSigners() []sdk.AccAddress { address, err := sdk.AccAddressFromBech32(msg.Signer)