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.

Sign up for GitHub

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

Jump to bottom

CIP-0052 | Add public keys to auditor table #406

Merged
merged 2 commits into from
Jan 17, 2023
Merged

CIP-0052 | Add public keys to auditor table #406

merged 2 commits into from
Jan 17, 2023

Conversation

simonjohnthompson
Copy link
Contributor

Added field for public key information for auditors. This to be used to verify signed audit certification metadata.

@rphair rphair added the Update Adds content or significantly reworks an existing proposal label Dec 6, 2022
@rphair rphair changed the title CIP52: added auditor public key info to auditor table (section 3). CIP-0052 | Add public keys to auditor table Dec 6, 2022
rphair
rphair reviewed Dec 6, 2022
View reviewed changes
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.

@simonjohnthompson it's good to have that extra field in the table for sure... but is this PR complete without those keys filled in? If not, should this PR be kept in abeyance until those keys are added in further commits?

@rphair rphair mentioned this pull request Dec 6, 2022
Closed
@L-as
Copy link
Contributor

L-as commented Dec 15, 2022 •
edited
Loading

We should definitely add the public keys first. I suppose we can just comment on this PR.

@rphair
Copy link
Collaborator

rphair commented Dec 15, 2022

I suppose we can just comment on this PR.

sure @L-as if they're posted here in a way that allows editors to verify their validity (e.g. from the auditor's GitHub account), we can commit them if the PR author hasn't done so already.

@L-as
Copy link
Contributor

L-as commented Dec 21, 2022

MLabs has a PGP-compatible key (using ed25519) suitable for signing messages. The fingerprint is 64BC640B5454215D12165EEAEEFF303D2643ABA2.

@rphair
Copy link
Collaborator

rphair commented Dec 21, 2022

@L-as @simonjohnthompson is the last commit clear enough according to how people will be using the table?

@L-as
Copy link
Contributor

L-as commented Dec 22, 2022

I'd say so.

@rphair
Copy link
Collaborator

rphair commented Jan 16, 2023

Thanks @L-as for the help so far getting the key for MLabs ... @simonjohnthompson I'm marking this as Waiting for Author because this is coming up for initial review at our CIP meeting tomorrow and I (and maybe other editors) still have no idea how to get the remaining keys for FYEO, Hachi & Tweag... can you tag some GitHub IDs for them there and ask them to contribute their keys from an official source?

@rphair rphair added the State: Waiting for Author Proposal showing lack of documented progress by authors. label Jan 16, 2023
@KtorZ
Copy link
Member

KtorZ commented Jan 17, 2023

Suggestion: public keys here may be added over time by a representative of each relevant party, with a commit signed by the respective private key and from an account that belongs to the party's organization.

How does that sound?

SebastienGllmt
SebastienGllmt approved these changes Jan 17, 2023
View reviewed changes
Copy link
Contributor

@SebastienGllmt SebastienGllmt left a comment

Choose a reason for hiding this comment

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

Makes sense. Not sure there is a reason to block this PR waiting for other entities to add something. They can just make a followup PR if they want to

@KtorZ KtorZ merged commit c854763 into cardano-foundation:master Jan 17, 2023
@L-as
Copy link
Contributor

L-as commented Jan 18, 2023

Suggestion: public keys here may be added over time by a representative of each relevant party, with a commit signed by the respective private key and from an account that belongs to the party's organization.

How does that sound?

It's not sound. There's a cyclic dependency. s/signed commit/PR by member of org/.

@KtorZ
Copy link
Member

KtorZ commented Jan 19, 2023 •
edited
Loading

There's a cyclic dependency.

What do you mean?

  • Alice puts the public key in the file.
  • Alice commits and signs the change with corresponding private key
  • Alice opens a PR

where's the cycle exactly?

@L-as
Copy link
Contributor

L-as commented Jan 19, 2023

That only tells us that the owner of the key consented, not that the owner is the correct one.

@KtorZ
Copy link
Member

KtorZ commented Jan 19, 2023

Hence the second part: "and from an account that belongs to the party's organization"

@rphair rphair mentioned this pull request Jul 25, 2023
Merged
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Nov 17, 2023
@simonjohnthompson @rphair
CIP-0052 | Add public keys to auditor table (cardano-foundation#406)
153f7f1 
* CIP52: added auditor public key info to auditor table (section 3).

* added MLabs public key

Co-authored-by: Robert Phair <rphair@cosd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rphair rphair rphair left review comments

@SebastienGllmt SebastienGllmt SebastienGllmt approved these changes

@KtorZ KtorZ KtorZ approved these changes

Assignees
No one assigned
Labels
State: Waiting for Author Proposal showing lack of documented progress by authors. Update Adds content or significantly reworks an existing proposal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@simonjohnthompson @L-as @rphair @KtorZ @SebastienGllmt