EN-33 Add role management, ownership transfer, and contract upgradeability by marwen-abid · Pull Request #2 · stellar/debit-card-reference

marwen-abid · 2026-04-14T00:09:47Z

Add get_owner() and get_pauser() query functions to expose the two top-level roles that were previously only readable internally.
Add set_owner() for single-signer ownership transfer, gated by current owner auth. Not blocked by pause state.
Add set_pauser() with dual-auth: callable by the current owner or the current pauser. When the owner calls it, the pause guard is skipped — this is the critical recovery path for a compromised pauser key that has frozen the contract. When the pauser calls it, require_not_paused is enforced.
Add upgrade() on the factory (owner-gated) to replace WASM bytecode in-place via Soroban's native update_current_contract_wasm.
Add upgrade() on the issuer contract (factory-gated) and upgrade_issuer() on the factory so the owner can upgrade deployed issuer contracts through the factory.
New audit events: OwnerUpdated, PauserUpdated, ContractUpgraded, IssuerUpgraded.

Copilot

Pull request overview

This PR introduces explicit role management (owner/pauser), controlled role rotation, and upgradeability for the factory and deployed issuer contracts, along with new audit events to improve observability of administrative actions.

Changes:

Added get_owner / get_pauser queries plus set_owner and dual-authority set_pauser rotation logic on the factory.
Added upgrade entrypoints: upgrade (factory self-upgrade), upgrade_issuer (factory-triggered issuer upgrades), and upgrade on the issuer (factory-gated).
Added new audit events (OwnerUpdated, PauserUpdated, ContractUpgraded, IssuerUpgraded) and expanded test coverage for role transitions and upgrade authorization.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`contracts/issuer/src/lib.rs`	Adds factory-gated in-place WASM upgrade entrypoint for issuer contracts.
`contracts/factory/src/lib.rs`	Adds role query/rotation APIs and factory + issuer upgrade entrypoints; emits new audit events.
`contracts/factory/src/events.rs`	Defines new contract events for role updates and upgrades.
`contracts/factory/src/test.rs`	Adds tests for new role APIs and upgrade authorization/success paths.
`contracts/factory/Makefile`	Adjusts the expected location of the built factory WASM artifact.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T00:31:44Z

contracts/factory/src/lib.rs

+        let issuer = IssuerClient::new(&env, &issuer_contract_address);
+        issuer.upgrade(&new_wasm_hash);
+
+        IssuerUpgraded {
+            issuer_id,
+            token,
+            new_wasm_hash,
+        }
+        .publish(&env);


upgrade_issuer emits an IssuerUpgraded audit event, but current tests only cover auth and success/failure paths and do not verify the event payload. Consider adding an event assertion to ensure issuer_id, token, and new_wasm_hash are logged correctly.

Copilot · 2026-04-14T00:31:44Z

contracts/factory/src/lib.rs

+    /// # Arguments
+    /// * `env` - Contract environment.
+    /// * `caller` - Address invoking the operation (must be the current owner or pauser).
+    /// * `new_pauser` - Address to receive the pauser role.
+    ///
+    /// # Authorization
+    /// Requires authorization from `caller`, who must be the current owner or pauser.
+    pub fn set_pauser(env: Env, caller: Address, new_pauser: Address) {
+        let current_owner = owner(&env);
+        let current_pauser = pauser(&env);
+
+        if caller == current_owner {
+            caller.require_auth();
+        } else if caller == current_pauser {
+            require_not_paused(&env);
+            caller.require_auth();
+        } else {
+            panic_with_error!(&env, FactoryError::NotAuthorized);
+        }


set_pauser requires the caller to pass a caller argument, and the function’s behavior (pause guard bypass vs. enforced) depends on that parameter rather than the actual invoker. This is easy for clients to misuse and allows a transaction signed by the owner to bypass the pause guard even if the invoker is the pauser (or vice versa). Consider deriving the caller from the environment (e.g., the immediate invoker) and branching based on whether that invoker equals the stored owner or pauser, then requiring auth for that same invoker.

We can split it into two functions, or remove caller and check auth for both roles.

Copilot · 2026-04-14T00:31:44Z

contracts/factory/Makefile

 build:
 	stellar contract build
-	@ls -l target/wasm32v1-none/release/*.wasm
+	@ls -l ../target/wasm32v1-none/release/factory.wasm


The Makefile now checks for the built WASM at ../target/.../factory.wasm, but other build-time references in this crate (e.g., contract WASM imports in tests) are relative to a target directory under contracts/factory. Unless the build pipeline has been switched to a workspace-level target dir, this path change can make make build report a missing artifact even when the build succeeded. Align the ls path with the actual target dir used by your build/test tooling.

Suggested change

@ls -l ../target/wasm32v1-none/release/factory.wasm

@ls -l target/wasm32v1-none/release/factory.wasm

Copilot · 2026-04-14T00:31:45Z

contracts/factory/src/lib.rs

+    pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) {
+        owner(&env).require_auth();
+
+        env.deployer()
+            .update_current_contract_wasm(new_wasm_hash.clone());
+
+        ContractUpgraded { new_wasm_hash }.publish(&env);
+    }


upgrade emits a ContractUpgraded audit event, but there is no test asserting that the event is emitted with the expected WASM hash. Adding an event assertion (similar to the OwnerUpdated / PauserUpdated tests) would help prevent regressions in audit logging.

JiahuiWho · 2026-04-14T13:28:48Z

contracts/factory/src/lib.rs

+    pub fn set_owner(env: Env, new_owner: Address) {
+        let current_owner = owner(&env);
+        current_owner.require_auth();
+
+        storage::set_owner(&env, &new_owner);
+
+        OwnerUpdated {
+            old_owner: current_owner,
+            new_owner,
+        }
+        .publish(&env);
+    }


set_owner only requires auth from the current owner. If an incorrect new_owner is passed, ownership is irrevocably lost.

Bridge's own Solana contract (update_admin) mitigates this by requiring both the current and new admin to sign the same transaction. The equivalent here would be adding new_owner.require_auth()

Just something to consider.

JiahuiWho · 2026-04-14T13:35:18Z

contracts/issuer/src/lib.rs

+        let factory: Address = env
+            .storage()
+            .instance()
+            .get(&DataKey::Factory)
+            .expect("factory not set");
+        factory.require_auth();


Worth extracting into a helper function?

EN-33 Add Ability to Edit Owner, edit Pauser and Upgrade Card Contract

f792c0b

marwen-abid self-assigned this Apr 14, 2026

marwen-abid requested a review from Copilot April 14, 2026 00:27

Copilot started reviewing on behalf of marwen-abid April 14, 2026 00:27 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

JiahuiWho reviewed Apr 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EN-33 Add role management, ownership transfer, and contract upgradeability#2

EN-33 Add role management, ownership transfer, and contract upgradeability#2
marwen-abid wants to merge 1 commit intodevelopfrom
feat/EN-33-edit-owner-and-pauser

marwen-abid commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

JiahuiWho Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

JiahuiWho Apr 14, 2026

Uh oh!

JiahuiWho Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	@ls -l ../target/wasm32v1-none/release/factory.wasm
	@ls -l target/wasm32v1-none/release/factory.wasm

Conversation

marwen-abid commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

JiahuiWho Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

JiahuiWho Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

JiahuiWho Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants