-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(rust/rbac-registration): Introduce Cip0134UriSet type #119
Conversation
d1251df
to
5d20379
Compare
a4366b2
to
6bb16b6
Compare
✅ Test Report | |
6bb16b6
to
f34c796
Compare
rust/rbac-registration/src/cardano/cip509/utils/cip134/uri_set.rs
Outdated
Show resolved
Hide resolved
/// | ||
/// This field isn't present in the encoded format and is populated by processing both | ||
/// `x509_certs` and `c509_certs` fields. | ||
pub certificate_uris: Cip0134UriSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this certificate_uris
is too specific for Cip509Metadata
(specific to role 0)
Might be useful to add it in RegistrationChain
.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps I'm missing something, but certificates be present in the next registrations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DIscussed: we decided to proceed with this approach for now because it isn't going to be merged directly to the main branch. But we will consider this again during the final review.
4c4d4e4
to
448899b
Compare
448899b
to
8160fdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM🥳
}, | ||
} | ||
} else { | ||
// Handle the x509 chunks 10 11 12 | ||
let x509_chunks = X509Chunks::decode(d, ctx)?; | ||
cip509_metadatum.x509_chunks = x509_chunks; | ||
// Technically it is possible to store multiple copies (or different instances) of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for miscommunication, storing multiple violate the CDDL spec, so only 1 is allowed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you were clear, perhaps it is a misunderstanding from my side. What I wanted to say with this comment is that it is possible to produce metadata with multiple different types, but it indeed would violate the specification.
6ad8cdc
into
rbac-registration-improvements
Description
Cip0134UriSet
type was added.Option<Vec<_>>
fields of theCip509RbacMetadata
structure were changed toVec<_>
.Related Issue(s)
Closes #104.
Description of Changes
I have renamed
Cip0134UriList
toCip0134UriSet
because I don't think that "list" is a proper description for the current structure.This pull request should be merged into the
rbac-registration-improvements
branch instead ofmain
. The goal is to have separate smaller incremental changes that can be reviewed together in a separate pull request (fromrbac-registration-improvements
tomain
).Please confirm the following checks