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(e2e): fix e2e test for consensus min gas fee #4244 #4317

Merged

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Feb 14, 2023

Closes: #XXX

This relates to #4244

  • updating chain init image needed for upgrade testing
  • fixing e2e chain init image upload in ci to Dockerhub: fix(ci): incorrect e2e images pushed to Docker hub #4319
  • updating pre-upgrade pool id used in various tests
  • setting the config for the hermes relayer to account for the new fee logic
  • fixing SendIBC to exclude fee token from the balance check
  • adding docs

@p0mvn p0mvn changed the title chore(e2e): update init image on main for #4244 chore(e2e): fix e2e test for consensus min gas fee #4244 Feb 14, 2023
@p0mvn p0mvn changed the title chore(e2e): fix e2e test for consensus min gas fee #4244 fix(e2e): fix e2e test for consensus min gas fee #4244 Feb 14, 2023
@github-actions github-actions bot added the C:docs Improvements or additions to documentation label Feb 15, 2023
@p0mvn p0mvn added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch labels Feb 15, 2023
@@ -57,7 +58,8 @@ account_prefix = 'osmo'
key_name = 'val01-osmosis-b'
store_prefix = 'ibc'
max_gas = 6000000
gas_price = { price = 0.000, denom = 'uosmo' }
default_gas = 400000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we putting default gas? Won't we get out of gas errors routinely?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU, this is what the relayer assumes for calculating fees when estimation fails ref: informalsystems/hermes#1457

We use the value of 40k in the tx arguments as well so I just reused it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, the only remaining problem is the geometric twap failing for reasons I cannot currently explain. Please let me know if something immediately comes to mind as the solution.

I'm going to keep investigating in the meantime

@p0mvn p0mvn marked this pull request as ready for review February 16, 2023 10:08
@p0mvn p0mvn requested a review from ValarDragon February 16, 2023 10:09
@ValarDragon ValarDragon merged commit 63c07fa into dev/consensus_min_gas_fee Feb 16, 2023
@ValarDragon ValarDragon deleted the roman/dev/consensus_min_gas_fee branch February 16, 2023 10:27
ValarDragon added a commit that referenced this pull request Feb 16, 2023
* Refactor tests a bit

* Add consensus min gas fee

* Fix tests

* Delete more crud

* Cleanup more duplicate test cases

* flip isCheckTx sign

* One more test needed

* Run invalid fee denom test across both check & deliver

* Remove extra line

* Add hack to get around txfee problem in ibctesting

* Handle genesis

* Minor code cleanup

* tryfix simulation

* Fix for legacy simulator

* Update x/txfees/keeper/feedecorator_test.go

Co-authored-by: Matt, Park <[email protected]>

* Apply matt comments

* See what happens by adding fees

* Add genesis logic update

* Try fixing e2e

* Add some better logging to find error

* remove stat, fix typo

* fix(e2e): fix e2e test for consensus min gas fee #4244 (#4317)

* chore(e2e): update init image on main for #4244

* changes

* fix more problems

* more fixes

* lint

* twap gebug logs

* arithmetic fix

* skip twap test

* link issue

* Temp disable geo twap E2E

---------

Co-authored-by: Matt, Park <[email protected]>
Co-authored-by: Roman <[email protected]>
mergify bot pushed a commit that referenced this pull request Feb 16, 2023
* Refactor tests a bit

* Add consensus min gas fee

* Fix tests

* Delete more crud

* Cleanup more duplicate test cases

* flip isCheckTx sign

* One more test needed

* Run invalid fee denom test across both check & deliver

* Remove extra line

* Add hack to get around txfee problem in ibctesting

* Handle genesis

* Minor code cleanup

* tryfix simulation

* Fix for legacy simulator

* Update x/txfees/keeper/feedecorator_test.go

Co-authored-by: Matt, Park <[email protected]>

* Apply matt comments

* See what happens by adding fees

* Add genesis logic update

* Try fixing e2e

* Add some better logging to find error

* remove stat, fix typo

* fix(e2e): fix e2e test for consensus min gas fee #4244 (#4317)

* chore(e2e): update init image on main for #4244

* changes

* fix more problems

* more fixes

* lint

* twap gebug logs

* arithmetic fix

* skip twap test

* link issue

* Temp disable geo twap E2E

---------

Co-authored-by: Matt, Park <[email protected]>
Co-authored-by: Roman <[email protected]>
(cherry picked from commit 05fda4d)
ValarDragon added a commit that referenced this pull request Feb 16, 2023
* Refactor tests a bit

* Add consensus min gas fee

* Fix tests

* Delete more crud

* Cleanup more duplicate test cases

* flip isCheckTx sign

* One more test needed

* Run invalid fee denom test across both check & deliver

* Remove extra line

* Add hack to get around txfee problem in ibctesting

* Handle genesis

* Minor code cleanup

* tryfix simulation

* Fix for legacy simulator

* Update x/txfees/keeper/feedecorator_test.go

Co-authored-by: Matt, Park <[email protected]>

* Apply matt comments

* See what happens by adding fees

* Add genesis logic update

* Try fixing e2e

* Add some better logging to find error

* remove stat, fix typo

* fix(e2e): fix e2e test for consensus min gas fee #4244 (#4317)

* chore(e2e): update init image on main for #4244

* changes

* fix more problems

* more fixes

* lint

* twap gebug logs

* arithmetic fix

* skip twap test

* link issue

* Temp disable geo twap E2E

---------

Co-authored-by: Matt, Park <[email protected]>
Co-authored-by: Roman <[email protected]>
(cherry picked from commit 05fda4d)

Co-authored-by: Dev Ojha <[email protected]>
pysel pushed a commit that referenced this pull request Feb 18, 2023
* Refactor tests a bit

* Add consensus min gas fee

* Fix tests

* Delete more crud

* Cleanup more duplicate test cases

* flip isCheckTx sign

* One more test needed

* Run invalid fee denom test across both check & deliver

* Remove extra line

* Add hack to get around txfee problem in ibctesting

* Handle genesis

* Minor code cleanup

* tryfix simulation

* Fix for legacy simulator

* Update x/txfees/keeper/feedecorator_test.go

Co-authored-by: Matt, Park <[email protected]>

* Apply matt comments

* See what happens by adding fees

* Add genesis logic update

* Try fixing e2e

* Add some better logging to find error

* remove stat, fix typo

* fix(e2e): fix e2e test for consensus min gas fee #4244 (#4317)

* chore(e2e): update init image on main for #4244

* changes

* fix more problems

* more fixes

* lint

* twap gebug logs

* arithmetic fix

* skip twap test

* link issue

* Temp disable geo twap E2E

---------

Co-authored-by: Matt, Park <[email protected]>
Co-authored-by: Roman <[email protected]>
pysel pushed a commit that referenced this pull request Feb 21, 2023
* Refactor tests a bit

* Add consensus min gas fee

* Fix tests

* Delete more crud

* Cleanup more duplicate test cases

* flip isCheckTx sign

* One more test needed

* Run invalid fee denom test across both check & deliver

* Remove extra line

* Add hack to get around txfee problem in ibctesting

* Handle genesis

* Minor code cleanup

* tryfix simulation

* Fix for legacy simulator

* Update x/txfees/keeper/feedecorator_test.go

Co-authored-by: Matt, Park <[email protected]>

* Apply matt comments

* See what happens by adding fees

* Add genesis logic update

* Try fixing e2e

* Add some better logging to find error

* remove stat, fix typo

* fix(e2e): fix e2e test for consensus min gas fee #4244 (#4317)

* chore(e2e): update init image on main for #4244

* changes

* fix more problems

* more fixes

* lint

* twap gebug logs

* arithmetic fix

* skip twap test

* link issue

* Temp disable geo twap E2E

---------

Co-authored-by: Matt, Park <[email protected]>
Co-authored-by: Roman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v15.x backport patches to v15.x branch C:docs Improvements or additions to documentation V:state/compatible/backport State machine compatible PR, should be backported
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants