Verify gas consumption of PFD

[rootulp]

Summary of Bug

having ran a few rollmint chains submitting PFDs and submitting PFDs manually, I've never actually seen my balance change

Source @jcstein

Version

Arabica

Steps to Reproduce

1. Submit a PFD
2. Verify that balance does / does not change

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[rootulp]

I couldn't easily acquire Arabica tokens but I was able to repro on main on a local devnet

$ celestia-appd query account celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
'@type': /cosmos.auth.v1beta1.BaseAccount
account_number: "0"
address: celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
pub_key:
  '@type': /cosmos.crypto.secp256k1.PubKey
  key: A5GDXc2/suE5N2biyVplRXg3vYzizF6+lCdyS0PNI1RM
sequence: "1"

$ celestia-appd query bank balances celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
balances:
- amount: "999995000000000"
  denom: utia
pagination:
  next_key: null
  total: "0"

$ celestia-appd tx blob payForData 0101010101010101 0101010101010101 --from validator --keyring-backend test
// trimmed output

wait a few blocks

$ celestia-appd query account celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
'@type': /cosmos.auth.v1beta1.BaseAccount
account_number: "0"
address: celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
pub_key:
  '@type': /cosmos.crypto.secp256k1.PubKey
  key: A5GDXc2/suE5N2biyVplRXg3vYzizF6+lCdyS0PNI1RM
sequence: "2"

$ celestia-appd query bank balances celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
balances:
- amount: "999995000000000"
  denom: utia
pagination:
  next_key: null
  total: "0"

[rootulp]

Context

• Deduct fees for PayForData transactions  #380
• Ensure fees were deducted when submitting a PayForData in the cli integration test #389

[evan-forbes]

I think the fee is set to zero by default, and unless the node has changed their app.toml config, then fee free txs will be allowed to be submitted and included in a block.

This could still be a bug tho, we should verify after specifying a fee

[rootulp]

Cosmos SDK docs on Main Gas Meter:

Gas consumption can be done manually, generally by the module developer in the BeginBlocker, EndBlocker or Msg service, but most of the time it is done automatically whenever there is a read or write to the store. This automatic gas consumption logic is implemented in a special store called GasKv.

Even though keeper isn't explicitly listed as a location to do gas consumption, it seems possible per x/authz/keeper/keeper.go. If keeper isn't a valid location to do gas consumption, I think we could use the Msg service.

[evan-forbes]

Even though keeper isn't explicitly listed as a location to do gas consumption, it seems possible

I'm confused, are we not consuming gas in the same way here?

[rootulp]

Ahh @evan-forbes is correct, I wasn't specifying a fee:

$ celestia-appd tx blob payForData 0101010101010101 0101010101010101 --from validator --keyring-backend test --help --fees 10utia
// trimmed output

wait a few blocks

$ celestia-appd query bank balances celestia1g5n0ma5k3sqswfdge4k32525gdpcmcr8gqvh6z
balances:
- amount: "999994999999990"
  denom: utia
pagination:
  next_key: null
  total: "0"

I wonder if we should specify a default fee of 1utia in config so that testnet transactions more closely resemble mainnet behavior

[rootulp]

I'm confused, are we not consuming gas in the same way here?

Right we do. I was under the impression that this line wasn't working as expected but it does appear working and valid so we don't need to change it.

[evan-forbes]

phew, ok I was panicking there a bit 😅

we have

Undecided
default minimum fee in app.toml

#669 (comment) fwiw, which would require some fee by default. We will definitely have some value there other than zero, but not sure what yet. It kinda depends on the token price

[rootulp]

phew, ok I was panicking there a bit 😅

Same

We will definitely have some value there other than zero

Thanks for the link. Agreed on some non-zero value. Out of curiosity, where would we define something like:

minimum-gas-prices = "1utia"

such that it ends up in the app.toml?

[evan-forbes]

we should just be able to change this iirc

celestia-app/cmd/celestia-appd/cmd/root.go

Line 111 in 18c9cab

srvCfg.MinGasPrices = fmt.Sprintf("0%s", app.BondDenom)

[evan-forbes]

can we close this issue?

[jcstein]

thank you for clearing this up 🙏