10000 More info when database is missing a node, plus related pruning bugfix by carver · Pull Request #83 · ethereum/py-trie · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

More info when database is missing a node, plus related pruning bugfix #83

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

Merged
merged 3 commits into from
Mar 18, 2019

Conversation

carver
Copy link
Contributor
@carver carver commented Mar 13, 2019

What was wrong?

When a node in the trie was missing, we:

  1. got relatively little information back about what happened
  2. would prune nodes before attempting to set/delete, so extra trie elements would disappear

How was it fixed?

  • Add a custom exception for a missing trie node
  • Include information in the exception like the root hash, the key requested, and the hash of the node missing
  • Add tests that uncovered pruning bug
  • Fix pruning bug

TODO

  • Debug what appears to be situations where pruning is incomplete
  • Remove type check from custom exception (or maybe improve the error message and add it to all values?)

Cute Animal Picture

Cute animal picture

@carver
Copy link
Contributor Author
carver commented Mar 13, 2019

Interesting, I wasn't sure circle was going to run on a draft

@carver carver changed the title Extra missing node data More info when database is missing a node, plus related pruning bugfix Mar 13, 2019
@carver carver force-pushed the extra-missing-node-data branch from 076e35a to e6ff7da Compare March 14, 2019 21:37
@carver carver marked this pull request as ready for review March 14, 2019 22:01
@carver carver requested a review from pipermerriam March 14, 2019 22:01
@carver carver mentioned this pull request Mar 15, 2019
trie/hexary.py Outdated
node_type = get_node_type(node)

# node is mutable, so capture the key for later pruning now
prune_key = self._get_prune_key(node)
Copy link
Member

Choose a reason for hiding this comment

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

This seems like potential for forgetting, what about something like:

with self._prune_node(node):
    ...  # do things that might raise errors

The pruning would only happen if the context exits without exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh, how did I miss that. Thanks!

@carver carver merged commit 4ba2b0d into ethereum:master Mar 18, 2019
@carver carver deleted the extra-missing-node-data branch March 18, 2019 19:33
Copy link
Member
@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

probably should be cleaned up.

# Prune only if no exception is raised
try:
yield
except:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need this try/except block at all (and we probably end up taking a performance hit for it).

try:
    thing()
except:
    raise
else:
    something()

# equivalent to
thing()
something()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#86

@carver carver mentioned this pull request Mar 19, 2019
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
0