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

feat : upgraded the app with evm module to SDK v0.50 #1036

Merged
merged 28 commits into from
Dec 7, 2023

Conversation

vishal-kanna
Copy link
Contributor

No description provided.

Base automatically changed from sdk-changes to v0.50.0-upgrade November 22, 2023 13:25
app/app.go Outdated Show resolved Hide resolved
x/evm/genesis_test.go Outdated Show resolved Hide resolved
x/evm/keeper/attest.go Outdated Show resolved Hide resolved
x/evm/keeper/attest.go Outdated Show resolved Hide resolved
x/evm/keeper/attest.go Outdated Show resolved Hide resolved
x/evm/keeper/attest.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/valset/module.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
@aleem1314
Copy link
Contributor

aleem1314 commented Nov 24, 2023

Here are few things we have to do after fixing errors in each module

  • Params are deprecated. Move params to module state Ref.
  • Update module to use collections
  • Add AutoCLI support

x/evm/keeper/attest.go Outdated Show resolved Hide resolved
@aleem1314
Copy link
Contributor

Update this branch with v0.50.0-upgrade and revert all logger changes

x/evm/types/params.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
app/app.go Outdated Show resolved Hide resolved
Copy link

@akhilkumarpilli akhilkumarpilli left a comment

Choose a reason for hiding this comment

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

Still there are few things need to be updated:

  • Add signer field proto annotation, remove GetSigners and GetSignBytes implementations in every sdk.Msg defined in this module.
  • Add EndBlocker, BeginBlocker and other compiler assertions (if required any) in module.go.
  • Update Endblocker method structure to new SDK defined type if there are not validator updates involved in it.
  • Update BeginBlocker method structure.

Please go through migration docs much deeper and update any if still not updated.

app/app.go Outdated
Comment on lines 590 to 591
nil,
nil,

Choose a reason for hiding this comment

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

Why these values are nil? Remove these fields if not needed or add TODO comment if they need to be updated later.

@byte-bandit
Copy link
Contributor

Still there are few things need to be updated:

  • Add signer field proto annotation, remove GetSigners and GetSignBytes implementations in every sdk.Msg defined in this module.

Warning

Paloma is using a custom signer function using an embedded Metadata field on every message. There's a custom Ante handler responsible for parsing and verifying signatures on messages. This should stay as is and not be touched. GetSigners and GetSignBytes should be handled by libmeta on every message, but they need to stay implemented for this to work.

app/app.go Outdated Show resolved Hide resolved
@aleem1314 aleem1314 merged commit 8a60bd6 into v0.50.0-upgrade Dec 7, 2023
0 of 2 checks passed
@aleem1314 aleem1314 deleted the evm-changes branch December 7, 2023 09:09
Comment on lines +70 to +71
sdkCtx := sdk.UnwrapSDKContext(ctx)
logger := k.Logger(sdkCtx).WithFields(
Copy link
Contributor

Choose a reason for hiding this comment

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

@aleem1314 @vishal-kanna

Please start using liblog.FromSDKLogger(k.Logger(SdkCtx) in the future. There are still ocurrances here without.

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.

6 participants