feat(rust/rbac-registration): Introduce Cip0134UriSet type #119

stanislav-tkach · 2024-12-20T17:36:11Z

Description

The Cip0134UriSet type was added.
Option<Vec<_>> fields of the Cip509RbacMetadata structure were changed to Vec<_>.
Error handling becomes stricter.

Related Issue(s)

Closes #104.

Description of Changes

I have renamed Cip0134UriList to Cip0134UriSet 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 of main. The goal is to have separate smaller incremental changes that can be reviewed together in a separate pull request (from rbac-registration-improvements to main).

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

github-actions · 2024-12-25T17:58:12Z

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

rust/rbac-registration/src/cardano/cip509/utils/cip134/uri_set.rs

rust/rbac-registration/src/cardano/cip509/mod.rs

bkioshn · 2024-12-27T02:06:26Z

rust/rbac-registration/src/cardano/cip509/rbac/mod.rs

+    ///
+    /// 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,


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?

Perhaps I'm missing something, but certificates be present in the next registrations?

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.

bkioshn

LGTM🥳

bkioshn · 2024-12-30T02:19:08Z

rust/rbac-registration/src/cardano/cip509/mod.rs

                // 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


Sorry for miscommunication, storing multiple violate the CDDL spec, so only 1 is allowed

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.

