Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: reduce testnode overrides #4066

Merged
merged 2 commits into from
Dec 2, 2024
Merged

test: reduce testnode overrides #4066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c1727b
test: reduce testnode overrides
rootulp Nov 27, 2024
d8c24c7
fix: TestIntegrationTestSuite/TestFillBlock
rootulp Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion test/util/testnode/comet_node_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
abci "github.com/tendermint/tendermint/abci/types"
tmconfig "github.com/tendermint/tendermint/config"
tmrand "github.com/tendermint/tendermint/libs/rand"
coretypes "github.com/tendermint/tendermint/rpc/core/types"
)
Expand All @@ -34,6 +35,23 @@ type IntegrationTestSuite struct {
cctx Context
}

func customTendermintConfig() *tmconfig.Config {
tmCfg := DefaultTendermintConfig()
// Override the mempool's MaxTxBytes to allow the testnode to accept a
// transaction that fills the entire square. Any blob transaction larger
// than the square size will still fail no matter what.
maxTxBytes := appconsts.DefaultUpperBoundMaxBytes
tmCfg.Mempool.MaxTxBytes = maxTxBytes

// Override the MaxBodyBytes to allow the testnode to accept very large
// transactions and respond to queries with large responses (200 MiB was
// chosen only as an arbitrary large number).
tmCfg.RPC.MaxBodyBytes = 200 * mebibyte

tmCfg.RPC.TimeoutBroadcastTxCommit = time.Minute
return tmCfg
}
Comment on lines +38 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the results, I can now provide a final response since we have found the mainnet configuration values in app/default_overrides.go.

Adjust test configuration values to match mainnet settings

The test configuration values significantly differ from mainnet settings and should be aligned:

  • MaxBodyBytes: Test uses 200 MiB vs mainnet's 8 MiB
  • TimeoutBroadcastTxCommit: Test uses 60s vs mainnet's 50s

Please update these values in customTendermintConfig() to match the mainnet configuration from app/default_overrides.go.

🔗 Analysis chain

Verify configuration values against mainnet

According to the PR objectives, these configurations should align with mainnet values. Let's verify these settings.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for mainnet configuration references
rg -A 5 "MaxBodyBytes|TimeoutBroadcastTxCommit" --type go

# Search for any other test configurations
ast-grep --pattern 'func $_()*Config() *tmconfig.Config {
  $$$
}'

Length of output: 2080


func (s *IntegrationTestSuite) SetupSuite() {
t := s.T()
s.accounts = RandomAccounts(10)
Expand All @@ -44,7 +62,8 @@ func (s *IntegrationTestSuite) SetupSuite() {

cfg := DefaultConfig().
WithFundedAccounts(s.accounts...).
WithModifiers(genesis.SetBlobParams(ecfg.Codec, blobGenState.Params))
WithModifiers(genesis.SetBlobParams(ecfg.Codec, blobGenState.Params)).
WithTendermintConfig(customTendermintConfig())

cctx, _, _ := NewNetwork(t, cfg)
s.cctx = cctx
Expand Down
15 changes: 1 addition & 14 deletions test/util/testnode/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,20 +150,7 @@ func DefaultTendermintConfig() *tmconfig.Config {
// node produces blocks.
tmCfg.Consensus.TimeoutCommit = 1 * time.Millisecond

// Override the mempool's MaxTxBytes to allow the testnode to accept a
// transaction that fills the entire square. Any blob transaction larger
// than the square size will still fail no matter what.
maxTxBytes := appconsts.DefaultUpperBoundMaxBytes
tmCfg.Mempool.MaxTxBytes = maxTxBytes

// Override the MaxBodyBytes to allow the testnode to accept very large
// transactions and respond to queries with large responses (200 MiB was
// chosen only as an arbitrary large number).
tmCfg.RPC.MaxBodyBytes = 200 * mebibyte

tmCfg.RPC.TimeoutBroadcastTxCommit = time.Minute

// set all the ports to random open ones
// Set all the ports to random open ones.
tmCfg.RPC.ListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort())
tmCfg.P2P.ListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort())
tmCfg.RPC.GRPCListenAddress = fmt.Sprintf("tcp://127.0.0.1:%d", mustGetFreePort())
Comment on lines +153 to 156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Caution: Configuration removal requires test adjustments

The removed configurations are actively used in test files and serve specific purposes:

  • test/util/testnode/comet_node_test.go uses these settings to handle large transactions and responses
  • test/e2e/testnet/setup.go and test/e2e/benchmark/benchmark.go configure mempool size for test scenarios

These test-specific configurations should be preserved or alternative approaches should be implemented to maintain test functionality.

🔗 Analysis chain

LGTM! Configuration simplification aligns with mainnet.

The removal of custom mempool and RPC configurations while keeping only the necessary network address settings aligns well with the PR objective of matching mainnet configuration. This change ensures that tests run with settings closer to production.

Let's verify that no other tests rely on the removed configurations:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for tests that might be affected by the removal of MaxTxBytes, MaxBodyBytes, and TimeoutBroadcastTxCommit configs
rg -l "MaxTxBytes|MaxBodyBytes|TimeoutBroadcastTxCommit" --type go test/

Length of output: 170

Script:

#!/bin/bash
# Let's check the actual usage of these configurations in the found files
echo "=== test/util/testnode/comet_node_test.go ==="
rg -A 5 "MaxTxBytes|MaxBodyBytes|TimeoutBroadcastTxCommit" test/util/testnode/comet_node_test.go

echo -e "\n=== test/e2e/testnet/setup.go ==="
rg -A 5 "MaxTxBytes|MaxBodyBytes|TimeoutBroadcastTxCommit" test/e2e/testnet/setup.go

echo -e "\n=== test/e2e/benchmark/benchmark.go ==="
rg -A 5 "MaxTxBytes|MaxBodyBytes|TimeoutBroadcastTxCommit" test/e2e/benchmark/benchmark.go

Length of output: 1674

Expand Down
Loading