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): Record each role data fields #244

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bkioshn
Copy link
Contributor

@bkioshn bkioshn commented Mar 19, 2025

Description

Record each role data fields

Related Issue(s)

Closes #241

Description of Changes

Each role data fields are record separately which is stored in role_data_record.
This role_data_record is difference from role_data_history where role_data_history store the whole role data at the specific point.
For example, for role 0,

Roledata at point 1222 {
     signing_key: Some(A)
     encryption_key: None
     payment_address: 0x123
    ......
}

Roledata at point 1322 {
     signing_key: None
     encryption_key: Some(B)
     payment_key: 0x456
    ......
}

Roledata at point 1422 {
     signing_key: Some(C)
     encryption_key: Some(B)
     payment_key: 0x456
    ......
}

Each entry of Roledata as shown above will be store in role_data_history

The role_data_record of role 0 will be

RoleDataRecord {
    signing_keys: [<1222, A>, <1422, C>]
    encryption_keys: [<1322, B>]
    payment_keys: [0x123, 0x456, 0x456]
    ....
}

Note that when dealing with key rotation, if the key added is already exist in the list, it will not be added into the list.

Breaking Changes

Renaming

  • all_role_data -> role_data_history
  • role_data -> role_data_record and the structure changes

Test

This change need to be tested again once test data is fully prepared
input-output-hk/catalyst-voices#2037

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

@bkioshn bkioshn added enhancement New feature or request F14 labels Mar 19, 2025
@bkioshn bkioshn self-assigned this Mar 19, 2025
Copy link
Contributor

github-actions bot commented Mar 19, 2025

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

@bkioshn bkioshn added the review me PR is ready for review label Mar 19, 2025
@stanislav-tkach stanislav-tkach requested a review from stevenj March 19, 2025 12:23
.or_insert(RoleDataRecord::new());

// Update role data record
if let Some(record) = role_data_record.get_mut(&number) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, but maybe it would be better to move this block to role_data_record.entry(number).and_modify(...) closure?.. Currently this condition is slightly confusing. because the record is always present because we inserting it above.

pub struct RoleDataRecord {
/// List of signing key and its associated point + tx index .
/// The vector index is used to indicate the key rotation.
signing_keys: Vec<PointData<KeyLocalRef>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Historically, this will become inaccurate.
As time passes, the array being referenced will change, so the key ref will not point to the historical key/cert, it will point to the latest key/cert in the array.

Simplified Example:

simple_keys_array = [none, key_a, key_b]
role_key = points to simple_keys_array[1]

So, this would create a signing key with that ref, in the first element of the vec.
If an update occurs like this:

simple_keys_array = [none, key_c, key_b]

The vec will still refer to the first element which is now key_c and not key_a as it was.
What should happen is we end up with a vec that has a form like:

signing_keys: [(time0, key_a,) (time1, key_c)]

So the changes over time are acurately captured.
The reality is, its most likely that updates will simply just replace the key in the array and will not (because they don't need to) update the key reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request F14 review me PR is ready for review
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

RBAC registration - record key rotation in role data
3 participants