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

The default priority mechanism should be based on gas price rather than fee #12932

Closed
yihuang opened this issue Aug 16, 2022 · 8 comments · Fixed by #12953
Closed

The default priority mechanism should be based on gas price rather than fee #12932

yihuang opened this issue Aug 16, 2022 · 8 comments · Fixed by #12953

Comments

@yihuang
Copy link
Collaborator

yihuang commented Aug 16, 2022

Summary of Bug

Currently the default priority mechanism is based on tx fee amount: https://github.com/cosmos/cosmos-sdk/blob/release/v0.46.x/x/auth/ante/validator_tx_fee.go#L54, I think it's more reasonable to based on price instead, fee = price * gas limit.

Version

main and 0.46

Steps to Reproduce

@tac0turtle
Copy link
Member

would you not need an oracle to get price of the asset?

@yihuang
Copy link
Collaborator Author

yihuang commented Aug 16, 2022

would you not need an oracle to get price of the asset?

I mean the gas price specified in the tx, gas price = fee / gas limit, and use the amount of the price as the priority.
Otherwise, a tx that uses more gas will have a higher priority, even with the same gas price.

@yihuang
Copy link
Collaborator Author

yihuang commented Aug 16, 2022

It's also a workaround for this issue: informalsystems/hermes#2350
Because currently relayer is always configured with a static gas price, and with this change, it'll always get a static priority, which prevents the relayer msgs from reorder.

@yihuang yihuang changed the title The default priority mechanism should be based on price rather than fee The default priority mechanism should be based on gas price rather than fee Aug 16, 2022
@alexanderbez
Copy link
Contributor

alexanderbez commented Aug 16, 2022

I mean we can base it on gas price, but what does this buy us if the fee is based on the gas price? Also, the idea is that apps write their own priority AnteHandler.

Otherwise, a tx that uses more gas will have a higher priority, even with the same gas price.

Correct, but why is this a bad thing? Are you saying it's to give them equal/fair ranking?

@yihuang
Copy link
Collaborator Author

yihuang commented Aug 17, 2022

I mean we can base it on gas price, but what does this buy us if the fee is based on the gas price? Also, the idea is that apps write their own priority AnteHandler.

One concrete positivity is that the default setting won't break the IBC relayers, because currently they always set a static gas price, and they don't want their transactions to get reordered.

Otherwise, a tx that uses more gas will have a higher priority, even with the same gas price.

Correct, but why is this a bad thing? Are you saying it's to give them equal/fair ranking?

Yeah, it seems fairer to me, that's how ethereum has always been doing before EIP-1559.

@alexanderbez
Copy link
Contributor

Ok, I don't see this as a problem actually. Makes sense. Wanna submit a quick PR @yihuang? :)

yihuang added a commit to yihuang/cosmos-sdk that referenced this issue Aug 18, 2022
Closes: cosmos#12932
gas price based priority is more intuitive and works better with ibc relayers.
@yihuang
Copy link
Collaborator Author

yihuang commented Aug 18, 2022

Ok, I don't see this as a problem actually. Makes sense. Wanna submit a quick PR @yihuang? :)

sure, submitted.

@robert-zaremba
Copy link
Collaborator

One thing to note is that by default transactions are not charged based on the gas used but by the specified fee. So the fee equals gas_price * gas_limit (when gas price is provided)

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

Successfully merging a pull request may close this issue.

4 participants