chore: lower the default mempool min gas price #2843
Conversation
app/default_overrides_test.go
Outdated
| @@ -62,7 +62,7 @@ func TestDefaultAppConfig(t *testing.T) { | |||
|
|
|||
| assert.Equal(t, uint64(1500), cfg.StateSync.SnapshotInterval) | |||
| assert.Equal(t, uint32(2), cfg.StateSync.SnapshotKeepRecent) | |||
| assert.Equal(t, "0.1utia", cfg.MinGasPrices) | |||
| assert.Equal(t, "0.01utia", cfg.MinGasPrices) | |||
There was a problem hiding this comment.
[question] how was the previous value of .1 utia determined?
There was a problem hiding this comment.
just rough estimates iirc
rootulp
added
the
backport:v1.x
evan-forbes
dismissed stale reviews from cmwaters, staheri14, and rootulp
via
6933d28
WalkthroughThe recent update involves a reduction in the minimum gas price within a blockchain-related application. This change lowers the cost threshold for transactions to be processed, potentially allowing for higher transaction throughput or lower costs for users. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- app/default_overrides_test.go (1 hunks)
- pkg/appconsts/initial_consts.go (1 hunks)
Additional comments: 2
app/default_overrides_test.go (1)
- 65-65: The change to the
MinGasPricesvalue in the test configuration correctly reflects the new lower minimum gas price of "0.002utia". This is consistent with the pull request's intent to lower the default minimum gas price in the mempool.pkg/appconsts/initial_consts.go (1)
- 20-23: The change to
DefaultMinGasPricefrom 0.1 to 0.002 is consistent with the pull request's description to lower the default minimum gas price. This change will affect theapp.tomlconfiguration and the transaction filtering process. Ensure that all dependent configurations and documentation are updated to reflect this new default value.
|
#makedatafreeagain |
| @@ -62,7 +62,7 @@ func TestDefaultAppConfig(t *testing.T) { | |||
|
|
|||
| assert.Equal(t, uint64(1500), cfg.StateSync.SnapshotInterval) | |||
| assert.Equal(t, uint32(2), cfg.StateSync.SnapshotKeepRecent) | |||
| assert.Equal(t, "0.1utia", cfg.MinGasPrices) | |||
| assert.Equal(t, "0.002utia", cfg.MinGasPrices) | |||
There was a problem hiding this comment.
[question] how was .002utia arrived at? IIUC 0.1utia was arrived at based on rough estimates. How did the estimates change to go from 0.1 -> 0.01 -> 0.002?
There was a problem hiding this comment.
If the original value was off by two orders of magnitude, I would expect 0.1 -> 0.01 -> 0.001
There was a problem hiding this comment.
https://github.com/cosmos/chain-registry/blob/957a4c779024535ea947e13db6b2d101305fcadd/celestia/chain.json#L21 I'll keep an eye on thei thread and match what the consensus here is, in chain-registry
There was a problem hiding this comment.
Since .002utia got merged here, I think we may want to update chain-registry to
"low_gas_price": 0.002,
"average_gas_price": 0.004,
"high_gas_price": 0.008
but we likely only want to do this after a decent chunk of validators have updated to a release that includes this. On second thought, I think their locally configured default won't get overridden with this new default so our release notes should probably explicitly call out the manual action that validators need to take.
There was a problem hiding this comment.
cosmos/chain-registry#3239
chainapsis/keplr-chain-registry#290
updating these now, but leaving in draft
## Overview The mempool minfee seems too high as the goal of it is just to reduce spam if the blocks are empty. The proper solution will come with a consensus critical dynamic minfee. For now the default should be lowered by an order of magnitude. ## Checklist - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [x] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Adjusted the minimum gas price settings for better transaction filtering efficiency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit 1eb203c)
## Overview The mempool minfee seems too high as the goal of it is just to reduce spam if the blocks are empty. The proper solution will come with a consensus critical dynamic minfee. For now the default should be lowered by an order of magnitude. ## Checklist - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [x] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Adjusted the minimum gas price settings for better transaction filtering efficiency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit 1eb203c)
Overview
The mempool minfee seems too high as the goal of it is just to reduce spam if the blocks are empty. The proper solution will come with a consensus critical dynamic minfee. For now the default should be lowered by an order of magnitude.
Checklist
Summary by CodeRabbit