From 8e0cf71abce509ceda2786bde6a745e5efddeb2b Mon Sep 17 00:00:00 2001 From: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Date: Mon, 23 May 2022 13:48:10 -0500 Subject: [PATCH] Integration test for max block size (#419) * base for new integration test * fix split shares edge case * add vs code to gitignore * get rid of prepadding messages and use code from core to help generate share commitments * cleanup * remove padding to fix test * nextlowest power of two * moar bugs * update go mod * fix validate basic test for padding * patch test to create exact number of shares correctly * minor cleanup * fix integration test that requires multiple signers * fix cli integration test * format gitignore * only use one delimLen function * use consistent function to calculate msg shares used * formatting * use correct estimation technique for message shares taken * remove commented code * use release version of core * estimate the square size correctly for small square sizes * patch encoding config to register pfd types for lagacy amino * use the short test flag when using the race detector * clean up comment * use test.short * fix remainging denom in test * add comment explaining stuct init --- .github/workflows/test.yml | 2 +- .gitignore | 1 + Makefile | 2 +- app/app.go | 15 + app/encoding/encoding.go | 19 +- app/prepare_proposal.go | 13 +- app/split_shares.go | 37 ++- app/test/block_size_test.go | 290 ++++++++++++++++++ app/test/prepare_proposal_test.go | 71 +++-- app/test/process_proposal_test.go | 2 +- app/test/split_shares_test.go | 12 +- cmd/celestia-appd/root.go | 6 +- contrib/test_cover.sh | 2 +- go.mod | 2 +- go.sum | 4 +- testutil/network/network.go | 26 +- testutil/test_app.go | 4 +- x/payment/client/cli/wirepayfordata.go | 14 +- x/payment/client/testutil/integration_test.go | 83 +++-- x/payment/types/builder.go | 67 +++- x/payment/types/codec.go | 17 +- x/payment/types/payfordata.go | 82 +++-- x/payment/types/payfordata_test.go | 93 +++--- x/payment/types/wirepayfordata.go | 24 +- x/payment/types/wirepayfordata_test.go | 11 - 25 files changed, 641 insertions(+), 258 deletions(-) create mode 100644 app/test/block_size_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d6902b1f81..92e100d33e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -182,7 +182,7 @@ jobs: if: env.GIT_DIFF - name: test & coverage report creation run: | - cat pkgs.txt.part.${{ matrix.part }} | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' > ${{ matrix.part }}-race-output.txt + cat pkgs.txt.part.${{ matrix.part }} | xargs go test -mod=readonly -json -timeout 30m -race -test.short -tags='cgo ledger test_ledger_mock' > ${{ matrix.part }}-race-output.txt if: env.GIT_DIFF - uses: actions/upload-artifact@v3 with: diff --git a/.gitignore b/.gitignore index 47855a536f..d1de802be9 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ secret.yml build coverage.txt tools-stamp +.vscode diff --git a/Makefile b/Makefile index 46c8c8b491..56102d649c 100644 --- a/Makefile +++ b/Makefile @@ -77,7 +77,7 @@ test-unit: @VERSION=$(VERSION) go test -mod=readonly -tags='ledger test_ledger_mock' ./... test-race: - @VERSION=$(VERSION) go test -mod=readonly -race -tags='ledger test_ledger_mock' ./... + @VERSION=$(VERSION) go test -mod=readonly -race -test.short -tags='ledger test_ledger_mock' ./... benchmark: @go test -mod=readonly -bench=. ./... diff --git a/app/app.go b/app/app.go index abc56235bf..209f6c898f 100644 --- a/app/app.go +++ b/app/app.go @@ -145,6 +145,10 @@ var ( qgbmodule.AppModuleBasic{}, ) + // ModuleEncodingRegisters keeps track of all the module methods needed to + // register interfaces and specific type to encoding config + ModuleEncodingRegisters = moduleMapToSlice(ModuleBasics) + // module account permissions maccPerms = map[string][]string{ authtypes.FeeCollectorName: nil, @@ -683,3 +687,14 @@ func (app *App) setTxHandler(txConfig client.TxConfig, indexEventsStr []string) app.SetTxHandler(txHandler) } + +func moduleMapToSlice(m module.BasicManager) []encoding.ModuleRegister { + // TODO: might be able to use some standard generics in go 1.18 + s := make([]encoding.ModuleRegister, len(m)) + i := 0 + for _, v := range m { + s[i] = v + i++ + } + return s +} diff --git a/app/encoding/encoding.go b/app/encoding/encoding.go index 8db940ed45..3ffbcaed50 100644 --- a/app/encoding/encoding.go +++ b/app/encoding/encoding.go @@ -8,7 +8,10 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/tx" ) -type InterfaceRegister func(codectypes.InterfaceRegistry) +type ModuleRegister interface { + RegisterLegacyAminoCodec(*codec.LegacyAmino) + RegisterInterfaces(codectypes.InterfaceRegistry) +} // EncodingConfig specifies the concrete encoding types to use for a given app. // This is provided for compatibility between protobuf and amino implementations. @@ -20,14 +23,22 @@ type EncodingConfig struct { } // MakeEncodingConfig creates an encoding config for the app. -func MakeEncodingConfig(regs ...InterfaceRegister) EncodingConfig { +func MakeEncodingConfig(regs ...ModuleRegister) EncodingConfig { + // create the codec amino := codec.NewLegacyAmino() - std.RegisterLegacyAminoCodec(amino) interfaceRegistry := codectypes.NewInterfaceRegistry() + + // register the standard types from the sdk + std.RegisterLegacyAminoCodec(amino) std.RegisterInterfaces(interfaceRegistry) + + // register specific modules for _, reg := range regs { - reg(interfaceRegistry) + reg.RegisterInterfaces(interfaceRegistry) + reg.RegisterLegacyAminoCodec(amino) } + + // create the final configuration marshaler := codec.NewProtoCodec(interfaceRegistry) txCfg := tx.NewTxConfig(marshaler, tx.DefaultSignModes) diff --git a/app/prepare_proposal.go b/app/prepare_proposal.go index 6fce50793e..f77267ef15 100644 --- a/app/prepare_proposal.go +++ b/app/prepare_proposal.go @@ -49,7 +49,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr func (app *App) estimateSquareSize(data *core.Data) uint64 { txBytes := 0 for _, tx := range data.Txs { - txBytes += len(tx) + delimLen(uint64(len(tx))) + txBytes += len(tx) + types.DelimLen(uint64(len(tx))) } txShareEstimate := txBytes / consts.TxShareSize if txBytes > 0 { @@ -58,7 +58,7 @@ func (app *App) estimateSquareSize(data *core.Data) uint64 { evdBytes := 0 for _, evd := range data.Evidence.Evidence { - evdBytes += evd.Size() + delimLen(uint64(evd.Size())) + evdBytes += evd.Size() + types.DelimLen(uint64(evd.Size())) } evdShareEstimate := evdBytes / consts.TxShareSize if evdBytes > 0 { @@ -68,9 +68,8 @@ func (app *App) estimateSquareSize(data *core.Data) uint64 { msgShareEstimate := estimateMsgShares(app.txConfig, data.Txs) totalShareEstimate := txShareEstimate + evdShareEstimate + msgShareEstimate - - estimatedSize := types.NextPowerOf2(uint64(math.Sqrt(float64(totalShareEstimate)))) - + sr := math.Sqrt(float64(totalShareEstimate)) + estimatedSize := types.NextHighestPowerOf2(uint64(sr)) switch { case estimatedSize > consts.MaxSquareSize: return consts.MaxSquareSize @@ -111,9 +110,7 @@ func estimateMsgShares(txConf client.TxConfig, txs [][]byte) int { continue } - msgShares += (wireMsg.MessageSize / consts.MsgShareSize) + 1 // plus one to round up - + msgShares += uint64(MsgSharesUsed(int(wireMsg.MessageSize))) } - return int(msgShares) } diff --git a/app/split_shares.go b/app/split_shares.go index ce0eb58287..263b147bdf 100644 --- a/app/split_shares.go +++ b/app/split_shares.go @@ -3,7 +3,6 @@ package app import ( "bytes" "crypto/sha256" - "math/bits" "sort" "github.com/celestiaorg/celestia-app/x/payment/types" @@ -23,10 +22,11 @@ import ( // block data are only used to avoid dereferening, not because we need the block // data to be mutable. func SplitShares(txConf client.TxConfig, squareSize uint64, data *core.Data) ([][]byte, *core.Data) { - var ( - processedTxs [][]byte - messages core.Messages - ) + processedTxs := make([][]byte, 0) + // we initiate this struct here so that the empty output is identiacal in + // tests + messages := core.Messages{} + sqwr := newShareSplitter(txConf, squareSize, data) for _, rawTx := range data.Txs { @@ -209,11 +209,11 @@ func (sqwr *shareSplitter) writeMalleatedTx( func (sqwr *shareSplitter) hasRoomForBoth(tx, msg []byte) bool { currentShareCount, availableBytes := sqwr.shareCount() - txBytesTaken := delimLen(uint64(len(tx))) + len(tx) + txBytesTaken := types.DelimLen(uint64(len(tx))) + len(tx) maxTxSharesTaken := ((txBytesTaken - availableBytes) / consts.TxShareSize) + 1 // plus one becuase we have to add at least one share - maxMsgSharesTaken := len(msg) / consts.MsgShareSize + maxMsgSharesTaken := MsgSharesUsed(len(msg)) return currentShareCount+maxTxSharesTaken+maxMsgSharesTaken <= sqwr.maxShareCount } @@ -221,7 +221,7 @@ func (sqwr *shareSplitter) hasRoomForBoth(tx, msg []byte) bool { func (sqwr *shareSplitter) hasRoomForTx(tx []byte) bool { currentShareCount, availableBytes := sqwr.shareCount() - bytesTaken := delimLen(uint64(len(tx))) + len(tx) + bytesTaken := types.DelimLen(uint64(len(tx))) + len(tx) if bytesTaken <= availableBytes { return true } @@ -238,9 +238,9 @@ func (sqwr *shareSplitter) shareCount() (count, availableTxBytes int) { } func (sqwr *shareSplitter) export() [][]byte { - count, pendingTxBytes := sqwr.shareCount() + count, availableBytes := sqwr.shareCount() // increment the count if there are any pending tx bytes - if pendingTxBytes > 0 { + if availableBytes < consts.TxShareSize { count++ } shares := make([][]byte, sqwr.maxShareCount) @@ -274,6 +274,19 @@ func (sqwr *shareSplitter) export() [][]byte { return shares } +// MsgSharesUsed calculates the minimum number of shares a message will take up. +// It accounts for the necessary delimiter and potential padding. +func MsgSharesUsed(msgSize int) int { + // add the delimiter to the message size + msgSize = types.DelimLen(uint64(msgSize)) + msgSize + shareCount := msgSize / consts.MsgShareSize + // increment the share count if the message overflows the last counted share + if msgSize%consts.MsgShareSize != 0 { + shareCount++ + } + return shareCount +} + func hasWirePayForData(tx sdk.Tx) bool { for _, msg := range tx.GetMsgs() { msgName := sdk.MsgTypeURL(msg) @@ -283,7 +296,3 @@ func hasWirePayForData(tx sdk.Tx) bool { } return false } - -func delimLen(x uint64) int { - return 8 - bits.LeadingZeros64(x)%8 -} diff --git a/app/test/block_size_test.go b/app/test/block_size_test.go new file mode 100644 index 0000000000..b61056c7f6 --- /dev/null +++ b/app/test/block_size_test.go @@ -0,0 +1,290 @@ +package app_test + +import ( + "bytes" + "context" + "encoding/hex" + "testing" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + + cosmosnet "github.com/cosmos/cosmos-sdk/testutil/network" + + "github.com/celestiaorg/celestia-app/app" + "github.com/celestiaorg/celestia-app/app/encoding" + "github.com/celestiaorg/celestia-app/testutil/network" + "github.com/celestiaorg/celestia-app/x/payment/types" + "github.com/celestiaorg/nmt/namespace" + abci "github.com/tendermint/tendermint/abci/types" + tmrand "github.com/tendermint/tendermint/libs/rand" + "github.com/tendermint/tendermint/pkg/consts" + rpctypes "github.com/tendermint/tendermint/rpc/coretypes" + coretypes "github.com/tendermint/tendermint/types" +) + +type IntegrationTestSuite struct { + suite.Suite + + cfg cosmosnet.Config + encCfg encoding.EncodingConfig + network *cosmosnet.Network + kr keyring.Keyring + accounts []string +} + +func NewIntegrationTestSuite(cfg cosmosnet.Config) *IntegrationTestSuite { + return &IntegrationTestSuite{cfg: cfg} +} + +func (s *IntegrationTestSuite) SetupSuite() { + s.T().Log("setting up integration test suite") + + if testing.Short() { + s.T().Skip("skipping test in unit-tests mode.") + } + + numAccounts := 100 + s.accounts = make([]string, numAccounts) + for i := 0; i < numAccounts; i++ { + s.accounts[i] = tmrand.Str(20) + } + + net := network.New(s.T(), s.cfg, s.accounts...) + + s.network = net + s.kr = net.Validators[0].ClientCtx.Keyring + s.encCfg = encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) + + _, err := s.network.WaitForHeight(1) + s.Require().NoError(err) +} + +func (s *IntegrationTestSuite) TearDownSuite() { + s.T().Log("tearing down integration test suite") + s.network.Cleanup() +} + +func (s *IntegrationTestSuite) TestSubmitWirePayForData() { + require := s.Require() + assert := s.Assert() + val := s.network.Validators[0] + + // tendermint's default tx size limit is 1Mb, so we get close to that + equallySized1MbTxGen := func(c client.Context) []coretypes.Tx { + equallySized1MbTxs, err := generateSignedWirePayForDataTxs(c, s.cfg.TxConfig, s.kr, 970000, s.accounts[:20]...) + require.NoError(err) + return equallySized1MbTxs + } + + // generate 100 randomly sized txs (max size == 100kb) by generating these + // transaction using some of the same accounts as the previous genertor, we + // are also testing to ensure that the sequence number is being utilized + // corrected in malleated txs + randoTxGen := func(c client.Context) []coretypes.Tx { + randomTxs, err := generateSignedWirePayForDataTxs(c, s.cfg.TxConfig, s.kr, -1, s.accounts...) + require.NoError(err) + return randomTxs + } + + type test struct { + name string + txGenerator func(clientCtx client.Context) []coretypes.Tx + } + + tests := []test{ + { + "20 ~1Mb txs", + equallySized1MbTxGen, + }, + { + "100 random txs", + randoTxGen, + }, + } + for _, tc := range tests { + s.Run(tc.name, func() { + txs := tc.txGenerator(val.ClientCtx) + hashes := make([]string, len(txs)) + + for i, tx := range txs { + res, err := val.ClientCtx.BroadcastTxSync(tx) + require.NoError(err) + require.Equal(abci.CodeTypeOK, res.Code) + hashes[i] = res.TxHash + } + + // wait a few blocks to clear the txs + for i := 0; i < 10; i++ { + require.NoError(s.network.WaitForNextBlock()) + } + + heights := make(map[int64]int) + for _, hash := range hashes { + // TODO: once we are able to query txs that span more than two + // shares, we should switch to proving txs existence in the block + resp, err := queryWithOutProof(val.ClientCtx, hash) + assert.NoError(err) + assert.Equal(uint32(0), abci.CodeTypeOK) + if resp.TxResult.Code == abci.CodeTypeOK { + heights[resp.Height]++ + } + } + + require.Greater(len(heights), 0) + + sizes := []uint64{} + // check the square size + for height := range heights { + node, err := val.ClientCtx.GetNode() + require.NoError(err) + blockRes, err := node.Block(context.Background(), &height) + require.NoError(err) + size := blockRes.Block.Data.OriginalSquareSize + + // perform basic checks on the size of the square + assert.LessOrEqual(size, uint64(consts.MaxSquareSize)) + assert.GreaterOrEqual(size, uint64(consts.MinSquareSize)) + sizes = append(sizes, size) + } + + // ensure that at least one of the blocks used the max square size + assert.Contains(sizes, uint64(consts.MaxSquareSize)) + + }) + require.NoError(s.network.WaitForNextBlock()) + } + +} + +func TestIntegrationTestSuite(t *testing.T) { + cfg := network.DefaultConfig() + cfg.EnableTMLogging = false + cfg.MinGasPrices = "0utia" + cfg.NumValidators = 2 + suite.Run(t, NewIntegrationTestSuite(cfg)) +} + +func generateSignedWirePayForDataTxs(clientCtx client.Context, txConfig client.TxConfig, kr keyring.Keyring, msgSize int, accounts ...string) ([]coretypes.Tx, error) { + txs := make([]coretypes.Tx, len(accounts)) + for i, account := range accounts { + signer := types.NewKeyringSigner(kr, account, clientCtx.ChainID) + + err := signer.UpdateAccountFromClient(clientCtx) + if err != nil { + return nil, err + } + + coin := sdk.Coin{ + Denom: app.BondDenom, + Amount: sdk.NewInt(1000000), + } + + opts := []types.TxBuilderOption{ + types.SetFeeAmount(sdk.NewCoins(coin)), + types.SetGasLimit(1000000000), + } + + thisMessageSize := msgSize + if msgSize < 1 { + for { + thisMessageSize = tmrand.NewRand().Intn(100000) + if thisMessageSize != 0 { + break + } + } + } + + // create a msg + msg, err := types.NewWirePayForData( + randomValidNamespace(), + tmrand.Bytes(thisMessageSize), + AllSquareSizes(thisMessageSize)..., + ) + if err != nil { + return nil, err + } + + err = msg.SignShareCommitments(signer, opts...) + if err != nil { + return nil, err + } + + builder := signer.NewTxBuilder(opts...) + + tx, err := signer.BuildSignedTx(builder, msg) + if err != nil { + return nil, err + } + + rawTx, err := txConfig.TxEncoder()(tx) + if err != nil { + return nil, err + } + + txs[i] = coretypes.Tx(rawTx) + } + + return txs, nil +} + +func randomValidNamespace() namespace.ID { + for { + s := tmrand.Bytes(8) + if bytes.Compare(s, consts.MaxReservedNamespace) > 0 { + return s + } + } +} + +func queryWithOutProof(clientCtx client.Context, hashHexStr string) (*rpctypes.ResultTx, error) { + hash, err := hex.DecodeString(hashHexStr) + if err != nil { + return nil, err + } + + node, err := clientCtx.GetNode() + if err != nil { + return nil, err + } + + return node.Tx(context.Background(), hash, false) +} + +// TODO: refactor these into a different package +// they will be useful for +// https://github.com/celestiaorg/celestia-app/issues/236 +// https://github.com/celestiaorg/celestia-app/issues/239 + +// generateAllSquareSizes generates and returns all of the possible square sizes +// using the maximum and minimum square sizes +func generateAllSquareSizes() []int { + sizes := []int{} + cursor := int(consts.MinSquareSize) + for cursor <= consts.MaxSquareSize { + sizes = append(sizes, cursor) + cursor *= 2 + } + return sizes +} + +// AllSquareSizes calculates all of the square sizes that message could possibly +// fit in +func AllSquareSizes(msgSize int) []uint64 { + allSizes := generateAllSquareSizes() + fitSizes := []uint64{} + shareCount := app.MsgSharesUsed(msgSize) + for _, size := range allSizes { + // if the number of shares is larger than that in the square, throw an error + // note, we use k*k-1 here because at least a single share will be reserved + // for the transaction paying for the message, therefore the max number of + // shares a message can be is number of shares in square -1. + if shareCount > (size*size)-1 { + continue + } + fitSizes = append(fitSizes, uint64(size)) + } + return fitSizes +} diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index eeea489432..a25ef1e248 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -10,6 +10,7 @@ import ( "github.com/celestiaorg/celestia-app/x/payment/types" "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" @@ -19,7 +20,7 @@ import ( func TestPrepareProposal(t *testing.T) { signer := testutil.GenerateKeyringSigner(t, testAccName) - encCfg := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encCfg := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) testApp := testutil.SetupTestAppWithGenesisValSet(t) @@ -31,28 +32,27 @@ 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) + firstRawTx := generateRawTx(t, encCfg.TxConfig, firstNS, firstMessage, signer, 2, 4, 8, 16) 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, 2, 4, 8, 16) thirdNS := []byte{3, 3, 3, 3, 3, 3, 3, 3} thirdMessage := []byte{1} - thirdRawTx := generateRawTx(t, encCfg.TxConfig, thirdNS, thirdMessage, signer, 2, 4, 8) + thirdRawTx := generateRawTx(t, encCfg.TxConfig, thirdNS, thirdMessage, signer, 2, 4, 8, 16) tests := []test{ { input: abci.RequestPrepareProposal{ BlockData: &core.Data{ - Txs: [][]byte{firstRawTx, secondRawTx, thirdRawTx}, - OriginalSquareSize: 4, + Txs: [][]byte{firstRawTx, secondRawTx, thirdRawTx}, }, }, expectedMessages: []*core.Message{ { - NamespaceId: secondNS, // the second message should be first - Data: append([]byte{2}, bytes.Repeat([]byte{0}, 255)...), // check that the message is padded + NamespaceId: secondNS, // the second message should be first + Data: []byte{2}, }, { NamespaceId: firstNS, @@ -60,7 +60,7 @@ func TestPrepareProposal(t *testing.T) { }, { NamespaceId: thirdNS, - Data: append([]byte{1}, bytes.Repeat([]byte{0}, 255)...), + Data: []byte{1}, }, }, expectedTxs: 3, @@ -71,23 +71,52 @@ func TestPrepareProposal(t *testing.T) { res := testApp.PrepareProposal(tt.input) assert.Equal(t, tt.expectedMessages, res.BlockData.Messages.MessagesList) assert.Equal(t, tt.expectedTxs, len(res.BlockData.Txs)) + + // verify the signatures of the prepared txs + sdata, err := signer.GetSignerData() + if err != nil { + require.NoError(t, err) + } + dec := app.MalleatedTxDecoder(encCfg.TxConfig.TxDecoder()) + for _, tx := range res.BlockData.Txs { + sTx, err := dec(tx) + require.NoError(t, err) + + sigTx, ok := sTx.(authsigning.SigVerifiableTx) + require.True(t, ok) + + sigs, err := sigTx.GetSignaturesV2() + require.NoError(t, err) + require.Equal(t, 1, len(sigs)) + sig := sigs[0] + + err = authsigning.VerifySignature( + sdata.PubKey, + sdata, + sig.Data, + encCfg.TxConfig.SignModeHandler(), + sTx, + ) + assert.NoError(t, err) + } } } func generateRawTx(t *testing.T, txConfig client.TxConfig, ns, message []byte, signer *types.KeyringSigner, ks ...uint64) (rawTx []byte) { - // create a msg - msg := generateSignedWirePayForData(t, ns, message, signer, ks...) - - builder := signer.NewTxBuilder() - coin := sdk.Coin{ - Denom: "token", - Amount: sdk.NewInt(1000), + Denom: app.BondDenom, + Amount: sdk.NewInt(10), } - builder.SetFeeAmount(sdk.NewCoins(coin)) - builder.SetGasLimit(10000) - builder.SetTimeoutHeight(99) + opts := []types.TxBuilderOption{ + types.SetFeeAmount(sdk.NewCoins(coin)), + types.SetGasLimit(10000000), + } + + // create a msg + msg := generateSignedWirePayForData(t, ns, message, signer, opts, ks...) + + builder := signer.NewTxBuilder(opts...) tx, err := signer.BuildSignedTx(builder, msg) require.NoError(t, err) @@ -99,13 +128,13 @@ func generateRawTx(t *testing.T, txConfig client.TxConfig, ns, message []byte, s return rawTx } -func generateSignedWirePayForData(t *testing.T, ns, message []byte, signer *types.KeyringSigner, ks ...uint64) *types.MsgWirePayForData { +func generateSignedWirePayForData(t *testing.T, ns, message []byte, signer *types.KeyringSigner, options []types.TxBuilderOption, ks ...uint64) *types.MsgWirePayForData { msg, err := types.NewWirePayForData(ns, message, ks...) if err != nil { t.Error(err) } - err = msg.SignShareCommitments(signer) + err = msg.SignShareCommitments(signer, options...) if err != nil { t.Error(err) } diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index ec1b505326..d2f6ac755c 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -26,7 +26,7 @@ func TestMessageInclusionCheck(t *testing.T) { testApp := testutil.SetupTestAppWithGenesisValSet(t) - encConf := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encConf := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) firstValidPFD, msg1 := genRandMsgPayForData(t, signer, 8) secondValidPFD, msg2 := genRandMsgPayForData(t, signer, 8) diff --git a/app/test/split_shares_test.go b/app/test/split_shares_test.go index 7a3fce7fd0..27d6fd1a9a 100644 --- a/app/test/split_shares_test.go +++ b/app/test/split_shares_test.go @@ -16,7 +16,7 @@ import ( ) func TestSplitShares(t *testing.T) { - encCfg := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encCfg := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) type test struct { squareSize uint64 @@ -66,6 +66,16 @@ func TestSplitShares(t *testing.T) { }, expectedTxCount: 3, }, + { + // 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 + squareSize: 16, + data: &core.Data{ + Txs: [][]byte{firstRawTx, secondRawTx, thirdRawTx}, + }, + expectedTxCount: 0, + }, } for _, tt := range tests { diff --git a/cmd/celestia-appd/root.go b/cmd/celestia-appd/root.go index 59bcdea09a..a91d994b45 100644 --- a/cmd/celestia-appd/root.go +++ b/cmd/celestia-appd/root.go @@ -39,7 +39,7 @@ import ( // NewRootCmd creates a new root command for celestia-appd. It is called once in the // main function. func NewRootCmd() *cobra.Command { - encodingConfig := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encodingConfig := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) cfg := sdk.GetConfig() cfg.SetBech32PrefixForAccount(app.Bech32PrefixAccAddr, app.Bech32PrefixAccPub) @@ -220,7 +220,7 @@ func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, appOpts serverty logger, db, traceStore, true, skipUpgradeHeights, cast.ToString(appOpts.Get(flags.FlagHome)), cast.ToUint(appOpts.Get(server.FlagInvCheckPeriod)), - encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces), // Ideally, we would reuse the one created by NewRootCmd. + encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...), // Ideally, we would reuse the one created by NewRootCmd. appOpts, baseapp.SetPruning(pruningOpts), baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))), @@ -238,7 +238,7 @@ func createAppAndExport( logger log.Logger, db dbm.DB, traceStore io.Writer, height int64, forZeroHeight bool, jailWhiteList []string, appOpts servertypes.AppOptions, ) (servertypes.ExportedApp, error) { - encCfg := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) // Ideally, we would reuse the one created by NewRootCmd. + encCfg := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) // Ideally, we would reuse the one created by NewRootCmd. encCfg.Codec = codec.NewProtoCodec(encCfg.InterfaceRegistry) var capp *app.App if height != -1 { diff --git a/contrib/test_cover.sh b/contrib/test_cover.sh index 24f7804b51..bfdf4f89ed 100644 --- a/contrib/test_cover.sh +++ b/contrib/test_cover.sh @@ -6,7 +6,7 @@ PKGS=$(go list ./... | grep -v '/simapp') set -e echo "mode: atomic" > coverage.txt for pkg in ${PKGS[@]}; do - go test -v -timeout 30m -race -coverprofile=profile.out -covermode=atomic -tags='ledger test_ledger_mock' "$pkg" + go test -v -timeout 30m -race -test.short -coverprofile=profile.out -covermode=atomic -tags='ledger test_ledger_mock' "$pkg" if [ -f profile.out ]; then tail -n +2 profile.out >> coverage.txt; rm profile.out diff --git a/go.mod b/go.mod index d92cb09df4..97bc303b68 100644 --- a/go.mod +++ b/go.mod @@ -158,5 +158,5 @@ replace ( github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76 github.com/cosmos/cosmos-sdk => github.com/celestiaorg/cosmos-sdk v1.1.0-sdk-v0.46.0 github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 - github.com/tendermint/tendermint v0.35.4 => github.com/celestiaorg/celestia-core v1.2.1-tm-v0.35.4 + github.com/tendermint/tendermint v0.35.4 => github.com/celestiaorg/celestia-core v1.2.2-tm-v0.35.4 ) diff --git a/go.sum b/go.sum index 82d736f724..80e65efeb3 100644 --- a/go.sum +++ b/go.sum @@ -122,8 +122,8 @@ github.com/btcsuite/snappy-go v1.0.0/go.mod h1:8woku9dyThutzjeg+3xrA5iCpBRH8XEEg github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY= github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs= github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ= -github.com/celestiaorg/celestia-core v1.2.1-tm-v0.35.4 h1:FB3yQIuUmYxYlgGu+s4zTOggIjxU39g/1BGyOKT+vz4= -github.com/celestiaorg/celestia-core v1.2.1-tm-v0.35.4/go.mod h1:qfSLGgzbnzEVXNMv80VxzXH3yAJHT/u6IlpNHJk0/aQ= +github.com/celestiaorg/celestia-core v1.2.2-tm-v0.35.4 h1:b+S47xLUVfmRAr+Wi6MH0b5HX6AqZUHxQRVioG851ys= +github.com/celestiaorg/celestia-core v1.2.2-tm-v0.35.4/go.mod h1:qfSLGgzbnzEVXNMv80VxzXH3yAJHT/u6IlpNHJk0/aQ= github.com/celestiaorg/cosmos-sdk v1.1.0-sdk-v0.46.0 h1:Avs0lxSYabPTGwCfS7nV0yCF1QKO0O1mOEE1L6C8tGo= github.com/celestiaorg/cosmos-sdk v1.1.0-sdk-v0.46.0/go.mod h1:SqOn+Sol7ydGocB4Qqo24chrwG6YW4OhFh6NSpsFYbk= github.com/celestiaorg/go-leopard v0.1.0 h1:28z2EkvKJIez5J9CEaiiUEC+OxalRLtTGJJ1oScfE1g= diff --git a/testutil/network/network.go b/testutil/network/network.go index 67571e660c..b6ed725bc4 100644 --- a/testutil/network/network.go +++ b/testutil/network/network.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/celestiaorg/celestia-app/app" + "github.com/celestiaorg/celestia-app/app/encoding" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" @@ -15,12 +17,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - "github.com/stretchr/testify/require" tmrand "github.com/tendermint/tendermint/libs/rand" tmdb "github.com/tendermint/tm-db" - - "github.com/celestiaorg/celestia-app/app" - "github.com/celestiaorg/celestia-app/app/encoding" ) func New(t *testing.T, config network.Config, genAccNames ...string) *network.Network { @@ -29,12 +27,10 @@ func New(t *testing.T, config network.Config, genAccNames ...string) *network.Ne // add genesis accounts genAuthAccs := make([]authtypes.GenesisAccount, len(genAccNames)) genBalances := make([]banktypes.Balance, len(genAccNames)) - mnemonics := make([]string, len(genAccNames)) for i, name := range genAccNames { - a, b, mnm := newGenAccout(kr, name, 1000000000000) + a, b := newGenAccout(kr, name, 1000000000000) genAuthAccs[i] = a genBalances[i] = b - mnemonics[i] = mnm } config, err := addGenAccounts(config, genAuthAccs, genBalances) @@ -49,11 +45,7 @@ func New(t *testing.T, config network.Config, genAccNames ...string) *network.Ne panic(err) } - // add the keys to the keyring that is used by the integration test - for i, name := range genAccNames { - _, err := net.Validators[0].ClientCtx.Keyring.NewAccount(name, mnemonics[i], "", "", hd.Secp256k1) - require.NoError(t, err) - } + net.Validators[0].ClientCtx.Keyring = kr return net } @@ -62,7 +54,7 @@ func New(t *testing.T, config network.Config, genAccNames ...string) *network.Ne // genesis and single validator. All other parameters are inherited from // cosmos-sdk/testutil/network.DefaultConfig func DefaultConfig() network.Config { - encCfg := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encCfg := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) return network.Config{ Codec: encCfg.Codec, @@ -119,15 +111,15 @@ func addGenAccounts(cfg network.Config, genAccounts []authtypes.GenesisAccount, return cfg, nil } -func newGenAccout(kr keyring.Keyring, name string, amount int64) (authtypes.GenesisAccount, banktypes.Balance, string) { - info, mnm, err := kr.NewMnemonic(name, keyring.English, "", "", hd.Secp256k1) +func newGenAccout(kr keyring.Keyring, name string, amount int64) (authtypes.GenesisAccount, banktypes.Balance) { + info, _, err := kr.NewMnemonic(name, keyring.English, "", "", hd.Secp256k1) if err != nil { panic(err) } // create coin balances := sdk.NewCoins( - sdk.NewCoin(fmt.Sprintf("%stoken", name), sdk.NewInt(amount)), + sdk.NewCoin(fmt.Sprintf("%stoken", app.BondDenom), sdk.NewInt(amount)), sdk.NewCoin(app.BondDenom, sdk.NewInt(amount)), ) @@ -146,5 +138,5 @@ func newGenAccout(kr keyring.Keyring, name string, amount int64) (authtypes.Gene panic(err) } - return authtypes.NewBaseAccount(addr, pub, 0, 0), bal, mnm + return authtypes.NewBaseAccount(addr, pub, 0, 0), bal } diff --git a/testutil/test_app.go b/testutil/test_app.go index 9b741e5f27..1856eac975 100644 --- a/testutil/test_app.go +++ b/testutil/test_app.go @@ -79,7 +79,7 @@ func SetupTestAppWithGenesisValSet(t *testing.T) *app.App { db := dbm.NewMemDB() skipUpgradeHeights := make(map[int64]bool) - encCfg := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encCfg := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) testApp := app.New( log.NewNopLogger(), db, nil, true, skipUpgradeHeights, @@ -292,7 +292,7 @@ func genesisStateWithValSet(t *testing.T, // GenerateKeyringSigner creates a types.KeyringSigner with keys generated for // the provided accounts func GenerateKeyringSigner(t *testing.T, acct string) *types.KeyringSigner { - encCfg := encoding.MakeEncodingConfig(app.ModuleBasics.RegisterInterfaces) + encCfg := encoding.MakeEncodingConfig(app.ModuleEncodingRegisters...) kr := generateKeyring(t, encCfg.Codec) return types.NewKeyringSigner(kr, acct, testChainID) } diff --git a/x/payment/client/cli/wirepayfordata.go b/x/payment/client/cli/wirepayfordata.go index f678017324..d57f579ee6 100644 --- a/x/payment/client/cli/wirepayfordata.go +++ b/x/payment/client/cli/wirepayfordata.go @@ -28,13 +28,6 @@ func CmdWirePayForData() *cobra.Command { return err } - // query for account number - fromAddress := clientCtx.GetFromAddress() - account, err := clientCtx.AccountRetriever.GetAccount(clientCtx, fromAddress) - if err != nil { - return err - } - // get the account name accName := clientCtx.GetFromName() if accName == "" { @@ -67,8 +60,10 @@ func CmdWirePayForData() *cobra.Command { // use the keyring to programmatically sign multiple PayForData txs signer := types.NewKeyringSigner(clientCtx.Keyring, accName, clientCtx.ChainID) - signer.SetAccountNumber(account.GetAccountNumber()) - signer.SetSequence(account.GetSequence()) + err = signer.UpdateAccountFromClient(clientCtx) + if err != nil { + return err + } // get and parse the gas limit for this tx rawGasLimit, err := cmd.Flags().GetString(flags.FlagGas) @@ -104,6 +99,7 @@ func CmdWirePayForData() *cobra.Command { if err = pfdMsg.ValidateBasic(); err != nil { return err } + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), pfdMsg) }, } diff --git a/x/payment/client/testutil/integration_test.go b/x/payment/client/testutil/integration_test.go index 7afa9759c8..8e6f5aa9e7 100644 --- a/x/payment/client/testutil/integration_test.go +++ b/x/payment/client/testutil/integration_test.go @@ -1,6 +1,7 @@ package testutil import ( + "encoding/hex" "fmt" "strconv" "testing" @@ -9,7 +10,6 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/suite" - "github.com/tendermint/tendermint/pkg/consts" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" cosmosnet "github.com/cosmos/cosmos-sdk/testutil/network" @@ -40,10 +40,6 @@ func NewIntegrationTestSuite(cfg cosmosnet.Config) *IntegrationTestSuite { func (s *IntegrationTestSuite) SetupSuite() { s.T().Log("setting up integration test suite") - if testing.Short() { - s.T().Skip("skipping test in unit-tests mode.") - } - net := network.New(s.T(), s.cfg, username) s.network = net @@ -82,20 +78,7 @@ func (s *IntegrationTestSuite) TestSubmitWirePayForData() { fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(2))).String()), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", paycli.FlagSquareSizes, "2"), - }, - false, 0, &sdk.TxResponse{}, - }, - { - "valid transaction list of square sizes", - []string{ - hexNS, - hexMsg, - fmt.Sprintf("--from=%s", username), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(2))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", paycli.FlagSquareSizes, "2,4,8,16,32"), + fmt.Sprintf("--%s=%s", paycli.FlagSquareSizes, "2,4,8,16"), }, false, 0, &sdk.TxResponse{}, }, @@ -124,39 +107,43 @@ func (s *IntegrationTestSuite) TestSubmitWirePayForData() { out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args) if tc.expectErr { require.Error(err) - } else { - require.NoError(err, "test: %s\noutput: %s", tc.name, out.String()) - err = clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType) - require.NoError(err, out.String(), "test: %s, output\n:", tc.name, out.String()) - - txResp := tc.respType.(*sdk.TxResponse) - require.Equal(tc.expectedCode, txResp.Code, - "test: %s, output\n:", tc.name, out.String()) - - events := txResp.Logs[0].GetEvents() - for _, e := range events { - switch e.Type { - case types.EventTypePayForData: - signer := e.GetAttributes()[0].GetValue() - _, err = sdk.AccAddressFromBech32(signer) - require.NoError(err) - msgSize, err := strconv.ParseUint(e.GetAttributes()[1].GetValue(), 10, 64) - require.NoError(err) - s.Equal(uint64(0), msgSize%consts.ShareSize, "Message length should be multiples of const.ShareSize=%v", consts.ShareSize) - } + return + } + require.NoError(err, "test: %s\noutput: %s", tc.name, out.String()) + + err = clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType) + require.NoError(err, out.String(), "test: %s, output\n:", tc.name, out.String()) + + txResp := tc.respType.(*sdk.TxResponse) + require.Equal(tc.expectedCode, txResp.Code, + "test: %s, output\n:", tc.name, out.String()) + + events := txResp.Logs[0].GetEvents() + for _, e := range events { + switch e.Type { + case types.EventTypePayForData: + signer := e.GetAttributes()[0].GetValue() + _, err = sdk.AccAddressFromBech32(signer) + require.NoError(err) + msg, err := hex.DecodeString(tc.args[1]) + require.NoError(err) + msgSize, err := strconv.ParseInt(e.GetAttributes()[1].GetValue(), 10, 64) + require.NoError(err) + require.Equal(len(msg), int(msgSize)) } + } - // wait for the tx to be indexed - s.Require().NoError(s.network.WaitForNextBlock()) + // wait for the tx to be indexed + s.Require().NoError(s.network.WaitForNextBlock()) - // attempt to query for the malleated transaction using the original tx's hash - qTxCmd := authcmd.QueryTxCmd() - out, err := clitestutil.ExecTestCLICmd(clientCtx, qTxCmd, []string{txResp.TxHash, "--output=json"}) - require.NoError(err) + // attempt to query for the malleated transaction using the original tx's hash + qTxCmd := authcmd.QueryTxCmd() + out, err = clitestutil.ExecTestCLICmd(clientCtx, qTxCmd, []string{txResp.TxHash, "--output=json"}) + require.NoError(err) + + var result sdk.TxResponse + s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &result)) - var result sdk.TxResponse - s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &result)) - } }) } } diff --git a/x/payment/types/builder.go b/x/payment/types/builder.go index 9f4954bcd4..8a2818d47c 100644 --- a/x/payment/types/builder.go +++ b/x/payment/types/builder.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/celestiaorg/celestia-app/app/encoding" + "github.com/cosmos/cosmos-sdk/client" sdkclient "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/crypto/keyring" sdktypes "github.com/cosmos/cosmos-sdk/types" @@ -64,9 +65,32 @@ func (k *KeyringSigner) QueryAccountNumber(ctx context.Context, conn *grpc.Clien return nil } +func (k *KeyringSigner) UpdateAccountFromClient(clientCtx client.Context) error { + rec := k.GetSignerInfo() + + addr, err := rec.GetAddress() + if err != nil { + return err + } + + accNum, seq, err := clientCtx.AccountRetriever.GetAccountNumberSequence(clientCtx, addr) + if err != nil { + return err + } + + k.SetAccountNumber(accNum) + k.SetSequence(seq) + + return nil +} + // NewTxBuilder returns the default sdk Tx builder using the celestia-app encoding config -func (k *KeyringSigner) NewTxBuilder() sdkclient.TxBuilder { - return k.encCfg.TxConfig.NewTxBuilder() +func (k *KeyringSigner) NewTxBuilder(opts ...TxBuilderOption) sdkclient.TxBuilder { + builder := k.encCfg.TxConfig.NewTxBuilder() + for _, opt := range opts { + builder = opt(builder) + } + return builder } // BuildSignedTx creates and signs a sdk.Tx that contains the provided message. The interal @@ -74,7 +98,6 @@ func (k *KeyringSigner) NewTxBuilder() sdkclient.TxBuilder { // k.SetAccountNumber for the built transactions to be valid. func (k *KeyringSigner) BuildSignedTx(builder sdkclient.TxBuilder, msg sdktypes.Msg) (authsigning.Tx, error) { k.RLock() - accountNumber := k.accountNumber sequence := k.sequence k.RUnlock() @@ -112,14 +135,15 @@ func (k *KeyringSigner) BuildSignedTx(builder sdkclient.TxBuilder, msg sdktypes. return nil, err } + signerData, err := k.GetSignerData() + if err != nil { + return nil, err + } + // Generate the bytes to be signed. bytesToSign, err := k.encCfg.TxConfig.SignModeHandler().GetSignBytes( signing.SignMode_SIGN_MODE_DIRECT, - authsigning.SignerData{ - ChainID: k.chainID, - AccountNumber: accountNumber, - Sequence: sequence, - }, + signerData, builder.GetTx(), ) if err != nil { @@ -188,6 +212,33 @@ func (k *KeyringSigner) GetSignerInfo() *keyring.Record { return info } +func (k *KeyringSigner) GetSignerData() (authsigning.SignerData, error) { + k.RLock() + accountNumber := k.accountNumber + sequence := k.sequence + k.RUnlock() + + record, err := k.Key(k.keyringAccName) + if err != nil { + return authsigning.SignerData{}, err + } + + pubKey, err := record.GetPubKey() + if err != nil { + return authsigning.SignerData{}, err + } + + address := pubKey.Address() + + return authsigning.SignerData{ + Address: address.String(), + ChainID: k.chainID, + AccountNumber: accountNumber, + Sequence: sequence, + PubKey: pubKey, + }, nil +} + // EncodeTx uses the keyring signer's encoding config to encode the provided sdk transaction func (k *KeyringSigner) EncodeTx(tx sdktypes.Tx) ([]byte, error) { return k.encCfg.TxConfig.TxEncoder()(tx) diff --git a/x/payment/types/codec.go b/x/payment/types/codec.go index 3034004ecc..b31a57c0b3 100644 --- a/x/payment/types/codec.go +++ b/x/payment/types/codec.go @@ -14,7 +14,8 @@ var ( ) func RegisterCodec(cdc *codec.LegacyAmino) { - cdc.RegisterConcrete(&MsgWirePayForData{}, "payment/WirePayForData", nil) + cdc.RegisterConcrete(&MsgWirePayForData{}, URLMsgWirePayForData, nil) + cdc.RegisterConcrete(&MsgPayForData{}, URLMsgPayForData, nil) } func RegisterInterfaces(registry codectypes.InterfaceRegistry) { @@ -39,9 +40,21 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc) } +type localEncoder struct { +} + +func (localEncoder) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { + RegisterCodec(cdc) +} + +func (localEncoder) RegisterInterfaces(r codectypes.InterfaceRegistry) { + RegisterInterfaces(r) +} + // makePaymentEncodingConfig uses the payment modules RegisterInterfaces // function to create an encoding config for the payment module. This is useful // so that we don't have to import the app package. func makePaymentEncodingConfig() encoding.EncodingConfig { - return encoding.MakeEncodingConfig(RegisterInterfaces) + e := localEncoder{} + return encoding.MakeEncodingConfig(e) } diff --git a/x/payment/types/payfordata.go b/x/payment/types/payfordata.go index 04273519fc..cfbc77661c 100644 --- a/x/payment/types/payfordata.go +++ b/x/payment/types/payfordata.go @@ -3,6 +3,7 @@ package types import ( "crypto/sha256" "fmt" + "math/bits" "github.com/celestiaorg/nmt" sdkclient "github.com/cosmos/cosmos-sdk/client" @@ -11,6 +12,7 @@ import ( authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/pkg/consts" + coretypes "github.com/tendermint/tendermint/types" ) const ( @@ -111,17 +113,24 @@ func BuildPayForDataTxFromWireTx( // squaresize using a namespace merkle tree and the rules described at // https://github.com/celestiaorg/celestia-specs/blob/master/src/rationale/message_block_layout.md#message-layout-rationale func CreateCommitment(k uint64, namespace, message []byte) ([]byte, error) { - // add padding to the message if necessary - message = padMessage(message) + msg := coretypes.Messages{ + MessagesList: []coretypes.Message{ + { + NamespaceID: namespace, + Data: message, + }, + }, + } - // break message into shares - shares := chunkMessage(message) + // split into shares that are length delimited and include the namespace in + // each share + shares := msg.SplitIntoShares().RawShares() // if the number of shares is larger than that in the square, throw an error // note, we use k*k-1 here because at least a single share will be reserved // for the transaction paying for the message, therefore the max number of // shares a message can be is number of shares in square -1. - if uint64(len(shares)) > k*k-1 { - return nil, fmt.Errorf("message size exceeds square size") + if uint64(len(shares)) > (k*k)-1 { + return nil, fmt.Errorf("message size exceeds max shares for square size %d: max %d taken %d", k, (k*k)-1, len(shares)) } // organize shares for merkle mountain range @@ -151,34 +160,6 @@ func CreateCommitment(k uint64, namespace, message []byte) ([]byte, error) { return merkle.HashFromByteSlices(subTreeRoots), nil } -// chunkMessage breaks the message into ShareSize pieces -func chunkMessage(message []byte) [][]byte { - var shares [][]byte - for i := 0; i < len(message); i += ShareSize { - end := i + ShareSize - if end > len(message) { - end = len(message) - } - shares = append(shares, message[i:end]) - } - return shares -} - -// padMessage adds padding to the msg if the length of the msg is not divisible -// by the share size specified in celestia-core -func padMessage(msg []byte) []byte { - // check if the message needs padding - if len(msg)%ShareSize == 0 { - return msg - } - - shareCount := (len(msg) / ShareSize) + 1 - - padded := make([]byte, shareCount*ShareSize) - copy(padded, msg) - return padded -} - // powerOf2MountainRange returns the heights of the subtrees for binary merkle // mountian range func powerOf2MountainRange(l, k uint64) []uint64 { @@ -190,7 +171,7 @@ func powerOf2MountainRange(l, k uint64) []uint64 { output = append(output, k) l = l - k case l < k: - p := NextPowerOf2(l) + p := nextLowestPowerOf2(l) output = append(output, p) l = l - p } @@ -199,13 +180,10 @@ func powerOf2MountainRange(l, k uint64) []uint64 { return output } -// NextPowerOf2 returns the next lowest power of 2 unless the input is a power +// NextHighestPowerOf2 returns the next lowest power of 2 unless the input is a power // of two, in which case it returns the input -func NextPowerOf2(v uint64) uint64 { - if v == 1 { - return 1 - } - // keep track of the input +func NextHighestPowerOf2(v uint64) uint64 { + // keep track of the value to check if its the same later i := v // find the next highest power using bit mashing @@ -218,13 +196,20 @@ func NextPowerOf2(v uint64) uint64 { v |= v >> 32 v++ - // check if the input was the next highest power - if i == v { - return v + // force the value to the next highest power of two if its the same + if v == i { + return 2 * v } - // return the next lowest power - return v / 2 + return v +} + +func nextLowestPowerOf2(v uint64) uint64 { + c := NextHighestPowerOf2(v) + if c == v { + return c + } + return c / 2 } // Check if number is power of 2 @@ -235,3 +220,8 @@ func powerOf2(v uint64) bool { return false } } + +// DelimLen calculates the length of the delimiter for a given message size +func DelimLen(x uint64) int { + return 8 - bits.LeadingZeros64(x)%8 +} diff --git a/x/payment/types/payfordata_test.go b/x/payment/types/payfordata_test.go index dff2b7447a..cf89eee8c8 100644 --- a/x/payment/types/payfordata_test.go +++ b/x/payment/types/payfordata_test.go @@ -8,9 +8,9 @@ import ( sdkclient "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" - authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/pkg/consts" ) func TestMountainRange(t *testing.T) { @@ -41,7 +41,7 @@ func TestMountainRange(t *testing.T) { } } -func TestNextPowerOf2(t *testing.T) { +func TestNextLowestPowerOf2(t *testing.T) { type test struct { input uint64 expected uint64 @@ -69,7 +69,40 @@ func TestNextPowerOf2(t *testing.T) { }, } for _, tt := range tests { - res := NextPowerOf2(tt.input) + res := nextLowestPowerOf2(tt.input) + assert.Equal(t, tt.expected, res) + } +} + +func TestNextHighestPowerOf2(t *testing.T) { + type test struct { + input uint64 + expected uint64 + } + tests := []test{ + { + input: 2, + expected: 4, + }, + { + input: 11, + expected: 16, + }, + { + input: 511, + expected: 512, + }, + { + input: 1, + expected: 2, + }, + { + input: 0, + expected: 0, + }, + } + for _, tt := range tests { + res := NextHighestPowerOf2(tt.input) assert.Equal(t, tt.expected, res) } } @@ -126,7 +159,7 @@ func TestCreateCommitment(t *testing.T) { k: 4, namespace: bytes.Repeat([]byte{0xFF}, 8), message: bytes.Repeat([]byte{0xFF}, 11*ShareSize), - expected: []byte{0x1c, 0x57, 0x89, 0x2f, 0xbe, 0xbf, 0xa2, 0xa4, 0x4c, 0x41, 0x9e, 0x2d, 0x88, 0xd5, 0x87, 0xc0, 0xbd, 0x37, 0xc0, 0x85, 0xbd, 0x10, 0x3c, 0x36, 0xd9, 0xa2, 0x4d, 0x4e, 0x31, 0xa2, 0xf8, 0x4e}, + expected: []byte{0xf2, 0xd4, 0xfc, 0x39, 0x4e, 0xf3, 0x97, 0x9d, 0xf4, 0x4c, 0x99, 0x87, 0x36, 0x7d, 0x7d, 0x4, 0xf2, 0xa7, 0x89, 0x26, 0x6d, 0xf5, 0x78, 0xe1, 0xff, 0x72, 0xb4, 0x75, 0x12, 0x1e, 0x71, 0xc3}, }, { k: 2, @@ -146,35 +179,6 @@ func TestCreateCommitment(t *testing.T) { } } -func TestPadMessage(t *testing.T) { - type test struct { - input []byte - expected []byte - } - tests := []test{ - { - input: []byte{1}, - expected: append([]byte{1}, bytes.Repeat([]byte{0}, ShareSize-1)...), - }, - { - input: []byte{}, - expected: []byte{}, - }, - { - input: bytes.Repeat([]byte{1}, ShareSize), - expected: bytes.Repeat([]byte{1}, ShareSize), - }, - { - input: bytes.Repeat([]byte{1}, (3*ShareSize)-10), - expected: append(bytes.Repeat([]byte{1}, (3*ShareSize)-10), bytes.Repeat([]byte{0}, 10)...), - }, - } - for _, tt := range tests { - res := padMessage(tt.input) - assert.Equal(t, tt.expected, res) - } -} - // TestSignMalleatedTxs checks to see that the signatures that are generated for // the PayForDatas malleated from the original WirePayForData are actually // valid. @@ -229,14 +233,13 @@ func TestSignMalleatedTxs(t *testing.T) { tx, err := signer.BuildSignedTx(builder, unsignedPFD) require.NoError(t, err) + signerData, err := signer.GetSignerData() + require.NoError(t, err) + // Generate the bytes to be signed. bytesToSign, err := signer.encCfg.TxConfig.SignModeHandler().GetSignBytes( signing.SignMode_SIGN_MODE_DIRECT, - authsigning.SignerData{ - ChainID: signer.chainID, - AccountNumber: signer.accountNumber, - Sequence: signer.sequence, - }, + signerData, tx, ) require.NoError(t, err) @@ -280,21 +283,21 @@ func TestProcessMessage(t *testing.T) { { name: "single share square size 2", ns: []byte{1, 1, 1, 1, 1, 1, 1, 1}, - msg: bytes.Repeat([]byte{1}, ShareSize), + msg: bytes.Repeat([]byte{1}, totalMsgSize(consts.MsgShareSize)), ss: 2, modify: dontModify, }, { name: "15 shares square size 4", ns: []byte{1, 1, 1, 1, 1, 1, 1, 2}, - msg: bytes.Repeat([]byte{2}, ShareSize*15), + msg: bytes.Repeat([]byte{2}, totalMsgSize(consts.MsgShareSize*15)), ss: 4, modify: dontModify, }, { - name: "", + name: "incorrect square size", ns: []byte{1, 1, 1, 1, 1, 1, 1, 2}, - msg: bytes.Repeat([]byte{2}, ShareSize*15), + msg: bytes.Repeat([]byte{2}, totalMsgSize(consts.MsgShareSize*15)), ss: 4, modify: func(wpfd *MsgWirePayForData) *MsgWirePayForData { wpfd.MessageShareCommitment[0].K = 99999 @@ -328,6 +331,12 @@ func TestProcessMessage(t *testing.T) { } } +// totalMsgSize subtracts the delimiter size from the desired total size. this +// is useful for testing for messages that occupy exactly so many shares. +func totalMsgSize(size int) int { + return size - DelimLen(uint64(size)) +} + func validWirePayForData(t *testing.T) *MsgWirePayForData { msg, err := NewWirePayForData( []byte{1, 2, 3, 4, 5, 6, 7, 8}, diff --git a/x/payment/types/wirepayfordata.go b/x/payment/types/wirepayfordata.go index 877c7273a5..ba2963a24d 100644 --- a/x/payment/types/wirepayfordata.go +++ b/x/payment/types/wirepayfordata.go @@ -2,6 +2,7 @@ package types import ( "bytes" + "errors" fmt "fmt" sdkclient "github.com/cosmos/cosmos-sdk/client" @@ -27,7 +28,6 @@ func NewWirePayForData(namespace, message []byte, sizes ...uint64) (*MsgWirePayF ) } - message = padMessage(message) out := &MsgWirePayForData{ MessageNameSpaceId: namespace, MessageSize: uint64(len(message)), @@ -57,14 +57,17 @@ func (msg *MsgWirePayForData) SignShareCommitments(signer *KeyringSigner, option return err } + if addr == nil { + return errors.New("failed to get address") + } + if addr.Empty() { + return errors.New("failed to get address") + } + msg.Signer = addr.String() // create an entire MsgPayForData and signing over it, including the signature in each commitment for i, commit := range msg.MessageShareCommitment { - builder := signer.NewTxBuilder() - - for _, option := range options { - builder = option(builder) - } + builder := signer.NewTxBuilder(options...) sig, err := msg.createPayForDataSignature(signer, builder, commit.K) if err != nil { @@ -93,15 +96,6 @@ func (msg *MsgWirePayForData) ValidateBasic() error { return sdkerrors.ErrInvalidAddress.Wrapf("invalid 'from' address: %s", err) } - // ensure that the included message is evenly divisible into shares - if msgMod := uint64(len(msg.GetMessage())) % ShareSize; msgMod != 0 { - return ErrInvalidDataSize.Wrapf( - "shareSize: %d, data length: %d", - len(msg.Message), - ShareSize, - ) - } - // make sure that the message size matches the actual size of the message if msg.MessageSize != uint64(len(msg.Message)) { return ErrDeclaredActualDataSizeMismatch.Wrapf( diff --git a/x/payment/types/wirepayfordata_test.go b/x/payment/types/wirepayfordata_test.go index 9f50afdb95..5b40aefc41 100644 --- a/x/payment/types/wirepayfordata_test.go +++ b/x/payment/types/wirepayfordata_test.go @@ -1,12 +1,10 @@ package types import ( - "bytes" "testing" sdkerrors "github.com/cosmos/cosmos-sdk/errors" "github.com/stretchr/testify/assert" - "github.com/tendermint/tendermint/pkg/consts" ) func TestWirePayForData_ValidateBasic(t *testing.T) { @@ -27,10 +25,6 @@ func TestWirePayForData_ValidateBasic(t *testing.T) { reservedMsg := validWirePayForData(t) reservedMsg.MessageNameSpaceId = []byte{0, 0, 0, 0, 0, 0, 0, 100} - // pfd that has a wrong msg size - invalidMsgSizeMsg := validWirePayForData(t) - invalidMsgSizeMsg.Message = bytes.Repeat([]byte{1}, consts.ShareSize-20) - // pfd that has a wrong msg size invalidDeclaredMsgSizeMsg := validWirePayForData(t) invalidDeclaredMsgSizeMsg.MessageSize = 999 @@ -63,11 +57,6 @@ func TestWirePayForData_ValidateBasic(t *testing.T) { msg: reservedMsg, wantErr: ErrReservedNamespace, }, - { - name: "invalid msg size", - msg: invalidMsgSizeMsg, - wantErr: ErrInvalidDataSize, - }, { name: "bad declared message size", msg: invalidDeclaredMsgSizeMsg,