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.

Sign up for GitHub

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

Jump to bottom

Feat: Upgrade SDK 47 and upstream x/evm and x/feemarket module from evmos #144

Open
wants to merge 91 commits into
base: main
Choose a base branch
from
Open

Feat: Upgrade SDK 47 and upstream x/evm and x/feemarket module from evmos #144

wants to merge 91 commits into from

Conversation

GNaD13
Copy link
Contributor

@GNaD13 GNaD13 commented Apr 28, 2024 •
edited
Loading

Close #143

@GNaD13 GNaD13 requested a review from jiujiteiro as a code owner April 28, 2024 07:04
@GNaD13 GNaD13 marked this pull request as draft April 28, 2024 07:04
@GNaD13 GNaD13 mentioned this pull request Apr 28, 2024
Open
jiujiteiro
jiujiteiro requested changes Apr 30, 2024
View reviewed changes
Copy link
Collaborator

@jiujiteiro jiujiteiro left a comment

Choose a reason for hiding this comment

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

left a few comments. Can you also make sure the lint job is passing?

CHANGELOG.md Outdated Show resolved Hide resolved
third_party/proto/ethermint/evm/v1/query.proto Outdated
@@ -8,7 +8,7 @@ import "ethermint/evm/v1/evm.proto";
import "ethermint/evm/v1/tx.proto";
import "google/protobuf/timestamp.proto";

option go_package = "github.com/evmos/ethermint/x/evm/types";
option go_package = "github.com/evmos/evmos/v18/x/evm/types";
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should double check that the proto files match the evmos ones. For example

https://github.com/evmos/evmos/blob/v18.0.0/proto/ethermint/evm/v1/query.proto

there are some differences. the rpc query url for example. there is no /etheremint in them.

third_party/proto/ethermint/evm/v1/evm.proto Outdated Show resolved Hide resolved
@GNaD13
Copy link
Contributor Author

GNaD13 commented Apr 30, 2024

left a few comments. Can you also make sure the lint job is passing?

Yeah, this PR is still on a draft. We will do all the linting after done with the code

neitdung and others added 24 commits May 6, 2024 14:45
@jiujiteiro
Copy link
Collaborator

jiujiteiro commented May 24, 2024 •
edited
Loading

@GNaD13 @neitdung once we change the upgrade from v3 to v2, can we add "v2" to the module path in the go.mod?

that way, all the imports will have /realiotech/realio-network/v2.....

this is a pattern to follow on every upgrade for versioning.

@GNaD13
Copy link
Contributor Author

GNaD13 commented May 24, 2024

@GNaD13 @neitdung once we change the upgrade from v3 to v2, can we add "v2" to the module path in the go.mod?

that way, all the imports will have /realiotech/realio-network/v2.....

this is a pattern to follow on every upgrade for versioning.

yeah, we got it

jiujiteiro
jiujiteiro reviewed Jun 14, 2024
View reviewed changes
Copy link
Collaborator

@jiujiteiro jiujiteiro left a comment

Choose a reason for hiding this comment

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

small comments. looks ready pending testing

app/upgrades/v2/upgrades.go Outdated Show resolved Hide resolved
scripts/protoc-swagger-gen.sh Outdated Show resolved Hide resolved
@GNaD13
Copy link
Contributor Author

GNaD13 commented Jun 15, 2024

Some issue with ante handler, we need to waiting for this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neitdung neitdung neitdung left review comments

@jiujiteiro jiujiteiro jiujiteiro requested changes

@haleyga haleyga Awaiting requested review from haleyga

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feat: Upstream x/evm module from evmos
3 participants
@GNaD13 @jiujiteiro @neitdung