feat(security): add issuer role separation controls - #177
Merged
El-swaggerito merged 1 commit intoJul 29, 2026
Conversation
Contributor
| \nThis PR is currently blocked by merge conflicts.\n\nPlease update the branch with the latest main branch and resolve the conflicts before it can be merged. |
Adds separation-of-duties controls for issuance, so a deployment can require that clearing an investor and funding that investor be two different keys. - src/issuer.rs: duty model (Compliance / Issuance / Emergency / Governance) derived from what the contract enforces, an opt-in IssuerSeparationPolicy with three independent controls (dual-duty, self-issuance, independent-approver), the non-panicking guard both mint_asset and the pre-flight read share, and admin-governed policy updates. - DataKey::ComplianceApprover records who last moved an address into Approved, written by every approval path; a revocation does not erase it. - New errors 3007 IssuanceDutyConflict, 3008 SelfIssuanceForbidden, 3009 IssuanceApproverConflict; new event issuer_separation_policy_updated carrying both the previous and new policy. - Default policy is fully permissive, so no existing deployment's behaviour changes; the policy setter is not gated by the policy it sets, so the strictest configuration can always be recovered from. - Corrects docs/admin-roles.md: EmergencyOfficer was listed as able to mint_asset / distribute_yield, but both call require_role(AssetManager) and have always rejected it. The duty table now matches the contract. - 16 tests, including a five-policy x four-caller x two-recipient matrix asserting the pre-flight read and the real mint always agree. - docs/issuer-role-separation.md with the duty map, controls, assumptions, and a recommended operating model; README, admin-roles, error-codes, events, capabilities (schema v4), and SDK fixtures updated.
Fury03
force-pushed
the
feat/issuer-role-separation
branch
from
July 29, 2026 15:02
2ee3296 to
c84ba99
Compare
Contributor
Author
|
Rebased onto
Post-rebase verification: |
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
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.
Summary
Closes #28
The role model answers which privileges an address holds. It does not answer the question an RWA auditor asks first: can the same key both decide who may hold the asset and decide who receives it?
Until now the answer was yes. The supreme admin bypasses every role check, so a single admin key can approve an address and then mint to it — no second party involved, and nothing in the contract to notice. That is the control failure behind self-allocation and fictitious-holder issuance fraud: not a bug in any one function, but the absence of a constraint between two correctly-functioning ones.
This PR adds that constraint as an opt-in policy, so no existing deployment's behaviour changes until an admin enables it.
Duties, not role names
The duty map is derived from what the contract enforces, not from what a role is named after:
ComplianceOfficerAssetManagerEmergencyOfficerAdminThe consequence: today the admin is the only address carrying both duties, so the dual-duty control is in practice what forces an admin to delegate issuance to a dedicated
AssetManagerkey. It is written against duties rather than against "the admin" so it keeps working if a future role combines them.The controls
allow_dual_duty_issuance: falseIssuanceDutyConflict(3007)allow_self_issuance: falseSelfIssuanceForbidden(3008)require_independent_approver: trueIssuanceApproverConflict(3009)Each is independent, so an issuer adopts only the parts their operating model supports.
DataKey::ComplianceApproverrecords who last moved an address intoApproved— written byset_compliance_status,whitelist_user, and batch updates; a revocation does not erase it, since revoking clearance should not erase who granted the clearance being revoked.Never self-locking.
set_issuer_separation_policyis deliberately not gated by the policy it sets, so the strictest configuration is always recoverable — verified bytest_separation_policy_can_never_lock_a_deployment_out_of_issuance.Pre-flight read.
check_issuance_authority(caller, recipient)returns the verdict from the same evaluationmint_assetenforces, along with the caller's effective role and duties and the recipient's recorded approver, so a dashboard can explain a refusal rather than just report one.Notes for the reviewer
mainare fixed here so the suite is green. Unrelated to this issue and kept minimal:fixtures/sdk/01-compliance.jsonhad drifted, andfixture_errorsassertedAssetNotActive(6000) /AssetLifecyclePaused(6001) for mints the contract already reports as7002/7000—docs/error-codes.mdalready documents 6000 as "Reserved. Superseded by 7000–7002; no longer emitted", so the expectations were stale, not the contract.cargo fmtalso normalizedsrc/config.rs/src/config_test.rs, unformatted onmain. The same three fixes appear in the PR for Add compliance status transition guards #27; whichever merges second can drop them.CAPABILITY_SCHEMA_VERSION3 → 4. The PR for Add compliance status transition guards #27 does the same; whichever merges second should become 5.Test Evidence
test_check_issuance_authority_matches_mint_enforcementis the load-bearing one: five policies × four caller classes × two recipients, each on a fresh deployment, asserting the pre-flight verdict and the real mint agree on outcome, error code, and resulting balance. Every recipient in that matrix is compliance-approved first, so the only thing that can refuse a mint there is a separation control.Completion Table
src/issuer.rs—IssuerDuty/role_has_duty()(duty map),IssuerSeparationPolicy(three controls),evaluate_issuance()/require_issuance_authority()(guard, called frommint_assetanddistribute_yield),set_issuer_separation_policy()(governance)test_role_duty_table_is_exact,test_dual_duty_issuance_is_refused_when_separation_is_enforced,test_self_issuance_is_refused_when_disallowed,test_independent_approver_control_enforces_four_eyesdocs/issuer-role-separation.mdaddedtest_issuance_guard_reports_not_initialized_instead_of_panicking,test_policy_update_is_blocked_while_paused,test_missing_issuance_duty_is_reported_separately_from_a_separation_failure,test_approver_record_tracks_every_approval_path,test_yield_distribution_respects_the_duty_control_only,test_separation_policy_can_never_lock_a_deployment_out_of_issuance,test_separation_controls_are_independentdocs/issuer-role-separation.mdtest_issuance_reads_never_mutate_state,test_policy_update_is_admin_only_and_emits_the_previous_policydocs/issuer-role-separation.md; cross-links toadmin-misuse-risks.md/threat-model.mdISSUER ROLE SEPARATIONinsrc/test.rs; 2 new SDK fixture scenarios; a recommended operating model reviewers can check againstcargo test196 passed;fixtures/sdk/05-errors.json—error-3007-issuance-duty-conflict;fixtures/sdk/04-events.json—event-issuer-separation-policy-updateddocs/issuer-role-separation.mdadmin-roles.mdgains a Separation of Duties section and the corrected role tableREADME.md,docs/admin-roles.md,docs/error-codes.md(3007–3009 + client guidance),docs/events.md(new topic)minting.issuer_separationcapability +issuer_separationregistry key +issuer_separation_enforcedruntime flag;IssuerDuty/IssuanceGuard/IssuerSeparationPolicydocumented as append-only ABIdocs/capabilities.md(fields, registry key, version note)Contributor Self-Assessment
admin-roles.mdcorrection and the three pre-existing repo fixes are called out above.fmt-check,clippy(no new warnings from the new module),test, and the wasm release build all pass locally.