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

Add test vectors for JWS and Eip712 #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add test vectors for JWS and Eip712 #13

wants to merge 1 commit into from

Conversation

oed
Copy link
Collaborator

@oed oed commented Feb 12, 2024

This PR adds two test vector CAR files for JWS and eip712 varsigs. The content of these files can be explored through the https://explore.ipld.io/ interface.

  • eip712 uses secp256k1 and keccak256
  • jws uses:
    • secp256k1 + sha256
    • p256 + sha256
    • ed25519 + sha256

The encoding follows this general format:
image

Specifically for eip712 and secp256k1 signatures this format is used:
image

@oed oed requested review from expede and Gozala as code owners February 12, 2024 10:17
@cla-bot cla-bot bot added the cla-signed label Feb 12, 2024
Copy link
Collaborator

@expede expede left a comment

Choose a reason for hiding this comment

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

Huzzah test vectors 👏 Approved! I'm also very happy that there's JWS in here.

Either in this PR or one to follow, we should include a /test-vectors/README.md that explains to inspect with explore.ipld.io. (Also possibly worth noting that explore.ipld.io wouldn't let me upload the CAR files without modifying the file extension. Seems odd).

@oed and I had a discussion in Discord about 1. where these should live (I think here is a great place), and 2. any changes that need to be made to the spec.

In this picture...

Screenshot 2024-02-13 at 08 42 44

...it says "codec optional". How does that get signalled? Is there a one-bit flag or similar?

My understanding (please correct me) about what we need to update in the spec to support this is:

  • Add an entry for EIP-712
  • Add text RECOMMENDing that encoding info be at the end of a varsig header, for consistency & implementation ease.
  • Clarify that there's a header and a signature, in case you want to use the header separately (there's a use case in UCAN)

Is that about right?

@oed
Copy link
Collaborator Author

oed commented Feb 14, 2024

Either in this PR or one to follow, we should include a /test-vectors/README.md that explains to inspect with explore.ipld.io. (Also possibly worth noting that explore.ipld.io wouldn't let me upload the CAR files without modifying the file extension. Seems odd).

Good point. I can update this PR! Strange it doesn't let you upload, it definitely works for me. What do you need to change the extension to? It should already be *.car.

...it says "codec optional". How does that get signalled? Is there a one-bit flag or similar?

I think this is supposed to depend on the key codec, the key type would basically dictate if it's needed or not. But in our test cases we actually include it for all key codecs.

My understanding (please correct me) about what we need to update in the spec to support this is:

Yes, the spec needs to be updated in a few places. I will follow up on that as soon as I have some spare cycles.

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

Successfully merging this pull request may close these issues.

2 participants