Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Add validation of custom transaction - Closes #3877 #3880

Merged
merged 13 commits into from
Jul 9, 2019
Merged

Add validation of custom transaction - Closes #3877 #3880

merged 13 commits into from
Jul 9, 2019

Conversation

shuse2
Copy link
Collaborator

@shuse2 shuse2 commented Jun 27, 2019

What was the problem?

Registering custom transaction did not have validation.

How did I fix it?

Adding validation for

  • The first prototype chain is BaseTransaction after generic Function
  • It has at least satisfies the public base transaction methods and properties

How to test it?

From SDK, use Application but use lisk-transactions/BaseTransaction and try to register
or
npm t

Review checklist

@shuse2 shuse2 self-assigned this Jun 27, 2019
@shuse2 shuse2 marked this pull request as ready for review June 27, 2019 16:20
@shuse2 shuse2 requested a review from lsilvs June 28, 2019 09:48
@shuse2 shuse2 requested a review from nazarhussain June 28, 2019 11:48
@shuse2 shuse2 requested a review from nazarhussain June 28, 2019 15:29
lsilvs
lsilvs approved these changes Jul 1, 2019
View reviewed changes
Copy link
Contributor

@lsilvs lsilvs left a comment

Choose a reason for hiding this comment

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

Just a minor typo in the test description. Other than that it looks good :)

framework/test/jest/unit/specs/controller/application.spec.js Outdated Show resolved Hide resolved
@shuse2 shuse2 mentioned this pull request Jul 2, 2019
Merged
@shuse2 shuse2 requested a review from nazarhussain July 4, 2019 06:54
@shuse2
Copy link
Collaborator Author

shuse2 commented Jul 8, 2019

Further discussion is moved to #3935

@shuse2 shuse2 dismissed nazarhussain’s stale review July 8, 2019 15:29

Further improvements are moved to another issue

@shuse2 shuse2 merged commit 45685c8 into release/2.1.0 Jul 9, 2019
@shuse2 shuse2 deleted the 3877-update_validation_of_custom_tx branch July 9, 2019 08:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mitsuaki-u mitsuaki-u mitsuaki-u approved these changes

@lsilvs lsilvs lsilvs approved these changes

@nazarhussain nazarhussain Awaiting requested review from nazarhussain

Assignees

@shuse2 shuse2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shuse2 @nazarhussain @lsilvs @mitsuaki-u