Skip to content
This repository was archived by the owner on Nov 30, 2021. It is now read-only.

fix panic when start a node with snapshot#673

Merged
fedekunze merged 11 commits intocosmos:developmentfrom
KamiD:fix-panic
Jan 7, 2021
Merged

fix panic when start a node with snapshot#673
fedekunze merged 11 commits intocosmos:developmentfrom
KamiD:fix-panic

Conversation

@KamiD
Copy link

@KamiD KamiD commented Dec 25, 2020

Closes: #672

Description

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@KamiD KamiD requested review from fedekunze and noot as code owners December 25, 2020 05:46
fedekunze
fedekunze reviewed Dec 29, 2020
View reviewed changes
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

A better alternative is to update the evm.NewKeeper constructor to return a pointer of the keeper and pass that to the app and the module manager.

@KamiD
Copy link
Author

KamiD commented Dec 30, 2020

A better alternative is to update the evm.NewKeeper constructor to return a pointer of the keeper and pass that to the app and the module manager.

Agree with you

@KamiD KamiD requested a review from fedekunze December 30, 2020 02:43
@KamiD
Copy link
Author

KamiD commented Jan 5, 2021

A better alternative is to update the evm.NewKeeper constructor to return a pointer of the keeper and pass that to the app and the module manager.

@fedekunze I have finished the change by your point

fedekunze
fedekunze approved these changes Jan 5, 2021
View reviewed changes
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add a changelog bugfix entry?

@KamiD
Copy link
Author

KamiD commented Jan 5, 2021

LGTM. Can you add a changelog bugfix entry?

Done

@fedekunze
Copy link
Contributor

fedekunze commented Jan 5, 2021

Seems that test-importer is failing

@KamiD
Copy link
Author

KamiD commented Jan 6, 2021

Seems that test-importer is failing

test-importer has been fixed but run test action failed because of github throw an internal error

@KamiD
Copy link
Author

KamiD commented Jan 6, 2021

Seems that test-importer is failing

Done

@KamiD KamiD requested a review from fedekunze January 7, 2021 08:55
@fedekunze fedekunze merged commit ffbb207 into cosmos:development Jan 7, 2021
@zhongqiuwood zhongqiuwood mentioned this pull request Jan 13, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@noot noot Awaiting requested review from noot

@fedekunze fedekunze Awaiting requested review from fedekunze

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

panic "wrong Block.Header.AppHash" happened when restart a node with snapshot

2 participants

@KamiD @fedekunze