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

feat(rust/rbac-registration): refactoring #127

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

stanislav-tkach
Copy link
Contributor

@stanislav-tkach stanislav-tkach commented Dec 30, 2024

Description

  • The Cip0134UriSet type was added.
  • Option<Vec<_>> fields of the Cip509RbacMetadata structure were changed to Vec<_>.
  • Error handling becomes stricter.
  • Store x509 certificates (Cip509RbacMetadata::x509_certs) in the decoded format.
  • Store certificate URIs in the registration chain.

Related Issue(s)

Closes #104.

Related Pull Requests

This is a combination of the following pull requests:

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@stanislav-tkach stanislav-tkach self-assigned this Dec 30, 2024
@stanislav-tkach stanislav-tkach changed the title feat(rust/rbac-registration): feat(rust/rbac-registration): refactoring Dec 30, 2024
Copy link
Contributor

github-actions bot commented Dec 30, 2024

Test Report | ${\color{lightgreen}Pass: 230/230}$ | ${\color{red}Fail: 0/230}$ |

@stanislav-tkach stanislav-tkach marked this pull request as ready for review January 2, 2025 09:49
bkioshn
bkioshn previously approved these changes Jan 3, 2025
Copy link
Contributor

@bkioshn bkioshn left a comment

Choose a reason for hiding this comment

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

LGTM 🌈

rust/rbac-registration/src/cardano/cip509/mod.rs Outdated Show resolved Hide resolved
rust/rbac-registration/src/cardano/cip509/mod.rs Outdated Show resolved Hide resolved
rust/rbac-registration/src/cardano/cip509/mod.rs Outdated Show resolved Hide resolved
rust/rbac-registration/src/cardano/cip509/mod.rs Outdated Show resolved Hide resolved
rust/rbac-registration/src/cardano/cip509/mod.rs Outdated Show resolved Hide resolved
rust/rbac-registration/src/cardano/cip509/rbac/mod.rs Outdated Show resolved Hide resolved
@@ -92,7 +107,13 @@ pub(crate) enum Cip509IntIdentifier {
impl Decode<'_, ()> for Cip509 {
fn decode(d: &mut Decoder, ctx: &mut ()) -> Result<Self, decode::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The metadata should take the paired transaction and witness set as context. Its not possible to decode properly without it, i don't think?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is needed in validation and collecting the data for the registration chain.

rust/rbac-registration/src/cardano/cip509/validation.rs Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jan 3, 2025

Test Report | ${\color{lightgreen}Pass: 256/256}$ | ${\color{red}Fail: 0/256}$ |

Copy link
Contributor

github-actions bot commented Jan 3, 2025

Test Report | ${\color{lightgreen}Pass: 256/256}$ | ${\color{red}Fail: 0/256}$ |

@stanislav-tkach stanislav-tkach force-pushed the rbac-registration-improvements branch from 5fad6f6 to a5af8a8 Compare January 3, 2025 23:46
@stanislav-tkach stanislav-tkach marked this pull request as draft January 4, 2025 11:40
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.

🛠️ [TASK] : Create a type representing a list of CIP-0134 URIs
3 participants