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

chore(strv2): Rename static precompiles #2365

Merged

Conversation

ramacarlucho
Copy link
Contributor

Description

Rename evm params. active_precompiles -> active_static_precompiles

Added migration logic, fix tests.
Registered migration on module.go

Closes: XAP-115


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the correct branch (see PR Targeting)

Reviewers Checklist

All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.

I have...

  • added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • confirmed all author checklist items have been addressed
  • confirmed that this PR does not change production code
  • reviewed content
  • tested instructions (if applicable)
  • confirmed all CI checks have passed

@ramacarlucho ramacarlucho requested a review from a team as a code owner February 19, 2024 22:51
@ramacarlucho ramacarlucho requested review from hanchon and GAtom22 and removed request for a team February 19, 2024 22:51
Copy link

linear bot commented Feb 19, 2024

Copy link
Contributor

@Vvaradinov Vvaradinov left a comment

Choose a reason for hiding this comment

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

Great work @ramacarlucho, left some comments. We should rename the migration folder so it follows the ordering of migrations instead of the version of Evmos. Also please add missing license header to the migrate file.

}
params.AllowUnprotectedTxs = paramsV16.AllowUnprotectedTxs
params.ActiveStaticPrecompiles = paramsV16.ActivePrecompiles
params.EVMChannels = types.DefaultEVMChannels
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be taken directly from the old params instead?

x/evm/module.go Outdated
Comment on lines 148 to 149
if err := cfg.RegisterMigration(types.ModuleName, 16, m.Migrate16to17); err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This should be the versioning of the migrations instead of the overall Evmos version.

cdc codec.BinaryCodec,
) error {
var (
paramsV16 v16types.V16Params
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This should be the migration version as oppose to the Evmos version.

x/evm/migrations/v6/migrate.go Outdated Show resolved Hide resolved
@ramacarlucho ramacarlucho merged commit a901b7f into facs95/dynamic-precompiles Feb 20, 2024
30 of 31 checks passed
@ramacarlucho ramacarlucho deleted the rama/rename-static-precompiles branch February 20, 2024 15:21
Vvaradinov added a commit that referenced this pull request Feb 21, 2024
* add new param

* full implementation

* load abi correctly

* update contract instance

* run make format

* rename function

* add mocks

* run make format

* make erc20keeper a pointer on app.go

* fix typo

* fix EnableDynamicPrecompiles

* improve error

* add comment to app.go

* run make format

* review comments

* remove data folders

* run make format

* refractor state_transition

* fix lint

* run make format

* update comments

* remove unused functions

* wip: fixing dynamic precompiles. Re-writing integration-tests for werc20 (#2343)

* wip: fixing dynamic precompiles. Re-writing integration-tests for werc20

* run make format

* address some linter warnings

* remove unnecessary werc20 event constants

* remove duplicate import

* revert adding args to deposit and withdraw functions

* fix some missing imports

* WIP test refactors

* WIP more test refactors

* re-add wevmos precompile to static precompiles

* use base denom for wevmos token pair

* more WIP

* more WIP

* run make format

* remove unnecessary level of nesting

* run gofumpt

* fix tests

* remove unused utils and move test runner into integration test file

* remove mentions of deposit and withdraw events in tx.go

* add some todo comments

* remove unnecessary print statement

* remove unnecessary migration test flows that now live in the v17 upgrades folder

* remove unnecessary ordered instruction for integration tests

* only use two keys for the integration tests

* address linters

---------

Co-authored-by: ramacarlucho <ramacarlucho@users.noreply.github.com>
Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>
Co-authored-by: MalteHerrmann <MalteHerrmann@users.noreply.github.com>

* fix regression

* imp(evm): Check for dynamic extensions during EVM initialization (#2356)

check for active dynamic precompiles as well as static ones in HasCustomPrecompiles

* imp(werc20): Remove WERC-20 precompile and register WEVMOS as standard ERC-20 precompile (#2352)

* move WEVMOS contract to contracts directory

* remove werc20 precompile

* add utility to get token pair from grpc handler

* adjust integration tests and upgrade logic to account for WEVMOS being registered as an ERC-20 token pair and precompile

* run make format

* add changelog entry

* make addNewTokenPair public

* move RegisterEVMExtensions util to v17 migration logic

* remove unused methods

* run gofumpt

* Update app/upgrades/v17/migration.go

* address review comments

* remove duplicate params checks in tests

* update RegisterERC20Extensions upgrade handler

* update NewTokenPair function

* remove unnecessary param

* update mocks

* run make format

* delete unnecessary keepers from migration logic

* fix test

* run make format

* dont use new function to register aevmos token pair

* remove unused keepers in migration logic

* remove incorrect test

* fix test for p256

---------

Co-authored-by: MalteHerrmann <MalteHerrmann@users.noreply.github.com>
Co-authored-by: Freddy Caceres <facs95@gmail.com>
Co-authored-by: facs95 <facs95@users.noreply.github.com>
Co-authored-by: Ramiro Carlucho <ramirocarlucho@gmail.com>
Co-authored-by: ramacarlucho <ramacarlucho@users.noreply.github.com>

* add wevmos pair to local node

* tests(strv2): Add unit tests for dynamic precompiles (#2366)

* add unit tests

* run make format

* test expected panic

* run make format

* check errors

---------

Co-authored-by: ramacarlucho <ramacarlucho@users.noreply.github.com>

* chore(strv2): Rename static precompiles (#2365)

* rename static precompiles

* rename upgrade

* read evmchannels from params

* fix old migration

* fix lint

* not break protobuf

* fix tests

---------

Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
Co-authored-by: Freddy Caceres <facs95@gmail.com>

* remove unnecessary function

* remove unnecessary import

* make enable dynamic precompiles more efficient

* run make format

* fix old register proto

* address pr comments

---------

Co-authored-by: facs95 <facs95@users.noreply.github.com>
Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: Ramiro Carlucho <ramirocarlucho@gmail.com>
Co-authored-by: ramacarlucho <ramacarlucho@users.noreply.github.com>
Co-authored-by: MalteHerrmann <malte@evmos.org>
Co-authored-by: MalteHerrmann <MalteHerrmann@users.noreply.github.com>
Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants