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

Nd unify legacy evm subspace usage #22

Merged
merged 13 commits into from
May 4, 2023

Conversation

nddeluca
Copy link
Member

@nddeluca nddeluca commented May 3, 2023

No description provided.

nddeluca added 11 commits May 3, 2023 14:59
prevent panics when using the feemarket keeper with a vanilla subspace
addition to ensuring that the migration stays compatible when a subpsace
is shared between the module (and thus the migration) and the keeper
which is a common setup in app.go
value is not fetched and converted into new params structure
converting them to new params;  this fixes key table incompatibility
with the migration and a bug where the merge split block renamed value
is not set from the historical state
keeper to simplify logic; avoid additional registration of types on
proto codec; remove unused v2 migration code; and avoid copying of
EIP712 data in parameters during conversion by using the same concrete
type
…keys

and amino codec through keeper to migrator.  Both keeper and migrator
are tested to ensure proper key table registraion that does not conflict
and to ensure historical params are fetchable
@nddeluca nddeluca requested review from DracoLi, galxy25 and drklee3 May 3, 2023 23:22
Copy link
Member

@drklee3 drklee3 left a comment

Choose a reason for hiding this comment

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

lgtm, updated tests look good

@nddeluca nddeluca merged commit 8bbf86e into kava-release-v0.21.x May 4, 2023
DracoLi pushed a commit that referenced this pull request Jun 18, 2023
* enable registration of feemarket legacy param key table registration to
prevent panics when using the feemarket keeper with a vanilla subspace

* touch up some comments for clarity, remove unused line

* add some tests that enforce keeper registration of the key table in
addition to ensuring that the migration stays compatible when a subpsace
is shared between the module (and thus the migration) and the keeper
which is a common setup in app.go

* add test for current bug in legacy merge fork block fetching where
value is not fetched and converted into new params structure

* get tests passing by using true legacy parameters in keeper and
converting them to new params;  this fixes key table incompatibility
with the migration and a bug where the merge split block renamed value
is not set from the historical state

* refactor legacy param handling in migration (non-state breaking) and in
keeper to simplify logic; avoid additional registration of types on
proto codec; remove unused v2 migration code; and avoid copying of
EIP712 data in parameters during conversion by using the same concrete
type

* remove dead code

* add test to ensure nil params do not panic for legacy historical queries

* fix test error from rebasing onto migration paramstore refactor

* remove oboselete test now that store migrations use an indepent subpsace
and it is not passed as an argument anymore

* refactor to use a shared legacy param store to avoid injecting param keys
and amino codec through keeper to migrator.  Both keeper and migrator
are tested to ensure proper key table registraion that does not conflict
and to ensure historical params are fetchable

* add additional test to ensure store registors it's own key table when
needed

* remove unused private member
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 this pull request may close these issues.

2 participants