Update framework use the new schema - Closes #5378 #5382
Conversation
| // Validate block header | ||
| const errors = validator.validate(blockHeaderSchema, block.header); | ||
| if (errors.length) { | ||
| throw new Error(errors[0].message); |
There was a problem hiding this comment.
On development branch introduced a new error LiskValidationError which contains an attribute errors. We should start using it in such cases.
throw new Error(errors);
There was a problem hiding this comment.
it is returning the error array, so i dont think this works
There was a problem hiding this comment.
Yes that LiskValidationError accepts array of errors. So all good to throw it. The message will be generated automatically something
Lisk validation failed and found 2 errors.
There was a problem hiding this comment.
ah you mean parse it to the error, but I think if it just return the particular error instead of asking it to handle here, it would be better.
but I do agree we should use the single error everywhere and we should fix it.
There was a problem hiding this comment.
@shuse2 Just for reference can you create an issue to remove all usages of throwing errors like throw new Error(errors[0].message) So we can fix it.
| // Validate block header | ||
| const errors = validator.validate(blockHeaderSchema, block.header); | ||
| if (errors.length) { | ||
| throw new Error(errors[0].message); |
There was a problem hiding this comment.
Yes that LiskValidationError accepts array of errors. So all good to throw it. The message will be generated automatically something
Lisk validation failed and found 2 errors.
| @@ -173,7 +175,7 @@ export class Chain { | |||
| private readonly blocksVerify: BlocksVerify; | |||
| private readonly _networkIdentifier: Buffer; | |||
| private readonly blockRewardArgs: BlockRewardOptions; | |||
There was a problem hiding this comment.
since we are refactoring, can we rename these private properties to follow _?
| @@ -30,6 +31,10 @@ const forkStatusList = [ | |||
| ForkStatus.DISCARD, | |||
| ]; | |||
|
|
|||
| export const EVENT_PROCESSOR_SYNC_REQUIRED = 'EVENT_PROCESSOR_SYNC_REQUIRED'; | |||
There was a problem hiding this comment.
PROCESSOR_SYNC_REQUIRED, at places we use the value without event_, maybe we should use one format.
framework/src/application/node/synchronizer/base_synchronizer.ts
Outdated
Show resolved
Hide resolved
What was the problem?
This PR resolves #5378
How was it solved?
How was it tested?