* Introduce Cip0134UriSet type (#119) * Store x509 certificates (Cip509RbacMetadata::x509_certs) in the decoded format (#128) * Store and update certificate URIs in the registration chain (#130) * Check for presense of purpose and txn_inputs_hash fields * Use Blake2b256Hash type * Make Cip509 field private * Use problem report * More problem report usage in Cip509RbacMetadata * Cip0134Uri::try_from(&[u8]) * Move addresses extraction to a function * Move Cip0134Uri to the cardano-blockchain-types crate * role data as map * Role0 constant * Member functions * Cip509 constructors * Validation * Fix after merge * Validation tests * Registration chain * Fix clippy warnings and some tests * Test decremented_index * decode context * Role data conversion * Fix clippy lints * Update the validation tests * Update chunks decoding tests * Move payment history types * Move payment history to Cip509 * Add TODO comments * Use TxnWitness from cardano-blockchain-types * Clippy * Use (more) types from the 'cardano-blockchain-types' crate * Update the registration chain tests * Fix clippy (again) * Fix warnings in the tests * Use the 'cbork-utils' crate * Remove MultiEraBlock::new_block * Fix the 'decode_any' usage * Split certificates into multiple files * Fix some review comments * Remove duplicated hash helpers * Fix after merge * Update the payment key logic * Remove unused dependencies * More error handling * Improve role 0 signing key validation * Error handling * More error handling * Fix Clippy * Improve error handling once again * update decode_revocation_list * test(rust/rbac-registration): Fix test in CIP509 (#160) * test(rbac-registration): fix test and test data Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): name txn_index Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): add more test to registration chain Signed-off-by: bkioshn <[email protected]> --------- Signed-off-by: bkioshn <[email protected]> * Fix after merge * Add the 'assert_valid' function for the test data * test(rust/rbac-registration): RBAC None role0 key reference (#163) * test(rbac-registration): fix test and test data Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): name txn_index Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): add more test to registration chain Signed-off-by: bkioshn <[email protected]> * test(rbac-registration): fix conway4 block Signed-off-by: bkioshn <[email protected]> --------- Signed-off-by: bkioshn <[email protected]> * Add/fix certificates/role checks * Check the encryption key for non-0 roles * Check for role 0 certificates for other roles * test(rust/rbac-registration): Fix test data + undefined bug (#168) * test(rbac-registration): fix test and test data Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): name txn_index Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): add more test to registration chain Signed-off-by: bkioshn <[email protected]> * test(rbac-registration): fix conway4 block Signed-off-by: bkioshn <[email protected]> * test(rbac-registration): fix conway4 block Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): consume undefined cbor Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): format Signed-off-by: bkioshn <[email protected]> --------- Signed-off-by: bkioshn <[email protected]> * Remove the 'raw_aux_module' because it was moved to cardano-blockchain-types * Use block.txn_metadata function * Remove removed params from the documentation * One more update for the role data validation * Remove unused dependency * Ensure that there is no encryption key for the role 0 * Use the encryption key value in the message * Use UuidV4 * Remove duplicated check * Revert UUID changes (temporary) * Update UUID names * Use revision for cbork-utils * Use revisions instead of paths for dependencies * test(rust/rbac-registration): Fix RBAC test data (#175) * test(rbac-registration): fix test and test data Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): name txn_index Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): add more test to registration chain Signed-off-by: bkioshn <[email protected]> * test(rbac-registration): fix conway4 block Signed-off-by: bkioshn <[email protected]> * test(rbac-registration): fix conway4 block Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): consume undefined cbor Signed-off-by: bkioshn <[email protected]> * fix(rbac-registration): format Signed-off-by: bkioshn <[email protected]> * test(rbac-registration): fix conway1 and 4 test data Signed-off-by: bkioshn <[email protected]> --------- Signed-off-by: bkioshn <[email protected]> * Use tag for dependencies * Update public key at 0 index error message --------- Signed-off-by: bkioshn <[email protected]> Co-authored-by: Steven Johnson <[email protected]> Co-authored-by: bkioshn <[email protected]>

stanislav-tkach added the do not merge yet PR is not ready to be merged yet label Dec 20, 2024

stanislav-tkach self-assigned this Dec 20, 2024

stanislav-tkach mentioned this pull request Dec 20, 2024

feat(rust/rbac-registration): Introduce Cip0134UriList type #106

Closed

8 tasks

stanislav-tkach changed the title ~~feat(rust/rbac-registration): Introduce Cip0134UriSet type~~ feat(rust/rbac-registration): Introduce Cip0134UriSet type Dec 20, 2024

stanislav-tkach force-pushed the cip0134set branch 2 times, most recently from d1251df to 5d20379 Compare December 25, 2024 14:51

stanislav-tkach requested a review from bkioshn December 25, 2024 14:57

stanislav-tkach force-pushed the cip0134set branch 3 times, most recently from a4366b2 to 6bb16b6 Compare December 25, 2024 17:11

stanislav-tkach marked this pull request as ready for review December 25, 2024 17:12

stanislav-tkach added review me PR is ready for review and removed do not merge yet PR is not ready to be merged yet labels Dec 25, 2024

stanislav-tkach force-pushed the cip0134set branch from 6bb16b6 to f34c796 Compare December 26, 2024 13:19

bkioshn reviewed Dec 27, 2024

View reviewed changes

stanislav-tkach force-pushed the cip0134set branch 3 times, most recently from 4c4d4e4 to 448899b Compare December 28, 2024 14:04

Introduce Cip0134UriSet type

8160fdf

stanislav-tkach force-pushed the cip0134set branch from 448899b to 8160fdf Compare December 28, 2024 15:12

bkioshn approved these changes Dec 30, 2024

View reviewed changes

stanislav-tkach merged commit 6ad8cdc into rbac-registration-improvements Dec 30, 2024
21 of 24 checks passed

stanislav-tkach deleted the cip0134set branch December 30, 2024 11:17

stanislav-tkach mentioned this pull request Dec 30, 2024

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

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(rust/rbac-registration): Introduce Cip0134UriSet type #119

feat(rust/rbac-registration): Introduce Cip0134UriSet type #119

Uh oh!

stanislav-tkach commented Dec 20, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Dec 25, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkioshn Dec 27, 2024

Uh oh!

stanislav-tkach Dec 27, 2024

Uh oh!

stanislav-tkach Dec 30, 2024

Uh oh!

bkioshn left a comment

Uh oh!

bkioshn Dec 30, 2024

Uh oh!

stanislav-tkach Dec 30, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(rust/rbac-registration): Introduce Cip0134UriSet type #119

feat(rust/rbac-registration): Introduce Cip0134UriSet type #119

Uh oh!

Conversation

stanislav-tkach commented Dec 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Description of Changes

Please confirm the following checks

Uh oh!

github-actions bot commented Dec 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkioshn Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

stanislav-tkach Dec 27, 2024

Choose a reason for hiding this comment

Uh oh!

stanislav-tkach Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

bkioshn left a comment

Choose a reason for hiding this comment

Uh oh!

bkioshn Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

stanislav-tkach Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stanislav-tkach commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 25, 2024 •

edited

Loading