test(testnode): configure custom min gas price #3680
Conversation
|
The sdk context on v1.x is populated with a min gas price but on main it isn't. I think this regression was caused by a refactor to testnode b/c the v1.x testnode sets min gas price here but on main, it uses the default app creator which doesn't set it. |
| encodingConfig, //encoding config | ||
| 0, // upgrade height v2 | ||
| simapp.EmptyAppOptions{}, | ||
| baseapp.SetMinGasPrices(fmt.Sprintf("%v%v", appconsts.DefaultMinGasPrice, app.BondDenom)), |
There was a problem hiding this comment.
this line is the fix
There was a problem hiding this comment.
when did this break? out of curiosity
There was a problem hiding this comment.
It works on v1.x but it is broken on main / v2.x. I think it broken during a refactor to testnode, potentially https://github.com/celestiaorg/celestia-app/pull/1991/files#diff-1408d16ad448ff1be464a1eb5af128736aca3302ba3a5d7fbebd7de86473a478R107
rootulp
added
the
backport:v2.x
rootulp
requested review from
cmwaters and
evan-forbes
and removed request for
a team
WalkthroughWalkthroughRecent changes enhance testing within the project by introducing new test cases and updating the test infrastructure. New test cases for querying minimum gas prices are added, and transaction handling in tests is refined. Additionally, configuration for test nodes is expanded with customizable application creators. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Test Suite
participant Config as Configurator
participant Network as Test Network
participant TxClient as Transaction Client
Tester->>Config: Load Default Config
Config->>Network: Create Test Network with Config
Network->>Tester: Provide Test Context
Tester->>TxClient: Submit Transaction with Custom Gas Limit
TxClient->>Network: Process Transaction
Network->>TxClient: Return Response
TxClient->>Tester: Assert Gas Limit in Response
sequenceDiagram
participant Tester as Test Suite
participant Config as Configurator
participant Network as Test Network
participant MinFeeClient as Min Gas Price Client
Tester->>Config: Load Config with Custom App Creator
Config->>Network: Create Test Network with Custom App
Network->>Tester: Provide Test Context
Tester->>MinFeeClient: Query Network Min Gas Price
MinFeeClient->>Network: Request Min Gas Price
Network->>MinFeeClient: Return Min Gas Price
MinFeeClient->>Tester: Validate Min Gas Price
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
app/test/testnode_test.go (1)
34-42: New test case: Query local minimum gas priceThis test case initializes a network, creates a service client, and checks the local minimum gas price against a hardcoded expected value. The test logic is clear, and the use of
require.NoErrorandassert.Equalensures proper error handling and value checking. However, the hardcoded minimum gas price value might need revisions if the configuration changes.Consider externalizing the hardcoded gas price value to a configuration or constant for easier maintenance.
Context: celestiaorg/celestia-node#3453 (comment) Fix testnode so that it is possible to query the local min gas price again. I plan on backporting this to v2.x to fix celestia-node integration tests which query local min gas price. (cherry picked from commit 5b27eba) # Conflicts: # app/test/testnode_test.go
Context: celestiaorg/celestia-node#3453 (comment) Fix testnode so that it is possible to query the local min gas price again. I plan on backporting this to v2.x to fix celestia-node integration tests which query local min gas price.<hr>This is an automatic backport of pull request #3680 done by [Mergify](https://mergify.com). --------- Co-authored-by: Rootul P <rootulp@gmail.com>
Context: celestiaorg/celestia-node#3453 (comment)
Fix testnode so that it is possible to query the local min gas price again. I plan on backporting this to v2.x to fix celestia-node integration tests which query local min gas price.