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

fix: patch default fee checker priority calculation (backport #2339) #2355

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Aug 24, 2023

This is an automatic backport of pull request #2339 done by Mergify.


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

## Overview

This PR patches the default priority calculation and adds it to our
antehandler. This approach copy paste forks [the code from the
sdk](https://github.com/celestiaorg/cosmos-sdk/blob/64adc6e40ae679003d2f779387e06c4105c99121/x/auth/ante/validator_tx_fee.go#L1-L65)
which is not exported. This avoids us from having to include a change in
the sdk, add more unit tests, more easily change this in the future, and
be more explicit over how priority is determined in the mempool.

This should fix the issue that we're seeing on arabica, however I'm
still writing an integration test to triple check things.

## 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

---------

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
(cherry picked from commit 02f37a3)
@github-actions github-actions bot added the bot item was created by a bot label Aug 24, 2023
@celestia-bot celestia-bot requested a review from a team August 24, 2023 21:57
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (v1.x@d9b6c3a). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             v1.x    #2355   +/-   ##
=======================================
  Coverage        ?   20.57%           
=======================================
  Files           ?      138           
  Lines           ?    15725           
  Branches        ?        0           
=======================================
  Hits            ?     3235           
  Misses          ?    12183           
  Partials        ?      307           

@evan-forbes evan-forbes enabled auto-merge (rebase) August 25, 2023 00:05
@evan-forbes evan-forbes merged commit 0fde03b into v1.x Aug 25, 2023
22 checks passed
@evan-forbes evan-forbes deleted the mergify/bp/v1.x/pr-2339 branch August 25, 2023 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot item was created by a bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants