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

Declare fields and methods starting with _ as private #690

Closed
alcuadrado opened this issue Apr 23, 2019 · 4 comments · Fixed by #930
Closed

Declare fields and methods starting with _ as private #690

alcuadrado opened this issue Apr 23, 2019 · 4 comments · Fixed by #930

Comments

@alcuadrado
Copy link
Member

This was tried in ethereumjs/ethereumjs-blockchain#98, but it breaks some tests. We should fix those to be able to declare private fields/methods properly.

Once that's done, their @hidden tsdoc tags should be removed.

@alcuadrado
Copy link
Member Author

I tried to fix this, but I'm not familiar enough with this library to make the required changes.

In particular, the logic around Blockchain.prototype._heads confusing and pretty undocumented.

If someone could point me to other projects consuming this library I may be able to document it and fix this.

@holgerd77
Copy link
Member

@vpulim Do you have got some hints here?

@vpulim
Copy link
Contributor

vpulim commented Apr 24, 2019

heads and the iterator function were around before I re-factored things to support Geth DBs. As far as I can tell, the purpose of Blockchain.prototype._heads is to keep track of and persist to disk the various block hashes used by the Blockchain.prototype.iterator() method. There is a separate hash for each unique name that is passed into the iterator() call.

This may be a good time to propose redo-ing the entire Blockchain class from scratch (if someone has the time). The legacy architecture is very hard to understand and makes it hard for contributors to make changes. This is probably the result of trying to have the Blockchain class do too many things. It should probably be split into multiple classes as @s1na proposed a while ago.

@evertonfraga evertonfraga transferred this issue from ethereumjs/ethereumjs-blockchain Apr 6, 2020
@jochem-brouwer
Copy link
Member

jochem-brouwer commented Oct 23, 2020

Linking #895

EDIT: this PR has now moved to #930

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants