feat: implement storage configuration and admin management#503
Open
OtowoSamuel wants to merge 1 commit into
Open
feat: implement storage configuration and admin management#503OtowoSamuel wants to merge 1 commit into
OtowoSamuel wants to merge 1 commit into
Conversation
- Create StorageKey enum for centralized storage key management (ANYTECHS#491) - Implement owner storage with getter/setter operations (ANYTECHS#493) - Add administrator storage with add/remove/check functionality (ANYTECHS#494) - Add default royalty configuration with BPS validation (ANYTECHS#469) Features: - Centralized StorageKey enum prevents duplicate keys - Owner-based authorization for all write operations - Admin vector with duplicate prevention - Royalty BPS validation (0-10000 range) - Comprehensive unit tests included Closes ANYTECHS#491 Closes ANYTECHS#493 Closes ANYTECHS#494 Closes ANYTECHS#469
|
@OtowoSamuel Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Soroban-based clips_nft contract crate that centralizes storage keys and adds on-chain configuration/state for owner, administrators, and default royalty basis points (BPS).
Changes:
- Adds
StorageKeyenum plus persistent storage helpers for owner, admins, and royalty BPS. - Exposes contract methods for owner transfer, admin management, and royalty get/set with BPS validation.
- Adds initial README documentation and unit tests for the new behaviors.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
clips_nft/src/lib.rs |
Implements storage keys, owner/admin/royalty storage + contract entrypoints and tests. |
clips_nft/README.md |
Documents storage layout and the new public contract methods. |
clips_nft/Cargo.toml |
Introduces the new contract crate configuration and Soroban SDK dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+62
| if let Some(pos) = admins.iter().position(|a| a == admin) { | ||
| admins.remove(pos); | ||
| env.storage().persistent().set(&StorageKey::Admin, &admins); | ||
| } |
Comment on lines
+110
to
+112
| fn panic_with_error(env: &Env, msg: &str) -> ! { | ||
| env.panic_with_error(msg.into()); | ||
| } |
Comment on lines
+124
to
+131
| pub fn initialize(env: Env, owner: Address, royalty_bps: u32) { | ||
| owner.require_auth(); | ||
|
|
||
| // Validate royalty BPS | ||
| validate_bps(royalty_bps); | ||
|
|
||
| // Initialize owner | ||
| save_owner(&env, &owner); |
| @@ -0,0 +1,268 @@ | |||
| #![no_std] | |||
Comment on lines
+203
to
+216
| fn test_owner_management() { | ||
| let env = soroban_sdk::Env::default(); | ||
| let owner = soroban_sdk::Address::generate(&env); | ||
|
|
||
| ClipsNftContract::initialize(env.clone(), owner.clone(), 500); | ||
|
|
||
| let retrieved_owner = ClipsNftContract::get_owner(env.clone()); | ||
| assert_eq!(retrieved_owner, owner); | ||
|
|
||
| let new_owner = soroban_sdk::Address::generate(&env); | ||
| ClipsNftContract::set_owner(env.clone(), new_owner.clone()); | ||
| let retrieved_owner = ClipsNftContract::get_owner(env.clone()); | ||
| assert_eq!(retrieved_owner, new_owner); | ||
| } |
Comment on lines
+219
to
+241
| fn test_admin_management() { | ||
| let env = soroban_sdk::Env::default(); | ||
| let owner = soroban_sdk::Address::generate(&env); | ||
| let admin1 = soroban_sdk::Address::generate(&env); | ||
| let admin2 = soroban_sdk::Address::generate(&env); | ||
|
|
||
| ClipsNftContract::initialize(env.clone(), owner.clone(), 500); | ||
|
|
||
| // Owner should be admin by default | ||
| assert!(ClipsNftContract::is_administrator(env.clone(), owner.clone())); | ||
|
|
||
| // Add admins | ||
| ClipsNftContract::add_administrator(env.clone(), admin1.clone()); | ||
| assert!(ClipsNftContract::is_administrator(env.clone(), admin1.clone())); | ||
|
|
||
| // Add another admin | ||
| ClipsNftContract::add_administrator(env.clone(), admin2.clone()); | ||
| assert!(ClipsNftContract::is_administrator(env.clone(), admin2.clone())); | ||
|
|
||
| // Remove admin | ||
| ClipsNftContract::remove_administrator(env.clone(), admin1.clone()); | ||
| assert!(!ClipsNftContract::is_administrator(env.clone(), admin1.clone())); | ||
| } |
Comment on lines
+244
to
+257
| fn test_royalty_configuration() { | ||
| let env = soroban_sdk::Env::default(); | ||
| let owner = soroban_sdk::Address::generate(&env); | ||
|
|
||
| ClipsNftContract::initialize(env.clone(), owner.clone(), 500); | ||
|
|
||
| let royalty = ClipsNftContract::get_royalty_bps(env.clone()); | ||
| assert_eq!(royalty, 500); | ||
|
|
||
| // Update royalty to 1000 (10%) | ||
| ClipsNftContract::set_royalty_bps(env.clone(), 1000); | ||
| let royalty = ClipsNftContract::get_royalty_bps(env.clone()); | ||
| assert_eq!(royalty, 1000); | ||
| } |
Comment on lines
+259
to
+267
| #[test] | ||
| #[should_panic] | ||
| fn test_invalid_bps() { | ||
| let env = soroban_sdk::Env::default(); | ||
| let owner = soroban_sdk::Address::generate(&env); | ||
|
|
||
| // Should panic with BPS > 10000 | ||
| ClipsNftContract::initialize(env.clone(), owner.clone(), 10001); | ||
| } |
Comment on lines
+9
to
+10
| [lib] | ||
| crate-type = ["cdylib"] |
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.
Description
This PR implements all storage configuration and admin management features for the ClipCash NFT contract.
Changes
Features
✅ Centralized
StorageKeyenum prevents duplicate keys✅ Owner-based authorization for all write operations
✅ Admin vector with automatic duplicate prevention
✅ Royalty BPS validation enforces valid range
✅ Comprehensive unit tests included
Contract Methods
Owner Management
get_owner()- Retrieve current ownerset_owner(new_owner)- Transfer ownership (owner auth required)Administrator Management
add_administrator(admin)- Add admin account (owner auth required)remove_administrator(admin)- Remove admin account (owner auth required)is_administrator(admin)- Check if address is adminRoyalty Configuration
get_royalty_bps()- Get default royalty percentageset_royalty_bps(bps)- Set royalty (owner auth required, 0-10000 valid range)Acceptance Criteria ✅
Testing
All functionality has been tested with unit tests covering:
Closes #469
Closes #491
Closes #493
Closes #494