From 92ffba954bd55804f13193e8a5e1767a8cb188b8 Mon Sep 17 00:00:00 2001 From: toni <143221387+xyztoni@users.noreply.github.com> Date: Wed, 21 Feb 2024 20:29:50 +0100 Subject: [PATCH 1/2] chore: typo fixes (#3123) Fixed multiple misspelled words and double word usages. --- specs/src/specs/ante_handler.md | 4 ++-- specs/src/specs/block_validity_rules.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/specs/src/specs/ante_handler.md b/specs/src/specs/ante_handler.md index 82d684da0a..25df1d4aac 100644 --- a/specs/src/specs/ante_handler.md +++ b/specs/src/specs/ante_handler.md @@ -18,7 +18,7 @@ The AnteHandler chains together several decorators to ensure the following crite - The tx's count of signatures <= the max number of signatures. The max number of signatures is [`TxSigLimit = 7`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L231). - The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the tx's signatures. - The tx's [signatures](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/types/tx/signing/signature.go#L10-L26) are valid. For each signature, ensure that the signature's sequence number (a.k.a nonce) matches the account sequence number of the signer. -- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and `gasPerBlobByte` is a governance parameter that can be modified (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). +- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and `gasPerBlobByte` is a governance parameter that can be modified (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). - The tx's total blob size is <= the max blob size. The max blob size is derived from the maximum valid square size. The max valid square size is the minimum of: `GovMaxSquareSize` and `SquareSizeUpperBound`. - The tx does not contain a message of type [MsgSubmitProposal](https://github.com/cosmos/cosmos-sdk/blob/d6d929843bbd331b885467475bcb3050788e30ca/proto/cosmos/gov/v1/tx.proto#L33-L43) with zero proposal messages. - The tx is not an IBC packet or update message that has already been processed. @@ -26,5 +26,5 @@ The AnteHandler chains together several decorators to ensure the following crite In addition to the above criteria, the AnteHandler also has a number of side-effects: - Tx fees are deducted from the tx's feepayer and added to the fee collector module account. -- Tx priority is calculated based on the the smallest denomination of gas price in the tx and set in context. +- Tx priority is calculated based on the smallest denomination of gas price in the tx and set in context. - The nonce of all tx signers is incremented by 1. diff --git a/specs/src/specs/block_validity_rules.md b/specs/src/specs/block_validity_rules.md index 86718b38aa..b95cdf89af 100644 --- a/specs/src/specs/block_validity_rules.md +++ b/specs/src/specs/block_validity_rules.md @@ -9,7 +9,7 @@ constrained light clients must be able to detect when a subset of these validity rules have not been followed in order to avoid making an honest majority assumption on the consensus network. This has a significant impact on their design. More information on how light clients can check the invalidity of a -block can be foud in the [Fraud Proofs](./fraud_proofs.md) spec. +block can be found in the [Fraud Proofs](./fraud_proofs.md) spec. > **Note** Celestia relies on CometBFT (formerly tendermint) for consensus, > meaning that it has single slot finality and is fork-free. Therefore, in order @@ -27,7 +27,7 @@ rules](https://github.com/cometbft/cometbft/blob/v0.34.28/spec/core/data_structu must be followed. Notably, this includes verifying data availability. Consensus nodes verify data -availabily by simply downloading the entire block. +availability by simply downloading the entire block. > **Note** Light clients only sample a fraction of the block. More details on > how sampling actually works can be found in the seminal ["Fraud and Data From 9fe864c9b64ef8d131f3f838062b0efb8ed59848 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 22 Feb 2024 11:05:12 +0100 Subject: [PATCH 2/2] refactor: first block prepare proposal test (#3122) There has been some fakiness in the block production tests. We don't need a full integration test. --- app/test/block_production_test.go | 91 ------------------------------- app/test/prepare_proposal_test.go | 22 ++++++++ test/util/direct_tx_gen.go | 3 + test/util/test_app.go | 35 +++++++----- 4 files changed, 45 insertions(+), 106 deletions(-) delete mode 100644 app/test/block_production_test.go diff --git a/app/test/block_production_test.go b/app/test/block_production_test.go deleted file mode 100644 index 7b401b8d21..0000000000 --- a/app/test/block_production_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package app_test - -import ( - "testing" - "time" - - "github.com/celestiaorg/celestia-app/test/util/testnode" - appns "github.com/celestiaorg/go-square/namespace" - "github.com/cosmos/cosmos-sdk/client/flags" - "github.com/stretchr/testify/suite" - tmrand "github.com/tendermint/tendermint/libs/rand" -) - -func TestBlockProductionTestSuite(t *testing.T) { - if testing.Short() { - t.Skip("skipping block production test suite in short mode.") - } - suite.Run(t, new(BlockProductionTestSuite)) -} - -type BlockProductionTestSuite struct { - suite.Suite - - accounts []string - cctx testnode.Context - timeoutCommit time.Duration -} - -func (s *BlockProductionTestSuite) SetupSuite() { - t := s.T() - s.timeoutCommit = 10 * time.Second - - accounts := make([]string, 40) - for i := 0; i < 40; i++ { - accounts[i] = tmrand.Str(10) - } - - cfg := testnode.DefaultConfig(). - WithFundedAccounts(accounts...). - WithTimeoutCommit(s.timeoutCommit) - - cctx, _, _ := testnode.NewNetwork(t, cfg) - s.cctx = cctx - s.accounts = accounts -} - -// Test_BlockOneTransactionNonInclusion tests that no transactions can be included in the first block. -func (s *BlockProductionTestSuite) Test_BlockOneTransactionNonInclusion() { - require := s.Require() - _, err := s.cctx.PostData(s.accounts[0], flags.BroadcastBlock, appns.RandomBlobNamespace(), tmrand.Bytes(100000)) - - // since the block production is delayed by 10 seconds, the transactions - // posted arrive when the node is still at height 0 (not started height 1 - // yet) this makes the post data fail with the following error: rpc error: - // code = Unknown desc = codespace sdk code 18: invalid request: - // failed to load state at height 0; no commit info found (latest height: 0) - require.Error(err) // change this to require.NoError(err) to see the error - require.ErrorContains(err, "rpc error: code = Unknown desc = codespace sdk code 18: invalid request: failed to load state at height 0; no commit info found (latest height: 0)") -} - -// Test_FirstBlockIsEmpty tests whether the first block is empty. -func (s *BlockProductionTestSuite) Test_FirstBlockIsEmpty() { - require := s.Require() - // wait until height 1 before posting transactions - // otherwise tx submission will fail - time.Sleep(1 * s.timeoutCommit) - // send some transactions, these should be included in the second block - _, err := s.cctx.PostData(s.accounts[0], flags.BroadcastBlock, appns.RandomBlobNamespace(), tmrand.Bytes(100000)) - require.NoError(err) - - // wait for 2*s.timeoutCommit+1*time.Second to ensure that the node is - // at height 2 - _, err = s.cctx.WaitForHeightWithTimeout(2, 2*s.timeoutCommit+1*time.Second) - require.NoError(err) - - // fetch the first block - one := int64(1) - b1, err := s.cctx.Client.Block(s.cctx.GoContext(), &one) - require.NoError(err) - // check whether the first block is empty - require.True(b1.Block.Header.Height == 1) - require.Equal(len(b1.Block.Data.Txs), 0) - - // fetch the second block - two := int64(2) - b2, err := s.cctx.Client.Block(s.cctx.GoContext(), &two) - require.NoError(err) - // check whether the second block is non-empty - require.True(b2.Block.Header.Height == 2) - require.True(len(b2.Block.Data.Txs) == 1) -} diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index 5c958e4ede..7804a5102e 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -217,3 +217,25 @@ func queryAccountInfo(capp *app.App, accs []string, kr keyring.Keyring) []blobfa } return infos } + +func TestPrepareProposalZeroTxsInFirstBlock(t *testing.T) { + accounts := testfactory.GenerateAccounts(6) + testApp, _, kr := testutil.NewTestAppWithGenesisSet(app.DefaultConsensusParams(), accounts...) + require.Equal(t, int64(0), testApp.LastBlockHeight()) + encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) + sendTxs := coretypes.Txs{testutil.SendTxWithManualSequence( + t, + encCfg.TxConfig, + kr, + accounts[0], + accounts[1], + 1000, + testutil.ChainID, + 1, + 1, + )}.ToSliceOfBytes() + resp := testApp.PrepareProposal(abci.RequestPrepareProposal{ + BlockData: &tmproto.Data{Txs: sendTxs}, + }) + require.Len(t, resp.BlockData.Txs, 0) +} diff --git a/test/util/direct_tx_gen.go b/test/util/direct_tx_gen.go index bb9a898cda..fa0b84f055 100644 --- a/test/util/direct_tx_gen.go +++ b/test/util/direct_tx_gen.go @@ -175,6 +175,9 @@ func SendTxsWithAccounts( // update the account info in the signer so the signature is valid acc := DirectQueryAccount(capp, signingAddr) + if acc == nil { + t.Fatalf("account %s not found", signingAddr) + } txs[i] = SendTxWithManualSequence( t, diff --git a/test/util/test_app.go b/test/util/test_app.go index 5fbd47c8de..18a1e29de2 100644 --- a/test/util/test_app.go +++ b/test/util/test_app.go @@ -51,6 +51,25 @@ func (ao EmptyAppOptions) Get(_ string) interface{} { // is bonded with a delegation of one consensus engine unit in the default token // of the app from first genesis account. A no-op logger is set in app. func SetupTestAppWithGenesisValSet(cparams *tmproto.ConsensusParams, genAccounts ...string) (*app.App, keyring.Keyring) { + testApp, valSet, kr := NewTestAppWithGenesisSet(cparams, genAccounts...) + + // commit genesis changes + testApp.Commit() + testApp.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{ + ChainID: ChainID, + Height: testApp.LastBlockHeight() + 1, + AppHash: testApp.LastCommitID().Hash, + ValidatorsHash: valSet.Hash(), + NextValidatorsHash: valSet.Hash(), + Version: tmversion.Consensus{ + App: cparams.Version.AppVersion, + }, + }}) + + return testApp, kr +} + +func NewTestAppWithGenesisSet(cparams *tmproto.ConsensusParams, genAccounts ...string) (*app.App, *tmtypes.ValidatorSet, keyring.Keyring) { // var cache sdk.MultiStorePersistentCache // EmptyAppOptions is a stub implementing AppOptions emptyOpts := EmptyAppOptions{} @@ -98,21 +117,7 @@ func SetupTestAppWithGenesisValSet(cparams *tmproto.ConsensusParams, genAccounts ChainId: ChainID, }, ) - - // commit genesis changes - testApp.Commit() - testApp.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{ - ChainID: ChainID, - Height: testApp.LastBlockHeight() + 1, - AppHash: testApp.LastCommitID().Hash, - ValidatorsHash: valSet.Hash(), - NextValidatorsHash: valSet.Hash(), - Version: tmversion.Consensus{ - App: cparams.Version.AppVersion, - }, - }}) - - return testApp, kr + return testApp, valSet, kr } // AddAccount mimics the cli addAccount command, providing an