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

Separate creation and loading of BlockChain<T> #2585

Closed
greymistcube opened this issue Nov 27, 2022 · 0 comments
Closed

Separate creation and loading of BlockChain<T> #2585

greymistcube opened this issue Nov 27, 2022 · 0 comments

Comments

@greymistcube
Copy link
Contributor

Related to #1486.

All BlockChain<T> instance creation gets to a private constructor. As a design choice, this in and of itself isn't bad per se, but I think many problems arise from trying to combine two separate use cases into one. The act of creating a new BlockChain<T> from scratch and loading a BlockChain<T> from a stored database are very different. There are some sanity checks in place but still it isn't obvious what the operating conditions of a working BlockChain<T> instance are, as a single constructor tries to juggle many different cases in one place. Some thoughts:

  • BlockChain<T> requires a genesis Block<T> to be created beforehand. I'd say this is inconsistent with overall design/assumptions and muddies whether IStore should take precedence over BlockChain<T> or it should be the other way around.
    • This is made harder with IBlockPolicy<T>. Unlike the name suggests, this is a mix of "block policy" (where the validation is done for a single Block<T> without any context) and "chain policy" (where the validation is done under a specific BlockChain<T> context).
    • There is no way to prevent IStore having Block<T>s that are inconsistent with IBlockPolicy<T>. In my opinion, this generally suggests that IStore should take precedence over BlockChain<T>.
    • On the other hand, the current implementation conditionally put BlockChain<T> before IStore, as BlockChain<T> manipulates IStore during construction when an empty IStore is passed as an argument.
      • Going back to the very first point, IStore itself has sufficient public API to "overwrite" a canonical genesis Block<T> by putting a new genesis Block<T> and changing the canonical chain id. Although this would be less expected behavior for BlockChain<T>() constructor when inconsistent IStore is passed in as an argument, at least for me, it isn't entirely unimaginable given the shape of the API.

I'd suggest introducing a separate factory method Create() for this purpose.

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

No branches or pull requests

2 participants