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

Blockchain: Deprecation Tasks / Head Function Renamings / Iterator adjustments #1754

Closed
holgerd77 opened this issue Feb 28, 2022 · 5 comments

Comments

@holgerd77
Copy link
Member

Part of #1717

The following tasks are to be done along deprecation for the Blockchain package:

  • The meta property can be removed
  • The deprecated getHead() function can be removed (this is not the canonical head but the DB iterator head, see associated code docs of the function itself and the replacement function getIteratorHead() (above))
  • Same with deprecated setHead() function

I would be a fan of the following renamings (maybe execute upon if no one objects here during the day or so):

  • getLatestHeader() -> getCanonicalHeadHeader()
  • getLatestBlock() -> getCanonicalHeadBlock()

This is to make this more expressive and semantically clear and better differentiate on the iterator head functions.

@holgerd77 holgerd77 changed the title Blockchain: Deprecation Tasks / Head Function Renamings Blockchain: Deprecation Tasks / Head Function Renamings / Iterator adjustments Feb 28, 2022
@holgerd77
Copy link
Member Author

Ah, and another deprecation task, will also put here since not too big: the iterator() function was expanded at some point (for the client) to not return void (so: give no feedback) but always return the number of blocks being ran.

The blockchain implementation is already reflecting this. The interface though still has void in it. This should be removed. Also the VM.runBlockchain() function still forwards (returns) this void which can also be removed along. (the client is also using blockchain.iterator() along execution but I guess that's fine? Might need some short confirmation though.)

@holgerd77
Copy link
Member Author

Ah, and another deprecation task, will also put here since not too big: the iterator() function was expanded at some point (for the client) to not return void (so: give no feedback) but always return the number of blocks being ran.

The blockchain implementation is already reflecting this. The interface though still has void in it. This should be removed. Also the VM.runBlockchain() function still forwards (returns) this void which can also be removed along. (the client is also using blockchain.iterator() along execution but I guess that's fine? Might need some short confirmation though.)

Ah, this additional point hasn't been tackled yet, right? @ScottyPoi do you also want to take this over (it's a really really small one, maybe you could do with the test re-additions), since you have also tackled the other stuff?

(remember to delete your local develop branch and re-pull to have a fresh copy to branch off 🙂)

@holgerd77
Copy link
Member Author

Here is a small sub-task not being tackled yet (see last comment) if I am not mistaken, just a short reminder that this won't get forgotten.

@ScottyPoi
Copy link
Contributor

ah, i took care of this a couple weeks back. must not have made it to a PR.

@holgerd77
Copy link
Member Author

Closed by #1822 and #1877

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.

2 participants