Skip to content
This repository has been archived by the owner on Jan 4, 2022. It is now read-only.

refactor!: split validation logic into basic, signature, fee and state #331

Merged
merged 3 commits into from
Aug 11, 2021

Conversation

shumkov
Copy link
Member

@shumkov shumkov commented Aug 8, 2021

Issue being fixed or feature implemented

In order to implement error codes for consensus errors, ranged by validation types, we need to refactor validation logic and separate basic, signature, fee, and state validation into separate functions.

What was done?

  • Renamed structure validation to basic validation
  • Moved signature validation from basic to a standalone method
  • Renamed data validation to state validation
  • Aligned project directories structure
  • stateTransition#validate accept validation options
  • Moved partial compound indices validation from state to basic function
  • validateIdentityExistence store fetched Identity in validation result

How Has This Been Tested?

With tests.

Breaking Changes

  • stateTransition.validateStructure renamed to stateTransition.validateBasic and doesn't perfom signature validation
  • stateTransition.validateData renamed to stateTransition.validateState

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lgtm-com
Copy link

lgtm-com bot commented Aug 8, 2021

This pull request introduces 1 alert when merging f9e5d2d into d804607 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@shumkov shumkov changed the title refactor: split validation logic into basic, signature, fee and state refactor!: split validation logic into basic, signature, fee and state Aug 8, 2021
@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 2 alerts when merging 9cc0e1b into d804607 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@shumkov shumkov requested review from shuplenkov, jawid-h and antouhou and removed request for shuplenkov August 9, 2021 07:10
@shumkov shumkov marked this pull request as ready for review August 9, 2021 07:10
@shumkov shumkov added this to the v0.21.0 milestone Aug 9, 2021
* @return {Promise<ValidationResult>}
*/
// eslint-disable-next-line no-unused-vars
async function validateIdentityTopUpTransitionState(stateTransition) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to keep stateTransition here?

Copy link
Member Author

@shumkov shumkov Aug 9, 2021

Choose a reason for hiding this comment

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

Just to keep the interface since we still passing the argument here.

Copy link
Contributor

@antouhou antouhou left a comment

Choose a reason for hiding this comment

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

Nothing I can spot with a naked eye

@shumkov shumkov merged commit a73f1bb into v0.21-dev Aug 11, 2021
@shumkov shumkov deleted the refactor-validation branch August 11, 2021 07:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants