diff --git a/app/process_proposal.go b/app/process_proposal.go index 039d4572d5..7f634d3f1d 100644 --- a/app/process_proposal.go +++ b/app/process_proposal.go @@ -3,14 +3,15 @@ package app import ( "bytes" - shares "github.com/celestiaorg/celestia-app/pkg/shares" - "github.com/celestiaorg/celestia-app/x/payment/types" sdk "github.com/cosmos/cosmos-sdk/types" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/pkg/da" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" + + shares "github.com/celestiaorg/celestia-app/pkg/shares" + "github.com/celestiaorg/celestia-app/x/payment/types" ) const ( diff --git a/app/split_shares.go b/app/split_shares.go index 53d0532b5d..ce123b8b83 100644 --- a/app/split_shares.go +++ b/app/split_shares.go @@ -5,13 +5,14 @@ import ( "crypto/sha256" "sort" - shares "github.com/celestiaorg/celestia-app/pkg/shares" - "github.com/celestiaorg/celestia-app/x/payment/types" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/tendermint/tendermint/pkg/consts" core "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" + + shares "github.com/celestiaorg/celestia-app/pkg/shares" + "github.com/celestiaorg/celestia-app/x/payment/types" ) // SplitShares uses the provided block data to create a flattened data square. diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index f8593d85ab..7cc737963d 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -4,10 +4,6 @@ import ( "bytes" "testing" - "github.com/celestiaorg/celestia-app/app" - "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" @@ -17,6 +13,11 @@ import ( abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/pkg/consts" core "github.com/tendermint/tendermint/proto/tendermint/types" + + "github.com/celestiaorg/celestia-app/app" + "github.com/celestiaorg/celestia-app/app/encoding" + "github.com/celestiaorg/celestia-app/testutil" + "github.com/celestiaorg/celestia-app/x/payment/types" ) func TestPrepareProposal(t *testing.T) { @@ -34,15 +35,15 @@ func TestPrepareProposal(t *testing.T) { firstNS := []byte{2, 2, 2, 2, 2, 2, 2, 2} firstMessage := bytes.Repeat([]byte{4}, 512) - firstRawTx := generateRawTx(t, encCfg.TxConfig, firstNS, firstMessage, signer, 2, 4, 8, 16) + firstRawTx := generateRawTx(t, encCfg.TxConfig, firstNS, firstMessage, signer, types.AllSquareSizes(len(firstMessage))...) secondNS := []byte{1, 1, 1, 1, 1, 1, 1, 1} secondMessage := []byte{2} - secondRawTx := generateRawTx(t, encCfg.TxConfig, secondNS, secondMessage, signer, 2, 4, 8, 16) + secondRawTx := generateRawTx(t, encCfg.TxConfig, secondNS, secondMessage, signer, types.AllSquareSizes(len(secondMessage))...) thirdNS := []byte{3, 3, 3, 3, 3, 3, 3, 3} thirdMessage := []byte{1} - thirdRawTx := generateRawTx(t, encCfg.TxConfig, thirdNS, thirdMessage, signer, 2, 4, 8, 16) + thirdRawTx := generateRawTx(t, encCfg.TxConfig, thirdNS, thirdMessage, signer, types.AllSquareSizes(len(thirdMessage))...) tests := []test{ { @@ -126,7 +127,8 @@ func TestPrepareMessagesWithReservedNamespaces(t *testing.T) { } for _, tt := range tests { - tx := generateRawTx(t, encCfg.TxConfig, tt.namespace, []byte{1}, signer, 2, 4, 8, 16) + message := []byte{1} + tx := generateRawTx(t, encCfg.TxConfig, tt.namespace, message, signer, types.AllSquareSizes(len(message))...) input := abci.RequestPrepareProposal{ BlockData: &core.Data{ Txs: [][]byte{tx}, diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index db816ad189..d88af1dff3 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -5,11 +5,6 @@ import ( "math/big" "testing" - "github.com/celestiaorg/celestia-app/app" - "github.com/celestiaorg/celestia-app/app/encoding" - shares "github.com/celestiaorg/celestia-app/pkg/shares" - "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" @@ -21,6 +16,12 @@ import ( "github.com/tendermint/tendermint/pkg/da" core "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" + + "github.com/celestiaorg/celestia-app/app" + "github.com/celestiaorg/celestia-app/app/encoding" + shares "github.com/celestiaorg/celestia-app/pkg/shares" + "github.com/celestiaorg/celestia-app/testutil" + "github.com/celestiaorg/celestia-app/x/payment/types" ) func TestMessageInclusionCheck(t *testing.T) { diff --git a/app/test/split_shares_test.go b/app/test/split_shares_test.go index 6962425ed3..10a8bc930f 100644 --- a/app/test/split_shares_test.go +++ b/app/test/split_shares_test.go @@ -4,15 +4,17 @@ import ( "bytes" "testing" - "github.com/celestiaorg/celestia-app/app" - "github.com/celestiaorg/celestia-app/app/encoding" - shares "github.com/celestiaorg/celestia-app/pkg/shares" - "github.com/celestiaorg/celestia-app/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/pkg/consts" "github.com/tendermint/tendermint/pkg/da" core "github.com/tendermint/tendermint/proto/tendermint/types" + + "github.com/celestiaorg/celestia-app/app" + "github.com/celestiaorg/celestia-app/app/encoding" + shares "github.com/celestiaorg/celestia-app/pkg/shares" + "github.com/celestiaorg/celestia-app/testutil" + "github.com/celestiaorg/celestia-app/x/payment/types" ) func TestSplitShares(t *testing.T) { @@ -28,15 +30,16 @@ func TestSplitShares(t *testing.T) { firstNS := []byte{2, 2, 2, 2, 2, 2, 2, 2} firstMessage := bytes.Repeat([]byte{4}, 512) - firstRawTx := generateRawTx(t, encCfg.TxConfig, firstNS, firstMessage, signer, 2, 4, 8) + firstRawTx := generateRawTx(t, encCfg.TxConfig, firstNS, firstMessage, signer, types.AllSquareSizes(len(firstMessage))...) secondNS := []byte{1, 1, 1, 1, 1, 1, 1, 1} secondMessage := []byte{2} - secondRawTx := generateRawTx(t, encCfg.TxConfig, secondNS, secondMessage, signer, 2, 4, 8) + secondRawTx := generateRawTx(t, encCfg.TxConfig, secondNS, secondMessage, signer, types.AllSquareSizes(len(secondMessage))...) thirdNS := []byte{3, 3, 3, 3, 3, 3, 3, 3} thirdMessage := []byte{1} - thirdRawTx := generateRawTx(t, encCfg.TxConfig, thirdNS, thirdMessage, signer, 2, 8) + invalidSquareSizes := []uint64{2, 8, 16, 32, 64, 128} // missing square size: 4 + thirdRawTx := generateRawTx(t, encCfg.TxConfig, thirdNS, thirdMessage, signer, invalidSquareSizes...) tests := []test{ { @@ -64,17 +67,16 @@ func TestSplitShares(t *testing.T) { data: &core.Data{ Txs: [][]byte{firstRawTx, secondRawTx, thirdRawTx}, }, - expectedTxCount: 3, + expectedTxCount: 2, }, { // calculate the square using the same txs but using a square size - // of 16, this should remove all of the txs as they weren't signed - // over for that square size + // of 16 squareSize: 16, data: &core.Data{ Txs: [][]byte{firstRawTx, secondRawTx, thirdRawTx}, }, - expectedTxCount: 0, + expectedTxCount: 2, }, } diff --git a/pkg/shares/parse_message_shares.go b/pkg/shares/parse_message_shares.go index cfc0ccfa2f..794e495f80 100644 --- a/pkg/shares/parse_message_shares.go +++ b/pkg/shares/parse_message_shares.go @@ -4,9 +4,10 @@ import ( "bytes" "encoding/binary" - "github.com/celestiaorg/celestia-app/pkg/appconsts" "github.com/tendermint/tendermint/pkg/consts" coretypes "github.com/tendermint/tendermint/types" + + "github.com/celestiaorg/celestia-app/pkg/appconsts" ) // parseMsgShares iterates through raw shares and separates the contiguous chunks diff --git a/x/payment/types/errors.go b/x/payment/types/errors.go index 7e167f693e..5cbc13e3cf 100644 --- a/x/payment/types/errors.go +++ b/x/payment/types/errors.go @@ -19,4 +19,5 @@ var ( ErrTxNamespace = sdkerrors.Register(ModuleName, 11119, "cannot use transaction namespace ID") ErrEvidenceNamespace = sdkerrors.Register(ModuleName, 11120, "cannot use evidence namespace ID") ErrNoMessageShareCommitments = sdkerrors.Register(ModuleName, 11121, "no message share commitments") + ErrInvalidShareCommitments = sdkerrors.Register(ModuleName, 11122, "invalid share commitments: all relevant square sizes must be committed to") ) diff --git a/x/payment/types/payfordata_test.go b/x/payment/types/payfordata_test.go index 3d56cff42e..8f4c0dc2a0 100644 --- a/x/payment/types/payfordata_test.go +++ b/x/payment/types/payfordata_test.go @@ -425,10 +425,11 @@ func totalMsgSize(size int) int { } func validWirePayForData(t *testing.T) *MsgWirePayForData { + message := bytes.Repeat([]byte{1}, 2000) msg, err := NewWirePayForData( []byte{1, 2, 3, 4, 5, 6, 7, 8}, - bytes.Repeat([]byte{1}, 2000), - 16, 32, 64, + message, + AllSquareSizes(len(message))..., ) if err != nil { panic(err) diff --git a/x/payment/types/wirepayfordata.go b/x/payment/types/wirepayfordata.go index 5f8443af7f..5c2aeb61d2 100644 --- a/x/payment/types/wirepayfordata.go +++ b/x/payment/types/wirepayfordata.go @@ -102,6 +102,16 @@ func (msg *MsgWirePayForData) ValidateBasic() error { ) } + if err := msg.ValidateMessageShareCommitments(); err != nil { + return err + } + + return nil +} + +// ValidateMessageShareCommitments returns an error if the message share +// commitments are invalid. +func (msg *MsgWirePayForData) ValidateMessageShareCommitments() error { for idx, commit := range msg.MessageShareCommitment { // check that each commit is valid if !powerOf2(commit.K) { @@ -122,9 +132,52 @@ func (msg *MsgWirePayForData) ValidateBasic() error { return ErrNoMessageShareCommitments } + if err := msg.ValidateAllSquareSizesCommitedTo(); err != nil { + return err + } + return nil } +// ValidateAllSquareSizesCommitedTo returns an error if the list of square sizes +// committed to don't match all square sizes expected for this message size. +func (msg *MsgWirePayForData) ValidateAllSquareSizesCommitedTo() error { + allSquareSizes := AllSquareSizes(int(msg.MessageSize)) + committedSquareSizes := msg.committedSquareSizes() + + if len(allSquareSizes) != len(committedSquareSizes) { + return ErrInvalidShareCommitments.Wrapf("length of all square sizes: %v must equal length of committed square sizes: %v", len(allSquareSizes), len(committedSquareSizes)) + } + + if !isEqual(allSquareSizes, committedSquareSizes) { + return ErrInvalidShareCommitments.Wrapf("all square sizes: %v, committed square sizes: %v", allSquareSizes, committedSquareSizes) + } + return nil +} + +// isEqual returns true if the given uint64 slices are equal +func isEqual(a, b []uint64) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true +} + +// commitedSquareSizes returns a list of square sizes that are present in a +// message's share commitment. +func (msg *MsgWirePayForData) committedSquareSizes() []uint64 { + squareSizes := make([]uint64, 0, len(msg.MessageShareCommitment)) + for _, commit := range msg.MessageShareCommitment { + squareSizes = append(squareSizes, commit.K) + } + return squareSizes +} + // 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 diff --git a/x/payment/types/wirepayfordata_test.go b/x/payment/types/wirepayfordata_test.go index 95f07a21f2..2edd557768 100644 --- a/x/payment/types/wirepayfordata_test.go +++ b/x/payment/types/wirepayfordata_test.go @@ -47,7 +47,11 @@ func TestWirePayForData_ValidateBasic(t *testing.T) { // pfd that has a different power of 2 square size badSquareSizeMsg := validWirePayForData(t) - badSquareSizeMsg.MessageShareCommitment[0].K = 4 + badSquareSizeMsg.MessageShareCommitment[0].K = 16 + + // pfd that signs over all squares but the first one + missingCommitmentForOneSquareSize := validWirePayForData(t) + missingCommitmentForOneSquareSize.MessageShareCommitment = missingCommitmentForOneSquareSize.MessageShareCommitment[1:] // pfd that signed over no squares noMessageShareCommitments := validWirePayForData(t) @@ -104,6 +108,11 @@ func TestWirePayForData_ValidateBasic(t *testing.T) { msg: noMessageShareCommitments, wantErr: ErrNoMessageShareCommitments, }, + { + name: "missing commitment for one square size", + msg: missingCommitmentForOneSquareSize, + wantErr: ErrInvalidShareCommitments, + }, } for _, tt := range tests {