Skip to content
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

Record file hash mismatch #159

Merged
merged 24 commits into from
Sep 2, 2019
Merged

Record file hash mismatch #159

merged 24 commits into from
Sep 2, 2019

Conversation

gregscullard
Copy link
Contributor

Detailed description:
This fixes a number of issues related to record file hash mismatches
-When downloading and parsing files in parallel, the two processes would overlap on files in /valid and a parsed file would move from /valid to /processed resulting in a gap during hash sequence validation in the downloader

Which issue(s) this PR fixes:
Fixes #157

Checklist

  • Documentation added
  • Tested against version 2 record files.

@gregscullard gregscullard added bug Type: Something isn't working OABlocker labels Aug 30, 2019
@gregscullard gregscullard added this to the 0.1.0 milestone Aug 30, 2019
src/test/java/com/hedera/UtilityTest.java Outdated Show resolved Hide resolved
src/main/java/com/hedera/utilities/Utility.java Outdated Show resolved Hide resolved
src/main/java/com/hedera/utilities/Utility.java Outdated Show resolved Hide resolved
src/main/java/com/hedera/utilities/Utility.java Outdated Show resolved Hide resolved
src/main/java/com/hedera/utilities/Utility.java Outdated Show resolved Hide resolved
src/main/java/com/hedera/parser/RecordFileParser.java Outdated Show resolved Hide resolved
Greg Scullard and others added 13 commits August 30, 2019 17:27
…type not known

Signed-off-by: Greg Scullard <gregscullard@hedera.com>
…sed by mirror node)

Signed-off-by: Greg Scullard <gregscullard@hedera.com>
…efore being moved to /valid

Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Greg Scullard and others added 2 commits August 30, 2019 18:08
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
…type not known

Ensures downloaded files are validated for previous hash continuity before being moved to /valid
modify the way to calculate RecordStream File Hash; Add a test; #157
Use the newer file hash calculator for recordStream files
PR Comments

Signed-off-by: Mike Burrage <mike.burrage@hedera.com>
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

Found a bug plus some logging enhancements

src/main/java/com/hedera/utilities/Utility.java Outdated Show resolved Hide resolved
src/main/java/com/hedera/utilities/Utility.java Outdated Show resolved Hide resolved
@steven-sheehy steven-sheehy changed the title 157 file hash mismatch Record file hash mismatch Aug 30, 2019
@steven-sheehy
Copy link
Member

My PR #170 added record download tests, but the test records are probably the old format. Hopefully the tests still pass and we can add some v2 test data in a later PR.

Greg Scullard added 2 commits September 1, 2019 22:04
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
@steven-sheehy
Copy link
Member

Other than still needing to calculate the event hash with the correct method this LGTM

Greg Scullard added 2 commits September 2, 2019 10:55
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
@gregscullard
Copy link
Contributor Author

Other than still needing to calculate the event hash with the correct method this LGTM

Done, also removed now obsolete getRecordFileHash

Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

Also remove the src/test/resources/*.rcd and sig files that were added and not used.

@@ -62,8 +62,9 @@

private static final Long SCALAR = 1_000_000_000L;

// private static final byte TYPE_PREV_HASH = 1; // next 48 bytes are hash384 of previous files
// private static final byte TYPE_RECORD = 2; // next data type is transaction and its record
static final int RECORD_FORMAT_VERSION = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Remove these added static variables as they're not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Greg Scullard added 2 commits September 2, 2019 17:26
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Signed-off-by: Greg Scullard <gregscullard@hedera.com>
@gregscullard
Copy link
Contributor Author

Also remove the src/test/resources/*.rcd and sig files that were added and not used.
Done

Signed-off-by: Greg Scullard <gregscullard@hedera.com>
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

LGTM

@gregscullard gregscullard merged commit e7d1bb0 into master Sep 2, 2019
@delete-merged-branch delete-merged-branch bot deleted the 157-file-hash-mismatch branch September 2, 2019 15:33
@steven-sheehy steven-sheehy added P1 parser Area: File parsing labels Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Type: Something isn't working P1 parser Area: File parsing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Record file hash mismatch
4 participants