Repository: celestiaorg/celestia-app

Issue:
Increase minimum local gas price from 0.002 to 0.004 utia
The change in the value of TIA has made DA much cheaper in recent months. To account for that, we are exploring doubling the minimum gas price to 0.004 utia. This needs to be done in a way with minimal disturbances to operating services. 

**[EDITED]** The current proposal looks as follows: 

v5:
- config minimum gas price "0.02utia"
- default minimum gas price 0.02utia
v6 (binary)
- config minimum gas price "0.02utia"
- new config minimum gas price "" (unset)
- logic for every received tx:
   - If config minimum is set use that
   - If after time, t, use new minimum gas price: 0.04
   - else use old minimum gas price: 0.02
- gas estimation service will return at least 0.04utia
Operator migrates config celestia-appd config migrate
- config minimum gas price "" (unset)


Notes:
- we should probably do this in v6 to ensure that all nodes have upgraded. Most operators don't do minor upgrades.
- Rather than some arbitrary time t, It would be simpler to just tie it with the v6 state machine. If we want to make future changes quickly we can also a) update local configs, b) update network-wide parameter or c) introduce a time variable after which to change.

The steps to making this change are as follows:

- [x] Ensure the Lumina rust light node either adopts the gas fee estimator API (CIP 018) or bumps the min fee constant to 0.004 utia
- [x] #5175
- [x] [check that consensus nodes should not provide a fee estimate that is less than their local minimum #5174](https://github.com/celestiaorg/celestia-app/issues/5174)
    - [x] v6 consensus nodes should return the greater of network, binary and configured minimums
- [x] Command to unset minimum gas price i.e. "" (See https://github.com/celestiaorg/celestia-app/issues/5366)  
- [x] Check if unset, then use the default minimum gas price of 0.04 (after time, t) or 0.02 (if before time, t)
- [x] Check that Keplr, Leap and other wallets that don't use CIP 018 will not be affected by the bump (or have already adjusted to it)
- [ ] Rollout to arabica and check the both light nodes and thus supported integrations work unaffected
- [ ] Rollout to mocha, wait one week for nodes to upgrade and check for problems
- [ ] Rollout to mainnet



Discussion:
@musalbas: Don't we also need to bump the min fee constant in celestia-node?

We should bump the min fee constant rather than simply rely on core/app fee node estimator, because if the light node uses an outdated core/app node, their transaction is less likely to be included.@walldiss: MinGasPrice could be pre-set manually on user side. Solution is not backwards compatible for such users (breaking). We need to ensure users who has the gas value hardcoded will update it on their side. Those include cases:
- hardcoded to default constant (requires node and lumina breaking bump)
- hardcoded to some arbitrary value (requires user to revisit the values)
- users who submit raw Tx to grpc without txClient (same as above)@cmwaters: > We should bump the min fee constant

We will. This is what this item is for 

> - [ ] Update the default values in celestia-app

---

> Don't we also need to bump the min fee constant in celestia-node?

Afaiu, there is no constant in celestia-node, and if there is, it's imported from celestia-app@cmwaters: The only usage of the min gas price in node that I can see occurs here: 

https://github.com/celestiaorg/celestia-node/blob/cb1ac073757cb528c9ec5395eb28192a29ee11cc/state/tx_config.go#L19

This doesn't need to be modified

@cmwaters: Users that manually set the gas price take fluctuations of the minimum gas price into their own hands. We don't want to override what they set. We instead need to warn them of the upcoming changes so they are prepared. It hasn't really been captured here but it's expected that the adoption / solutions team who talk with users on a regular basis will be able to inform them of the upcoming change@cmwaters: Checked in with Eiger regarding Lumina. Their light node doesn't rely on a fixed constant, rather they query the nodes minimum local gas price upon start up. If a transaction fails because it is too low, they then requery the min gas price (in case it has changed), that should be robust enough to not require any further changes to their software but we will double test on arabica and mocha to confirm@musalbas: Would that not run into the following issue?

>We should bump the min fee constant rather than simply rely on core/app fee node estimator, because if the light node uses an outdated core/app node, their transaction is less likely to be included.@cmwaters: > Would that not run into the following issue?

No, lumina has no min fee constant that they rely on@musalbas: Does that mean if a lumina client uses an outdated consensus node, transactions will fail? @cmwaters: No, lumina light node will query whatever minimum gas price the consensus node has, and will use that. It could be 0.002 or 0.004 or any other number that the node operator has manually configured@cmwaters: Leap and Keplr gas prices are above the new minimum default so are not affected:

https://github.com/chainapsis/keplr-chain-registry/blob/1004658b014dc5655936c4c4172b345b78c484eb/cosmos/celestia.json#L33-L36

https://github.com/cosmos/chain-registry/blob/8ddc6760e4bb7c60f0b56caeca2beb9e76e87e5a/celestia/chain.json#L20-L22@musalbas: Yeah but what if it uses an outdated consensus node? @cmwaters: the local minimum gas price and corresponding grpc endpoint has existed since the genesis launch@musalbas: Apologies, I'm not following. Are you implying Lumina uses a hardcoded/non-configurable gRPC endpoint?@cmwaters: It's okay. Lumina uses a configurable gRPC endpoint@musalbas: Right - so what happens if Lumina is configured with an outdated consensus node with too low a min fee?@cmwaters: Ah then they will pay that lower min fee. The consensus node will also accept that min fee and gossip the transaction around the network and it might even reach a proposer that is also outdated and adds it into the block (since nodes could upgrade at different times). The best way to coordinate an increase is through the network wide governance parameter else it can always happen that a transaction is accepted into a nodes mempool but never makes it on chain (or is delayed). 

We discussed this in yesterdays sync (especially given that we don't want to overwrite users config's). The option we agreed on is to add a new variable, something like `MinimumConfigurableGasPrice` which will be another constant that sets how low a node operator can set their minimum gas price (isn't consensus enforced). It will be tied with v5, that way when everyone upgrades to v5, the minimum gas price will be at least 0.04utia@musalbas: >Ah then they will pay that lower min fee. The consensus node will also accept that min fee and gossip the transaction around the network and it might even reach a proposer that is also outdated and adds it into the block (since nodes could upgrade at different times). The best way to coordinate an increase is through the network wide governance parameter else it can always happen that a transaction is accepted into a nodes mempool but never makes it on chain (or is delayed).

Right, but doesn't that mean they will have much worse transaction submission performance as their transaction might not included for a long time if most validators updated their defaults?@evan-forbes: @evan-forbes update in update-config command flups

Solution patch:
diff --git a/app/ante/fee.go b/app/ante/fee.go
index d3230498..5285b511 100644
--- a/app/ante/fee.go
+++ b/app/ante/fee.go
@@ -39,7 +39,12 @@ func ValidateTxFee(ctx sdk.Context, tx sdk.Tx, minfeeKeeper *minfeekeeper.Keeper
 	// This is only for local mempool purposes, and thus
 	// is only run on check tx.
 	if ctx.IsCheckTx() {
-		minGasPrice := ctx.MinGasPrices().AmountOf(appconsts.BondDenom)
+		// if the config is "" then we use the default min gas price
+		minGasPrice := math.LegacyNewDecWithPrec(int64(appconsts.DefaultMinGasPrice*1_000_000), 6)
+		if ctx.MinGasPrices().Len() > 0 {
+			minGasPrice = ctx.MinGasPrices().AmountOf(appconsts.BondDenom)
+		}
+		// NOTE: users can still specify a min gas price of 0utia
 		if !minGasPrice.IsZero() {
 			err := verifyMinFee(fee, gas, minGasPrice, "insufficient minimum gas price for this node")
 			if err != nil {



Test patch:
diff --git a/app/ante/fee_test.go b/app/ante/fee_test.go
index d6fc3823..76e05d4a 100644
--- a/app/ante/fee_test.go
+++ b/app/ante/fee_test.go
@@ -41,76 +41,107 @@ func TestValidateTxFee(t *testing.T) {
 
 	// Set the validator's fee
 	validatorMinGasPrice := 0.8
-	validatorMinGasPriceDec, err := sdkmath.LegacyNewDecFromStr(fmt.Sprintf("%f", validatorMinGasPrice))
-	require.NoError(t, err)
-	validatorMinGasPriceCoin := sdk.NewDecCoinFromDec(appconsts.BondDenom, validatorMinGasPriceDec)
+	validatorMinGasPriceCoin := fmt.Sprintf("%f%s", validatorMinGasPrice, appconsts.BondDenom)
 
 	feeAmount := int64(1000)
 
 	paramsKeeper, minFeeKeeper, stateStore := setUp(t)
 
 	testCases := []struct {
-		name      string
-		fee       sdk.Coins
-		gasLimit  uint64
-		isCheckTx bool
-		expErr    bool
+		name        string
+		fee         sdk.Coins
+		gasLimit    uint64
+		isCheckTx   bool
+		expErr      bool
+		minGasPrice string
 	}{
 		{
-			name:      "bad tx; fee below required minimum",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
-			gasLimit:  uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice),
-			isCheckTx: false,
-			expErr:    true,
+			name:        "bad tx; fee below required minimum",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
+			gasLimit:    uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice),
+			isCheckTx:   false,
+			expErr:      true,
+			minGasPrice: validatorMinGasPriceCoin,
+		},
+		{
+			name:        "good tx; fee equal to required minimum",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
+			gasLimit:    uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice),
+			isCheckTx:   false,
+			expErr:      false,
+			minGasPrice: validatorMinGasPriceCoin,
+		},
+		{
+			name:        "good tx; fee above required minimum",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
+			gasLimit:    uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice),
+			isCheckTx:   false,
+			expErr:      false,
+			minGasPrice: validatorMinGasPriceCoin,
+		},
+		{
+			name:        "good tx; gas limit and fee are maximum values",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, math.MaxInt64)),
+			gasLimit:    math.MaxUint64,
+			isCheckTx:   false,
+			expErr:      false,
+			minGasPrice: validatorMinGasPriceCoin,
 		},
 		{
-			name:      "good tx; fee equal to required minimum",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
-			gasLimit:  uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice),
-			isCheckTx: false,
-			expErr:    false,
+			name:        "bad tx; gas limit and fee are 0",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, 0)),
+			gasLimit:    0,
+			isCheckTx:   false,
+			expErr:      false,
+			minGasPrice: validatorMinGasPriceCoin,
 		},
 		{
-			name:      "good tx; fee above required minimum",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
-			gasLimit:  uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice),
-			isCheckTx: false,
-			expErr:    false,
+			name:        "good tx; minFee = 0.8, rounds up to 1",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
+			gasLimit:    400,
+			isCheckTx:   false,
+			expErr:      false,
+			minGasPrice: validatorMinGasPriceCoin,
 		},
 		{
-			name:      "good tx; gas limit and fee are maximum values",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, math.MaxInt64)),
-			gasLimit:  math.MaxUint64,
-			isCheckTx: false,
-			expErr:    false,
+			name:        "good tx; fee above node's required minimum",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
+			gasLimit:    uint64(float64(feeAmount) / validatorMinGasPrice),
+			isCheckTx:   true,
+			expErr:      false,
+			minGasPrice: validatorMinGasPriceCoin,
 		},
 		{
-			name:      "bad tx; gas limit and fee are 0",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, 0)),
-			gasLimit:  0,
-			isCheckTx: false,
-			expErr:    false,
+			name:        "bad tx; fee below node's required minimum",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
+			gasLimit:    uint64(float64(feeAmount) / validatorMinGasPrice),
+			isCheckTx:   true,
+			expErr:      true,
+			minGasPrice: validatorMinGasPriceCoin,
 		},
 		{
-			name:      "good tx; minFee = 0.8, rounds up to 1",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
-			gasLimit:  400,
-			isCheckTx: false,
-			expErr:    false,
+			name:        "min gas price is empty",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
+			gasLimit:    uint64(float64(feeAmount) / appconsts.DefaultMinGasPrice),
+			isCheckTx:   true,
+			expErr:      false,
+			minGasPrice: "", // should use the default min gas price
 		},
 		{
-			name:      "good tx; fee above node's required minimum",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
-			gasLimit:  uint64(float64(feeAmount) / validatorMinGasPrice),
-			isCheckTx: true,
-			expErr:    false,
+			name:        "min gas price is empty",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
+			gasLimit:    uint64(float64(feeAmount) / appconsts.DefaultMinGasPrice),
+			isCheckTx:   true,
+			expErr:      true,
+			minGasPrice: "", // should use the default min gas price
 		},
 		{
-			name:      "bad tx; fee below node's required minimum",
-			fee:       sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
-			gasLimit:  uint64(float64(feeAmount) / validatorMinGasPrice),
-			isCheckTx: true,
-			expErr:    true,
+			name:        "min gas price is 0utia",
+			fee:         sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
+			gasLimit:    uint64(float64(feeAmount) / appconsts.DefaultMinGasPrice),
+			isCheckTx:   true,
+			expErr:      false,
+			minGasPrice: "0utia",
 		},
 	}
 
@@ -121,7 +152,9 @@ func TestValidateTxFee(t *testing.T) {
 			tx := builder.GetTx()
 
 			ctx := sdk.NewContext(stateStore, tmproto.Header{}, tc.isCheckTx, log.NewNopLogger())
-			ctx = ctx.WithMinGasPrices(sdk.DecCoins{validatorMinGasPriceCoin})
+			minPrice, err := sdk.ParseDecCoins(tc.minGasPrice)
+			require.NoError(t, err)
+			ctx = ctx.WithMinGasPrices(minPrice)
 
 			networkMinGasPriceDec, err := sdkmath.LegacyNewDecFromStr(fmt.Sprintf("%f", appconsts.DefaultNetworkMinGasPrice))
 			require.NoError(t, err)
@@ -144,6 +177,17 @@ func TestValidateTxFee(t *testing.T) {
 	}
 }
 
+func TestParseMinGasPrice(t *testing.T) {
+	emptyCoins, err := sdk.ParseDecCoins("")
+	require.NoError(t, err)
+	require.Equal(t, emptyCoins.String(), "")
+	require.Len(t, emptyCoins, 0)
+
+	oneCoin, err := sdk.ParseDecCoins("0utia")
+	require.NoError(t, err)
+	require.Zero(t, oneCoin.AmountOf(appconsts.BondDenom).BigInt().Int64())
+}
+
 func setUp(t *testing.T) (paramkeeper.Keeper, *minfeekeeper.Keeper, storetypes.CommitMultiStore) {
 	storeKey := storetypes.NewKVStoreKey(paramtypes.StoreKey)
 	mfStoreKey := storetypes.NewKVStoreKey(minfeetypes.StoreKey)