Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Granular validation #121

Merged
merged 6 commits into from
Aug 1, 2019
Merged

Granular validation #121

merged 6 commits into from
Aug 1, 2019

Conversation

alcuadrado
Copy link
Member

This PR closes #120

I tried adding a test for this at the end of the huge async.series call, but I can't get it run. It's ignored by tape. Any idea?

@alcuadrado
Copy link
Member Author

@holgerd77 I updated this PR with the following changes:

  1. Add a @deprecated tag in Blockchain's and BlockchainOptions' validate field.
  2. Throw in the Blockchain constructor if both validate and validatePow/validateBlocks are used.
  3. Always set Blockchain#validate as true and ignore its value.
  4. Update the docs.

@holgerd77
Copy link
Member

@alcuadrado Ahm, you still have to set the validate option if it is used, otherwise it is not a deprecation but a removal, or am I getting something wrong? 😛

@holgerd77
Copy link
Member

Update: ah, I am getting something wrong, just seeing that you are directly assigning this to the specific valdation parameters. 😄

src/index.ts Outdated Show resolved Hide resolved
@alcuadrado
Copy link
Member Author

There was a bug on how validate and the default values were handled. I fixed it and updated a test to be compatible with the changes introduced by this PR.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Looks good.

@holgerd77 holgerd77 merged commit 885d251 into master Aug 1, 2019
@holgerd77 holgerd77 deleted the granular-validation branch August 1, 2019 20:34
@s1na s1na mentioned this pull request Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BlockchainOptions#validate is too coarse
2 participants