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.

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

feat/photon: prevent node runner to set uatone as only gas fees #68

Open
albttx opened this issue Dec 19, 2024 · 2 comments
Open

feat/photon: prevent node runner to set uatone as only gas fees #68

albttx opened this issue Dec 19, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@albttx
Copy link

albttx commented Dec 19, 2024

When running node on #57 ( feat/photon ), if the RPC is set with uatone as a gas fees, outputs could be weird to understand.

here is an example:

$ atomoned tx bank send my-wallet atone1xxxx 500000000uatone --yes --fees 5000uatone
[...]
raw_log: 'fee denom uatone not allowed: invalid fee token'

$ atomoned tx bank send my-wallet atone1xxxxxx 500000000uatone --yes --fees 5000uphoton
[...]
raw_log: 'insufficient fees; got: 5000uphoton required: 200uatone: insufficient fee'

A warning at start of the node or enforce a default minimum-gas-fees with format 0.001uatone,0.001uphoton would be a good idea i believe.

@albttx albttx added the bug Something isn't working label Dec 19, 2024
@albttx albttx changed the title feat/photon: prevent node runner to set uatone as gas fees feat/photon: prevent node runner to set uatone as only gas fees Dec 19, 2024
@tbruyelle tbruyelle added enhancement New feature or request and removed bug Something isn't working labels Dec 19, 2024
@tbruyelle
Copy link
Collaborator

The first error is returned by the photon ante handler which only accepts PHOTON for the fee token.
The second error is returned by the cosmos-sdk TxFeeChecker which ensures that the fee tokens passed in the transaction are compliant with the minimum-gas-prices config of the node.

While this is a normal behavior, having them in a row like that is confusing I admit, even considering that the node operator has simply not followed the v2 upgrading procedure.

As you suggested, we could return an error if the minimum-gas-prices does not contain at least PHOTON, but that sets a kind of precedent, as this setting is usually left to the node operator. I am not sure if we should actually check it.

Wdyt @giunatale ?

@giunatale
Copy link
Collaborator

agreed with @tbruyelle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants