Registry Controls - #2
Open
vwagner wants to merge 39 commits into
Open
Conversation
- Add 4 new RegistryRole enum values: LIEN_OWNER, SECURED_PARTY_FOR_LIEN, SECURED_PARTY_FOR_ENOTE, PLEDGEE - Change MsgGrantRole/MsgRevokeRole from single signer to repeated signers - Add MsgSetRoles/RoleUpdate for atomic multi-role updates - Add authorization.proto defining SignatureType, NftRole, Assignment enums and RoleAuthorization, Authorization, SignatureRequirement, RoleAssignment, RolePriority, RolePriorityEntry message types - Add types/authorization.go with static role authorization policies: DefaultRoleAuthorizations() and RoleAuthorizationMap() - Add keeper/authorization.go with full authorization validation engine: resolveRegistryRoleAddresses, resolveNFTRoleAddresses, resolveRolePriorityAddresses, resolveRoleAssignmentAddresses, evaluateSignatureRequirement, evaluateAuthorization, ValidateRoleChangeAuthorization - Update msg_server.go: GrantRole/RevokeRole use multi-signer auth; add SetRoles handler - Add keeper.go SetRoles method with atomic role updates and diff-based event emission - Update CLI grant-role/revoke-role commands for repeated signers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implements multi-party Controller authorization using single-signer messages whose approvals accumulate in registry state and auto-apply once the role's policy is satisfied. Unlike the native repeated-signers model, every message is single-signer, so each approval is delegable via authz. - proto: RoleChangeOperation enum + PendingRoleChange state; MsgProposeRoleChange and MsgApproveRoleChange (single-signer); proposed/approved/applied events. - keeper: PendingRoleChanges collection + accumulation/auto-apply reusing the existing role-typed policy engine. - types: deterministic change-id helper, ValidateBasic, event constructors. - tests: full Controller matrix plus authz delegation proof. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert MsgGrantRole/MsgRevokeRole/MsgSetRoles from repeated signers back
to a single signer field so all registry messages are single-signer and
therefore authz-delegable. Multi-party Controller updates now flow
exclusively through the Option B propose/approve accumulation path.
- tx.proto: 3 messages use single signer (regenerated tx.pb.go)
- keeper/msg_server.go: handlers pass []string{msg.Signer} to policy engine
- client/cli/tx.go: CLI uses Signer
- types/msgs.go: single-signer ValidateBasic
- types/msgs_test.go: makers and ValidateBasic cases updated
- Delete authorization_acceptance_test.go (multi-signer model removed);
Controller coverage lives in role_change_accumulation_acceptance_test.go
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expose and persist the Option B pending role-change state: - query.proto: PendingRoleChange(id) and PendingRoleChanges(key, pagination) RPCs plus REST routes; keeper GetPendingRoleChanges paginated walk with optional key filter; query-server handlers and request Validate methods - genesis.proto: GenesisState.pending_role_changes; InitGenesis/ExportGenesis round-trip pending changes so in-flight approvals survive export/import - CLI query: pending-role-change, pending-role-changes - CLI tx: propose-role-change, approve-role-change - Tests: query-by-id, list (all/filtered/empty/paginated), genesis round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Complete the Option B acceptance matrix: - Scenario C: Controller REVOKE accumulation (sole-controller revoke applies on one approval; with a Secured Party set, revoke requires current controller + secured party) - Invalidation-by-re-resolution: after the controller is rotated out from under a pending change, the stale approval no longer counts; the live current controller must approve for the change to apply - Edge cases: duplicate approval is idempotent, approving a non-existent change errors, and a registry deleted underneath cleans up the orphaned pending change Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hz reject matrix Add the three single-signer reject subtests the ticket enumerates for the Controller + Secured Party scenario (only current controller, only new controller, only secured party). Extend authz coverage to the full ticket reject matrix via a neutral executor that carries each party's delegated authority, with the proposal and every approval dispatched through authz MsgExec. Add a scenario 1 (no secured party) authz happy path to mirror "same scenarios as above" via authz. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Sort and de-duplicate addresses in NewPendingRoleChangeID so the id is truly set-based (c1) - Emit EventRoleChangeApproved only on newly recorded approvals (c2) - SetRoles returns RegistryNotFound only for collections.ErrNotFound; wrap other errors (c3) - Reject NftRole used with non-ASSIGNMENT_CURRENT* variants (c4) - Ignore approvals from addresses not eligible under the role policy to prevent state bloat/DoS (c5) - Deep-copy the original entry in SetRoles before mutation so diff events are correct (c6) - Validate role enum (Role.Validate()) per RoleUpdate in MsgSetRoles/MsgProposeRoleChange (c7) - Enforce exactly-one cardinality for ASSIGNMENT_CURRENT/ASSIGNMENT_NEW (c8) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Reject duplicate roles across the batch and duplicate addresses within a RoleUpdate in MsgSetRoles/MsgProposeRoleChange ValidateBasic (shared helper) - Fix propose-role-change CLI example to use secured_party_for_enote - Fail closed in evaluateSignatureRequirement on unspecified/unknown signature types - Fail closed in resolveRegistryRoleAddresses on unsupported/unspecified assignments - Fail closed in resolveRoleAssignmentAddresses when no role selector is set - Enforce exactly 0 or 2 args for the pending-role-changes query command Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Reject new role-change proposals from proposers not eligible to approve any affected role, so pending-change state only grows from actionable changes - Fail closed in resolveNFTRoleAddresses on unspecified/unknown NftRole values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
feat(registry): add multi-signer authorization controls (Phase 1 PoC)
Phase C1: introduce maintainer-managed RegistryClass config that overrides the module's static default authorization policy via two-tier resolution (class policy -> static default / NFT-owner fallback). - proto: RegistryClass message; registry_class_id on RegistryEntry and MsgRegisterNFT; MsgCreateRegistryClass and MsgUpdateRegistryClassRoleAuthorization; RegistryClass/RegistryClasses queries; class created/updated events; genesis registry_classes field - types: RegistryClass validation; new error codes; AllRequestMsgs and ValidateBasic for the new messages; genesis validation with duplicate-id check - keeper: registry class CRUD and roleAuthorizationsForEntry two-tier resolver; class-existence checks in RegisterNFT and RegistryBulkUpdate; CreateRegistry gains a registryClassID parameter; genesis and query-server handlers - cli: create/update class tx commands and class queries - tests: registry_class_acceptance_test covering class CRUD, maintainer enforcement, two-tier resolution, and genesis round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… policies
Phase C2: introduce registry module Params holding the default role
authorization policies, forming the middle tier of two-tier resolution
(registry class policy -> module params default -> NFT-owner fallback).
- proto: params.proto with Params{role_authorizations}; MsgUpdateParams
(authority) + response and UpdateParams rpc; Params query + REST route;
genesis params field; EventParamsUpdated
- types: DefaultParams (empty by default, preserving legacy NFT-owner
authorization), Params.Validate, MsgUpdateParams ValidateBasic and codec
registration, genesis params validation
- keeper: Params collections.Item + internal gov authority; GetParams/
SetParams/ValidateAuthority; resolver default tier now reads params;
UpdateParams handler (authority-gated); genesis and query handlers
- cli: update-params tx (governance) and params query
- rename DefaultRoleAuthorizations to ControllerRoleAuthorizations: the
CONTROLLER policy is a ticket example, not a chain default; the default
params are empty so behavior matches main (NFT-owner authorization)
- tests: params acceptance coverage (default empty, authority enforcement,
params-driven default tier, genesis round-trip); accumulation tests now
install the CONTROLLER policy explicitly
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hase C3) The ticket's per-role authorization policies (Originator, Lien Owner, Controller, Secured Party for Lien/eNote, Pledgee, Servicer, Sub-servicer) are user/governance-created RegistryClass role_authorizations data, not hard-coded chain logic. The policy engine already supports every construct they use, including the unilateral foreclosure path. So C3 adds validation and tests/examples rather than new role logic: - Add Validate() for SignatureType, NftRole, Assignment, and Assignment.IsCurrent(); deepen validateRoleAuthorizations to reject malformed authorization paths at create time (unknown/unspecified enums, missing selectors, nft_role with a NEW assignment, empty paths) instead of only failing closed at evaluation time. - Enforce class.Validate() on the registry-class create/update write path (defense-in-depth beyond ValidateBasic). - Add DART role-policy acceptance tests covering each role's consent matrix, the Lien Owner and Controller foreclosure paths, and malformed-policy rejection. - Ship x/registry/spec/examples/dart_registry_class.json (a complete DART registry class) kept in sync with the test builders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, C5)
Phase C4: emit a comprehensive provenance.registry.v1.EventRoleUpdated
(nft_id, asset_class_id, registry_class_id, role, addresses,
previous_addresses, signers) additively alongside the existing granular
EventRoleGranted/EventRoleRevoked events. The satisfying authorization
path's role/assignment/addresses are surfaced via CollectSatisfyingSigners;
the legacy non-policy path reports the NFT owner at ASSIGNMENT_CURRENT.
Emitted from GrantRole/RevokeRole/SetRoles and the pending-apply path
(per-role, with pre-mutation previous_addresses).
Phase C5: add a ValidateRoleChange dry-run query
(QueryValidateRoleChangeRequest{key, role_updates, approvers} ->
{error, authorized}) that reuses the accumulation evaluation read-only via
EvaluateRoleChange with no state writes. Adds the rpc/messages to
query.proto, a query_server handler, and the GetCmdQueryValidateRoleChange
CLI command. Bad request shapes return gRPC InvalidArgument.
Includes role_event_query_acceptance_test.go covering both phases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- class.go: roleAuthorizationsForEntry now fails fast (panic) on a store/read error resolving a registry class, instead of silently downgrading to the weaker params/legacy authorization tier. - genesis.go: GenesisState.Validate now verifies that every entry's registry_class_id references a registry class present in genesis, preventing an invalid state that would change authorization behavior at runtime. - msg_server.go SetRoles: authorize and resolve event signers against the additions (additions(current, desired)) rather than the full desired set, so ASSIGNMENT_NEW resolves only newly-added addresses (matches the pending-apply path) and no longer errors when the desired set contains current+new members. - cli/tx.go CmdUpdateParams: submit MsgUpdateParams wrapped in a governance proposal via provcli.GenerateOrBroadcastTxCLIAsGovProp + gov prop flags, matching the documented governance-only workflow used by other modules. Adds regression tests: genesis unknown/known class-id references and a SetRoles ASSIGNMENT_NEW additions scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…policy engine Add unit tests for RegistryClass/authorization config validation (every rejection path), the RegistryClass/RegistryClasses/Params query RPCs, keeper class store accessors, the policy engine's fail-closed branches, and revoke EventRoleUpdated emission. Raises keeper coverage 71% -> 79%. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update ledger keeper test call sites to the 4-arg CreateRegistry signature (pass "" for no registry class), fixing the test compile break. Clarify the registry_class_id proto comment to document the module-params default tier before the legacy NFT-owner fallback, and regenerate the pb.go. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lidation) emitRoleUpdated now panics if the post-mutation registry read-back errors, rather than silently emitting an EventRoleUpdated with empty current addresses (a nil entry remains valid for a cleaned-up registry). Validate QueryRegistryClassRequest.registry_class_id against the defined id format so malformed ids are rejected at request validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the company-specific "DART" product name with generic language throughout the registry module. Rename the acceptance test suite and example fixture, genericize identifiers (loan-registry-v1 class id, participant role policy builders), the spec README, and the proto/pb.go doc comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The validate-role-change CLI documented string enum values (e.g. "REGISTRY_ROLE_CONTROLLER") but used encoding/json, which only accepts numeric enum values for the int32-backed role field. Decode the role_updates argument through the proto-JSON codec (wrapped in the request message) so the documented string enum names work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the company-internal ticket identifier (sc-512248) and generic "ticket" mentions from registry test suite comments, the example spec README, and the ControllerRoleAuthorizations doc comment, replacing them with generic language. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A registry entry referencing a registry_class_id only verified the class existed, not that the class's asset_class_id matched the entry's. A class is scoped to a single asset class, so a mismatch would resolve role authorization against the wrong policy tier across unrelated collections. Enforce the match at every write/read site: RegisterNFT, RegistryBulkUpdate, GenesisState.Validate, and (defense-in-depth, fail-closed) the runtime roleAuthorizationsForEntry resolver. Add ErrRegistryClassAssetMismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xample The ValidateRoleChange REST endpoint was annotated as GET with path params, but the request carries role_updates and approvers lists that cannot be supplied via a query string (and the server rejects empty role_updates), making the REST route unusable. Switch it to POST with body "*" so clients can submit the full request message as JSON. Also make the create-registry-class help JSON example use consistent snake_case proto field names (registry_class_id) to match the generated example fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roleAuthorizationsForEntry now panics when an entry references a registry class that is absent from state, instead of silently downgrading to the params/legacy authorization tier. CreateRegistry enforces the registry class invariant (exists and matches the entry's asset class) at the keeper layer and rejects a nil key, preventing invalid state from direct keeper callers that bypass the msg/genesis paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
feat(registry): add registry classes for asset-class-level authorization
Legal RiskThe following dependencies were released under a license that RecommendationWhile merging is not directly blocked, it's best to pause and consider what it means to use this license before continuing. If you are unsure, reach out to your security team or Semgrep admin to address this issue. CC-BY-SA-4.0 GPL-3.0
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Overall, the PR looks good, but I have a few notes.
|
…ding on delete RegistryBulkUpdate previously only checked NFT ownership (or hard-coded authority addresses) before writing role sets. This allowed the bulk path to bypass the same multi-party authorization policies enforced by GrantRole, RevokeRole, and SetRoles. Non-authority signers updating an existing entry now have each role change validated through ValidateRoleChangeAuthorization. DeleteRegistry now removes all PendingRoleChange records for the deleted key so that stale approvals cannot be replayed if the same NFT is later re-registered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
An NFT owner could bypass multi-party role policies by unregistering and re-registering the NFT with new roles. UnregisterNFT now checks whether a CONTROLLER is set on the registry entry; if so, the signer must also be in the controller list. Ownership alone is insufficient when a controller is present. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A second ProposeRoleChange for a different set of role updates is now rejected when a pending change already exists for the same NFT key. This prevents conflicting proposals from accumulating and avoids ambiguity about which change should be applied. Re-proposing the identical role updates (same deterministic ID) is still permitted as a co-approval path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds two new capabilities requested in review feedback: Cancel: A new MsgCancelRoleChange endpoint lets the original proposer of a pending role change remove it at any time. Non-proposers are rejected. Emits EventRoleChangeCancelled on success. Expiry: A new pending_change_expiry Duration param controls how long a pending role change remains valid. When set, the expires_at timestamp is recorded on the PendingRoleChange at proposal time. ApproveRoleChange rejects and cleans up any change that has passed its expiry. A zero value (the default) preserves the existing behavior of no expiry. Proto changes: MsgCancelRoleChange/Response and CancelRoleChange RPC in tx.proto; EventRoleChangeCancelled in events.proto; expires_at field on PendingRoleChange in registry.proto; pending_change_expiry field on Params in params.proto. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BulkUpdate wrong address set: ValidateRoleChangeAuthorization now receives only the newly added addresses via additions(), matching the semantics of GrantRole and SetRoles. Passing the full desired list was incorrectly treating unchanged holders as incoming assignees under ASSIGNMENT_NEW* policies. Unregister deadlocks split owner/controller: The previous fix required the signer to be both the NFT owner and the controller, making unregistration impossible when the two are different accounts. Now the controller signs when one is set (NFT ownership not required), and the NFT owner signs only when no controller is set. Expiry bypass via propose: ProposeRoleChange now checks expiry when reusing an existing pending record (co-approval path), closing the bypass where MsgProposeRoleChange could apply a change after its expiry. Expired pending blocks proposals: ProposeRoleChange now cleans up expired pending records eagerly instead of treating them as live blockers, so a lapsed change no longer prevents new proposals indefinitely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tries to use a registry class Adds a new MsgAssociateRegistryClass message that allows an existing legacy registry entry (one created without a registry_class_id) to be associated with a registry class without modifying its roles. Authorization: either the CONTROLLER role address(es) OR the scope data-owner parties (if the NFT is a Provenance Metadata Scope) OR the NFT owner — no precedence, any one is sufficient. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit f3310ac. Configure here.
…PE_OWNER validateAssociateRegistryClassSigner was accepting any address in scope.Owners regardless of party role, which could allow non-data-owner parties (e.g. PARTY_TYPE_ORIGINATOR, PARTY_TYPE_SERVICER) to associate a registry class. Now requires party.Role == PARTY_TYPE_OWNER. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was created solely so that Bugbot can review the code. This PR will not be merged here.
Description
This pull request introduces a major enhancement to the
registrymodule by establishing a robust, multi-tiered framework for multi-party authorization and role-based permissions. It combines a new single-signer approval-accumulation model for executing multi-party authorizations with Registry Class policies for asset class-level (NFT collection) rule management.Together, these changes make it easier to define, manage, and execute custom authorization policies for NFT collections without requiring governance proposals for every change, while remaining fully compatible with Cosmos SDK
authz.Key Changes & Features
1. Multi-Party Authorization & Approval Accumulation
Because Cosmos SDK
authzMsgExeconly dispatches single-signer messages, this update introduces a single-signer approval-accumulation model rather than literal multi-signer messages.MsgProposeRoleChangeandMsgApproveRoleChange. Each message takes exactly one signer. Approvals accumulate in the registry state on aPendingRoleChange(keyed by a deterministic ID).Keeper.SetRoles. Ineligible approvals are safely ignored to bound state.MsgExec.MsgGrantRoleandMsgRevokeRoleremain single-signer; changes requiring multi-party authorization must use the new propose/approve flow.2. Registry Class Policies (Asset Class-Level)
Introduces the ability to define asset class-level authorization rules, allowing maintainers to set custom role update policies for entire NFT collections.
RegistryClassmessage to define these class-level authorization rules (authorization.proto). Added transaction endpoints for creating and updating registry classes.RegistryEntrycan now reference aregistry_class_idto determine its authorization policy for role updates (registry.proto). Support for associating new entries with a class during registration (MsgRegisterNFT) has also been added.3. Authorization Policy Engine & Roles
Establishes the underlying engine for evaluating complex role policies.
RegistryRoleenum values:LIEN_OWNER,SECURED_PARTY_FOR_LIEN,SECURED_PARTY_FOR_ENOTE, andPLEDGEE. AddedMsgSetRoles/RoleUpdatefor atomic, desired-state multi-role updates (used for the fast path and as the proposal payload).authorization.protodefiningSignatureType,NftRole,Assignment,RoleAuthorization,Authorization,SignatureRequirement,RoleAssignment,RolePriority, andRolePriorityEntry.DefaultRoleAuthorizations(),RoleAuthorizationMap()) and the evaluation engine inkeeper/authorization.gocontaining address resolvers, signature requirement evaluators, and policy approver collection (ValidateRoleChangeAuthorization,CollectPolicyApprovers).4. Governance & Module Parameters
Paramsmessage to define module-wide default authorization policies, which serve as a fallback when noregistry_class_idis set.MsgUpdateParams) to update registry module parameters.5. Queries, CLI, and Auditability
pending-role-changes), and for validating role changes against policies.propose-role-changeandapprove-role-change.Note
High Risk
Changes core on-chain authorization for registry role updates, unregister rules, and bulk updates; bugs could allow unauthorized role changes or block legitimate updates.
Overview
Major expansion of the
x/registrymodule so role changes are governed by configurable policies instead of NFT-owner-only checks, with an authz-friendly propose/approve path for multi-party consent.Registry classes & resolution tier: New
RegistryClasstypes and msgs (CreateRegistryClass,UpdateRegistryClassRoleAuthorization) define per–asset-classRoleAuthorizationrules. Entries gain optionalregistry_class_id(on register or viaMsgAssociateRegistryClass), with moduleParamsas the middle tier and legacy NFT-owner auth when no policy applies. Association is allowed for CONTROLLER, scope data owner (metadata scopes), or NFT owner—any one suffices.Policy engine:
authorization.protoand keeper logic evaluate signature requirements (ALL/ANY, CURRENT/NEW assignments, role priority, NFT owner role) with fail-closed behavior on misconfiguration.GrantRole,RevokeRole,SetRoles, and bulk updates on existing entries route through this engine;UnregisterNFTrequires CONTROLLER when that role is set.Multi-party flow:
MsgProposeRoleChange/MsgApproveRoleChange/MsgCancelRoleChangeaccumulate single-signer approvals onPendingRoleChange(genesis-exportable, one pending change per NFT, optional expiry), then apply atomically viaSetRoles. New participant roles (lien/eNote/pledgee-related) and richerEventRoleUpdated/ pending-class events support auditability.Surface area: New gRPC queries (pending changes, classes, params,
ValidateRoleChangedry-run), CLI tx/query commands, andCreateRegistry(..., registryClassID)call-site updates in ledger tests. Proto Java/Kotlin bindings temporarily stop excludinggoogle/**protos.Reviewed by Cursor Bugbot for commit 0dd4900. Bugbot is set up for automated code reviews on this repo. Configure here.