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

Validate created #34

Merged
merged 26 commits into from
Sep 5, 2024
Merged

Validate created #34

merged 26 commits into from
Sep 5, 2024

Conversation

aljones15
Copy link
Contributor

Adds a new minor feature created validation to DataIntegrityProof in the verify step.

@aljones15 aljones15 self-assigned this Sep 3, 2024
lib/DataIntegrityProof.js Outdated Show resolved Hide resolved
lib/DataIntegrityProof.js Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 82.50000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 89.94%. Comparing base (0004a66) to head (73d78ed).

Files with missing lines Patch % Lines
lib/util.js 70.83% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #34      +/-   ##
==========================================
- Coverage   90.52%   89.94%   -0.58%     
==========================================
  Files           4        4              
  Lines         549      587      +38     
==========================================
+ Hits          497      528      +31     
- Misses         52       59       +7     
Files with missing lines Coverage Δ
lib/DataIntegrityProof.js 90.31% <100.00%> (+0.27%) ⬆️
lib/util.js 83.33% <70.83%> (-8.34%) ⬇️

Continue to review full report in Codecov by Sentry.

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

@aljones15 aljones15 requested a review from dlongley September 4, 2024 14:43
@aljones15 aljones15 marked this pull request as ready for review September 5, 2024 15:38
@aljones15
Copy link
Contributor Author

aljones15 commented Sep 5, 2024

@dlongley removed the checks involving now and clockSkew and just throw if created is defined and does not match the RegExp (there's a test for it). LMK if this is ok and I can add it for expires to. I did keep the code to convert the timestamp to a Date.

CHANGELOG.md Outdated Show resolved Hide resolved
lib/DataIntegrityProof.js Outdated Show resolved Hide resolved
Comment on lines 265 to 281
async verifyProof({proof, proofSet, document, documentLoader}) {
async verifyProof({
proof, proofSet,
document, documentLoader,
}) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you revert this unnecessary change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted here: 0f66109

lib/DataIntegrityProof.js Outdated Show resolved Hide resolved
lib/util.js Outdated Show resolved Hide resolved
@dlongley dlongley merged commit 6eef4b6 into main Sep 5, 2024
5 checks passed
@dlongley dlongley deleted the validate-created branch September 5, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants