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

CIP-0108 | Ensure "CIP-0008" is in capital letters #951

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

palas
Copy link
Contributor

@palas palas commented Dec 16, 2024

This PR addresses the issue #949, by ensuring CIP-0008 is used in capital letters in the witnessAlgorithm field of the witness type in CIP-0108, updating the "no-confidence" example and the test vector Markdown documentation.

Fixes #949.

@palas palas changed the title Ensure "CIP-0008" is in capital letters (address #949) CIP-0108 | Ensure "CIP-0008" is in capital letters (address #949) Dec 16, 2024
@rphair rphair changed the title CIP-0108 | Ensure "CIP-0008" is in capital letters (address #949) CIP-0108 | Ensure "CIP-0008" is in capital letters Dec 16, 2024
@rphair rphair added the Category: Metadata Proposals belonging to the 'Metadata' category. label Dec 16, 2024
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Can't think of any technical things that would fail because of this schema change but please @Ryun1 @Crypto2099 @perturbing @gitmachtl confirm if you can.

@rphair rphair added Update Adds content or significantly reworks an existing proposal State: Last Check Review favourable with disputes resolved; staged for merging. labels Dec 16, 2024
@Ryun1
Copy link
Collaborator

Ryun1 commented Dec 18, 2024

Great spot @palas
ill reach out to tooling to double check that this change wouldnt break their implementations before we merge

@@ -34,7 +34,7 @@ Blake2b-256 hash digest of canonicalized body: `68d6fe27087457acf0164e65414238c4
### Motion of No-Confidence

Example metadata document file: [no-confidence.jsonld](./examples/no-confidence.jsonld).
Blake2b-256 of the file content (to go onchain): `6c27e5bd0d7cdec7ddb30956be0b5eac892a8330e00689692d18f3815a71bf9f`
Blake2b-256 of the file content (to go onchain): `87ba5c10c89484c52265b50d669b4acf116aaeb8a6fa35fbf06e5ec67cda9270`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I pushed this update, because of the change introduced in no-confidence example changed the hash digest

@Ryun1
Copy link
Collaborator

Ryun1 commented Jan 4, 2025

Spoke to a couple implementors, this change is all good to go.
Fortunately because governance actions are very expensive on mainnet, there isnt any examples of this on mainnet

Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

Thanks @palas 💪

@rphair rphair merged commit eb96b28 into cardano-foundation:master Jan 5, 2025
@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Jan 5, 2025
@gitmachtl
Copy link
Contributor

gitmachtl commented Jan 7, 2025

Why do we use the public key for CIP-0008 method here and not the Cose_Key?

Is there a written definition on the parameters used for this signature? Because CIP0008 allows for different ones. I guess we're going with the

  kty (1) - must be set to OKP (1)
  kid (2) - Optional, if present must be set to the same value as in the Sig_structures protectedHeader_cbor via map entry (4)
  alg (3) - must be set to EdDSA (-8)
  crv (-1) - must be set to Ed25519 (6)
  x (-2) - must be set to the public key bytes of the key used to sign the Sig_structure

one?

If so, that should be specified in CIP108 too imo. @Ryun1 ?

@Ryun1
Copy link
Collaborator

Ryun1 commented Jan 19, 2025

@gitmachtl
the motivation for this inclusion in CIP8 is so that these signatures can be created over CIP30 connections, so using light wallets

@gitmachtl
Copy link
Contributor

gitmachtl commented Jan 20, 2025

@Ryun1 yes adding CIP8 is totally fine! i am refering that there should be more detailed information in the CIP about which parameters should be used with CIP8. and normally we have a COSE_Sign1 signature cbor-structure/hex and a corresponding COSE_Key cbor-structure/hex, not only the publickey. for verification we need the parameters set as above, otherwise it would fail. but that information should be in the description of the CIPs imo. or we switch to use the COSE_Key in the publickey field of the signatures?

@gitmachtl
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Metadata Proposals belonging to the 'Metadata' category. Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CIP-0108 - Inconsistent field name
4 participants