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

Apply new rules for signatures and witness events #402

Merged
merged 7 commits into from
Jul 16, 2024
Merged

Conversation

it-spiderman
Copy link
Collaborator

@it-spiderman
Copy link
Collaborator Author

now that signature is stored on the revision that injects it (not on the one that was current at the time of signing), verification though external verifier fails.
This is becuase when you sign, messages is i sign .... {verification_hash of current revision}, and that is used for verification. But that hash specified in message does not match the revision that holds the signature (as it is now added to the revision after it, one that injects the sig)

Also, Verifier is freaking out in general

@FantasticoFox
Copy link
Collaborator

We will have a look today and test. Will get back with any feedback I have.

Thank you Dejan!

@FantasticoFox
Copy link
Collaborator

FantasticoFox commented Jun 26, 2024

Find an example JSON page. The Witness object exists but the path with the merkle-tree-path is completly missing.
[ ] fix to include merkle-tree path

6c3fbf41e5_Testpage_Witness.json

Signatures_hash must be included when caluclating the verification hash of the revision, which also stores the signature. Currently the signature hash is ignored.
[ ] fix: include signature hash in the hasher to calculate verification hash with signature hash

See example manipulated page I used for testing the signature (it should work if I put the signature back into the first revision, the verifier should return green as the second revision takes the signature hash as input to calculate the revision hash).
6c3fbf41e5_Testing_Signatures (2).json

image
Current hash does not include signature hash, it is only content + metadata_hash

The same problem persists with the witness hash, it is not included in the hasher but should be included.
[ ] fix: ensure that witness_hash is part of the hasher input to calculate verification hash of the second revision (see example)

image
Current hash does not include witness hash, it is only content + metadata_hash

@it-spiderman
Copy link
Collaborator Author

Updated commit, hopefully all is good now

@FantasticoFox
Copy link
Collaborator

Signature is tested and works. Verifier needs adjustment.
Manuel tests for signature checks out after putting the signature to its original position.
cf4f30f866_Whatever_Page.json

@FantasticoFox
Copy link
Collaborator

Witness data is malformatted and at the wrong place for verifier to process.

cf4f30f866_Interactive_Tutorial.json

"structured_merkle_proof": [] is still empty in the second revision and is now populated at the wrong position witness_slot in content. The structured merkle proof needs to be stored as JSON in the witness object outside of the content.

The witness-slot does contain now all verification data (including merkle-tree-proof). Its redudent to the witness data in the JSON and not required in full length in there as it is invisibile to the user anyway at this moment.
A useful information at this place would be to show the link to the onchain timestamp event with basic witness information.

@FantasticoFox
Copy link
Collaborator

Like the Signature field:

image

The Witness field should include useful user information about the witness event.

Those information are:

  • Witness Netzwerk (e.g. Mainnet, Sepolia, Holesky)
  • Link so Etherscan (Witness_event_transaction_hash) clickable, transaction hash needs to be visible for potential manuel lookup
  • Link to Witness event (the page which is visible in the publisher)

Successcreteria:

  • Please test that the verifier is getting green with signature and witness event being manuelly moved into the revision before for testing purpose.

@FantasticoFox FantasticoFox merged commit f947e40 into main Jul 16, 2024
4 checks passed
@FantasticoFox FantasticoFox deleted the new_rules branch July 16, 2024 16:27
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.

2 participants