Skip to content

Conversation

@Mr-Leshiy
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy commented Nov 7, 2025

Description

Related 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

…in` (#586)

* initial

* feat: provider

* feat: update chain

* feat: start_new_chain

* feat: merge validation result struct into cip509

* feat: exports

* feat: return new chain

* chore: return success object

* fix: older version

* feat: cat id in payload

* fix: new version

* chore: lintfix

* chore: remove persistent arguments

* tmp

* chore: complete moving to central module

* feat: ref fn

* chore: merge methods

* chore: minor

* feat: export modified chains

* chore: minor comment

* chore: rbac update logic

* chore: isolation

* chore: validation and lintfix

* docs: remove error doc

* chore: minor refactor

* fix: comments

* chore: validation function return

* Update rust/rbac-registration/src/providers.rs

Co-authored-by: Alex Pozhylenkov <[email protected]>

---------

Co-authored-by: Alex Pozhylenkov <[email protected]>
@Mr-Leshiy Mr-Leshiy added the draft Draft label Nov 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

📚 Docs Preview

The docs for this PR can be previewed at the following URL:

https://docs.dev.projectcatalyst.io/libs/feat/rbac

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

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

)

* wip

* wip

* wip

* wip

* revert

* wip

* add stolen_uris field

* wip

* wip

* wip

* fix

* wip

* wip

* fix spelling

* fix

* wip

* wip

* wip

* wip

* cleanup

* fix test

* fix clippy

* wip

* wip

* fix

* wip

* fix clippy

* fix

* wip

* wip
@Mr-Leshiy Mr-Leshiy added review me PR is ready for review squad: gatekeepers Catalyst App Backend, System Development & Integration Team do not merge yet PR is not ready to be merged yet and removed draft Draft labels Nov 19, 2025
@Mr-Leshiy Mr-Leshiy moved this from New to 👀 In review in Catalyst Nov 19, 2025
rafal-ch
rafal-ch previously approved these changes Nov 20, 2025
Copy link
Contributor

@rafal-ch rafal-ch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

/// Returns a list of all stake addresses.
/// Returns a set of all active (without taken) stake addresses.
#[must_use]
pub fn stake_addresses(&self) -> HashSet<StakeAddress> {
Copy link
Contributor

Choose a reason for hiding this comment

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

active_stake_addresses would be better

Comment on lines +124 to +126
let previous_addresses = self.stake_addresses();
let reg_addresses = cip509.stake_addresses();
let new_addresses: Vec<_> = reg_addresses.difference(&previous_addresses).collect();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding stake will be more readable when glancing through
eg. previous_stake_addresses

/// Get the latest encryption public key for a role.
/// Returns the public key and the rotation, `None` if not found.
#[must_use]
pub fn get_latest_encryption_pk_for_role(
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, if we are using public_key instead of pk this should be renamed, also other pk function too

) -> impl Future<Output = anyhow::Result<Option<CatalystId>>> + Send;

/// Update the chain by "taking" the given `StakeAddress` for the corresponding
/// RBAC chain's by the given `CatalystId`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is a bit confusing (for me 😅)

addr: &StakeAddress,
) -> impl Future<Output = anyhow::Result<bool>> + Send;

/// Returns a corresponding to the RBAC chain's Catalyst ID corresponding by the given
Copy link
Contributor

Choose a reason for hiding this comment

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

This one too. Maybe
"Returns the Catalyst ID associated with the RBAC chain for the given signing public key"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge yet PR is not ready to be merged yet review me PR is ready for review squad: gatekeepers Catalyst App Backend, System Development & Integration Team

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

5 participants