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

core/state/snapshot, cmd/geth: polish code #37

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

rjl493456442
Copy link

No description provided.

@rjl493456442 rjl493456442 requested a review from holiman as a code owner June 2, 2022 03:34
Comment on lines +87 to +90
if !errors.Is(err, errNoJournal) {
log.Info("Failed to resolve snapshot journal", "err", err)
return err
}
Copy link
Owner

Choose a reason for hiding this comment

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

This can't be right. But changing it to if errors.Is won't make it correct either, I'm not quite sure why the distinction between errors is needed here?

Copy link
Owner

Choose a reason for hiding this comment

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

I'll merge it and then fix it

Copy link
Owner

Choose a reason for hiding this comment

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

About this -- is there any reason to not just do

diff --git a/core/state/snapshot/utils.go b/core/state/snapshot/utils.go
index 505ad15a99..b88ea05484 100644
--- a/core/state/snapshot/utils.go
+++ b/core/state/snapshot/utils.go
@@ -84,7 +84,7 @@ func checkDanglingMemStorage(db ethdb.KeyValueStore) error {
 		}
 		return nil
 	})
-	if !errors.Is(err, errNoJournal) {
+	if err != nil {
 		log.Info("Failed to resolve snapshot journal", "err", err)
 		return err
 	}

@holiman holiman merged commit 633d198 into holiman:dangling_3 Jun 2, 2022
holiman added a commit that referenced this pull request Jun 7, 2022
…unt-check (ethereum#24765)

* cmd/geth, core/state/snapshot: rework journal loading, implement account-check

* core/state/snapshot, cmd/geth: polish code (#37)

* core/state/snapshot: minor nits

* core/state/snapshot: simplify error logic

* cmd/geth: go format

Co-authored-by: rjl493456442 <garyrong0905@gmail.com>
holiman pushed a commit that referenced this pull request Nov 26, 2022
holiman pushed a commit that referenced this pull request Jul 7, 2023
* Add breadcrumbs to docs

* change BreadcrumbLink to NextLink

* Update src/components/docs/Breadcrumbs.tsx

Co-authored-by: Paul Wackerow <54227730+wackerow@users.noreply.github.com>

* Update src/components/docs/Breadcrumbs.tsx

Co-authored-by: Paul Wackerow <54227730+wackerow@users.noreply.github.com>

Co-authored-by: Paul Wackerow <54227730+wackerow@users.noreply.github.com>
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