Skip to content

RBAC work, adding updateMetadata and updateAdminRoles operations#659

Open
rbaconcordium wants to merge 11 commits intofeature/rbacfrom
RBC-9/rbac
Open

RBAC work, adding updateMetadata and updateAdminRoles operations#659
rbaconcordium wants to merge 11 commits intofeature/rbacfrom
RBC-9/rbac

Conversation

@rbaconcordium
Copy link
Contributor

@rbaconcordium rbaconcordium commented Mar 2, 2026

Purpose

To introduce two operations, to update metadata and to update admin roles.

Changes

Added two functions, updateMetadata and updateAdminRoles.
Added TokenOperationType.UpdateMetadata and TokenOperationType.AssignAdminRoles, RevokeAdminRoles.
Added enum TokenAdminRole
Added type TokenUpdateAdminRoleDetails
Added operation types TokenUpdateAdminRoleOperation, TokenAssignAdminRolesOperation, TokenRevokeAdminRolesOperation and TokenUpdateMetadataOperation

Checklist

  • [ x ] My code follows the style of this project.
  • [ x ] The code compiles without warnings.
  • [ x ] I have performed a self-review of the changes.
  • [ x ] I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

CLA acceptance

_Remove if not applicable.

By submitting the contribution I accept the terms and conditions of the
Contributor License Agreement v1.0

@rbaconcordium rbaconcordium requested a review from soerenbf March 2, 2026 15:48
@rbaconcordium rbaconcordium changed the title initial work starting RBAC work, adding updateMetadata and updateAdminRoles operations Mar 3, 2026
Copy link
Contributor

@soerenbf soerenbf left a comment

Choose a reason for hiding this comment

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

Looks good in general. Here are a few things I think would be good to add/change:

  • Change the target branch to a feature branch that collects functionality for the feature (as discussed)
  • Add unit tests for the CBOR encoding of the operations added
  • Update the CHANGELOG.md

Comment on lines +952 to +980

/**
* assign roles to admin or revoke roles from admin
* @param {TokenOperationType} type - The type of operation, either `AssignAdminRoles` or `RevokeAdminRoles`.
* @param {Token} token - The token to update.
* @param {TokenUpdateAdminRolesDetails} updateAdminRoleDetails - role details to be processed
* @param {AccountAddress.Type} sender - The account address of the sender.
* @param {AccountSigner} signer - The signer responsible for signing the transaction.
* @param {TokenUpdateMetadata} [metadata={ expiry: TransactionExpiry.futureMinutes(5) }] - The metadata for the token update.
* @returns A promise that resolves to the transaction hash.

*/
export async function updateAdminRoles(
type: TokenOperationType.AssignAdminRoles | TokenOperationType.RevokeAdminRoles,
token: Token,
updateAdminRoleDetails: TokenUpdateAdminRolesDetails,
sender: AccountAddress.Type,
signer: AccountSigner,
metadata?: TokenUpdateMetadata
): Promise<TransactionHash.Type> {
switch (type) {
case TokenOperationType.AssignAdminRoles:
return assignAdminRoles(token, updateAdminRoleDetails, sender, signer, metadata);
case TokenOperationType.RevokeAdminRoles:
return revokeAdminRoles(token, updateAdminRoleDetails, sender, signer, metadata);
default:
throw new Error(`Unsupported operation type: ${type}`);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this. The type in the cddl is mostly there to collect similar operations under a single name but it's not useful I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing this function and exporting the individual functions instead.

UpdateAllowList = 'allowList',
UpdateDenyList = 'denyList',
Pause = 'pause',
Unpause = 'unpause',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Unpause = 'unpause',

There's no "unpause" role, the "pause" role will be used for this purpose as well 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing

@rbaconcordium rbaconcordium changed the base branch from main to feature/rbac March 4, 2026 13:47
@rbaconcordium
Copy link
Contributor Author

Looks good in general. Here are a few things I think would be good to add/change:

  • Change the target branch to a feature branch that collects functionality for the feature (as discussed)
  • Add unit tests for the CBOR encoding of the operations added
  • Update the CHANGELOG.md

I have changed as per above.

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.

2 participants