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

fix(crypto): validate the vendor field length #1289

Merged
merged 4 commits into from Nov 5, 2018

Conversation

Projects
None yet
3 participants
@faustbrian
Collaborator

faustbrian commented Nov 5, 2018

Proposed changes

The vendor field length was never validated.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

Checklist

  • I have read the CONTRIBUTING documentation
  • Lint and unit tests pass locally with my changes

@wafflebot wafflebot bot added the review label Nov 5, 2018

@codecov-io

This comment has been minimized.

codecov-io commented Nov 5, 2018

Codecov Report

Merging #1289 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1289   +/-   ##
========================================
  Coverage    75.08%   75.08%           
========================================
  Files          416      416           
  Lines         7272     7272           
  Branches       988      988           
========================================
  Hits          5460     5460           
  Misses        1596     1596           
  Partials       216      216
Impacted Files Coverage Δ
...b/validation/rules/models/transactions/transfer.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7da86c...7f1fca0. Read the comment docs.

@faustbrian

This comment has been minimized.

Collaborator

faustbrian commented Nov 5, 2018

Testing sync from 0 at the moment as there are already some transactions that have a vendor field length above 64 characters so we might need to add exceptions.

@faustbrian faustbrian changed the title from fix(crypto): validate the vendor field length to [WIP] fix(crypto): validate the vendor field length Nov 5, 2018

faustbrian added some commits Nov 5, 2018

@faustbrian faustbrian changed the title from [WIP] fix(crypto): validate the vendor field length to fix(crypto): validate the vendor field length Nov 5, 2018

@faustbrian faustbrian requested a review from alexbarnsley Nov 5, 2018

@faustbrian

This comment has been minimized.

Collaborator

faustbrian commented Nov 5, 2018

Added exceptions for devnet, mainnet should be fine according to this query on a v1 database.

ark_mainnet=> SELECT * FROM transactions WHERE length("vendorField") > 64;
 id | rowId | blockId | type | timestamp | senderPublicKey | senderId | recipientId | amount | fee | signature | signSignature | requesterPublicKey | vendorField | signatures | rawasset
----+-------+---------+------+-----------+-----------------+----------+-------------+--------+-----+-----------+---------------+--------------------+-------------+------------+----------
(0 rows)

@faustbrian faustbrian merged commit ab9553d into develop Nov 5, 2018

1 check passed

ci/circleci: test-node10 Your tests passed on CircleCI!
Details

@wafflebot wafflebot bot removed the review label Nov 5, 2018

@faustbrian faustbrian deleted the validation branch Nov 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment