feat: implement treasury tracking and withdrawal functionality#54
Open
sudo-robi wants to merge 3 commits intoHubDApp:mainfrom
Open
feat: implement treasury tracking and withdrawal functionality#54sudo-robi wants to merge 3 commits intoHubDApp:mainfrom
sudo-robi wants to merge 3 commits intoHubDApp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Implements on-chain treasury balance tracking and admin-controlled withdrawals in the Soroban-based Dongle smart contract, alongside broader refactors to storage keys, types, events, and tests/snapshots.
Changes:
- Added treasury storage keys, per-token treasury balance tracking, and withdrawal plumbing in
FeeManager/ contract API. - Implemented verification record persistence and sync of project verification status.
- Refactored review storage to persist reviews and list reviewers per project; updated tests and snapshots; bumped
soroban-sdkto v22.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| dongle-smartcontract/src/lib.rs | Exposes public contract API for project/review/verification/fee+treasury actions; wires registries/managers together. |
| dongle-smartcontract/src/fee_manager.rs | Implements fee configuration, fee collection into contract, treasury balance tracking, and withdrawals. |
| dongle-smartcontract/src/storage_keys.rs | Adds treasury-related keys and additional keys for migrated project/review indexing. |
| dongle-smartcontract/src/events.rs | Adds fee/treasury/verification event publishers and related event symbols. |
| dongle-smartcontract/src/verification_registry.rs | Persists verification records, publishes verification events, and syncs project verification status. |
| dongle-smartcontract/src/project_registry.rs | Implements persistent project registration/update/listing and verification status updates. |
| dongle-smartcontract/src/review_registry.rs | Persists reviews and maintains a per-project reviewer index for retrieving all reviews. |
| dongle-smartcontract/src/types.rs | Expands Project and VerificationRecord schemas; adds timestamps; extends VerificationStatus. |
| dongle-smartcontract/src/errors.rs | Adds new error variants related to treasury and fee configuration/withdrawals. |
| dongle-smartcontract/src/utils.rs | Removes unused/placeholder admin and storage helpers; simplifies validation helper logic. |
| dongle-smartcontract/src/test.rs | Replaces prior tests with updated tests for project, review, and verification sync flows. |
| dongle-smartcontract/src/test_treasury.rs | Adds unit tests for fee configuration, fee collection, treasury withdrawal, and treasury address management. |
| dongle-smartcontract/test_snapshots/test_treasury/test_fee_configuration.1.json | Snapshot for fee configuration test behavior. |
| dongle-smartcontract/test_snapshots/test_treasury/test_verification_fee_collection.1.json | Snapshot for verification fee collection affecting treasury balance. |
| dongle-smartcontract/test_snapshots/test_treasury/test_insufficient_treasury_funds.1.json | Snapshot for insufficient-funds withdrawal attempt. |
| dongle-smartcontract/test_snapshots/test_treasury/test_unauthorized_withdrawal.1.json | Snapshot for unauthorized withdrawal attempt. |
| dongle-smartcontract/test_snapshots/test/test_register_project_success.1.json | Snapshot updated for project registration flow. |
| dongle-smartcontract/test_snapshots/test/test_update_project_success.1.json | Snapshot updated for project update flow. |
| dongle-smartcontract/test_snapshots/test/test_add_review_success.1.json | Snapshot updated for review submission flow. |
| dongle-smartcontract/test_snapshots/test/test_get_project_reviews.1.json | Snapshot updated for retrieving reviews by project. |
| dongle-smartcontract/test_snapshots/test/test_verification_status_sync.1.json | Snapshot updated for verification status synchronization between registries. |
| dongle-smartcontract/test_snapshots/review_registry/test/test_add_review_event.1.json | Snapshot updated for review submission event behavior. |
| dongle-smartcontract/test_snapshots/review_registry/test/test_update_review_event.1.json | Snapshot updated for review update event behavior. |
| dongle-smartcontract/test_snapshots/review_registry/test/test_delete_review_event.1.json | Snapshot updated for review delete event behavior. |
| dongle-smartcontract/Cargo.toml | Bumps soroban-sdk to v22 and enables rlib output for tests. |
| dongle-smartcontract/build_errors_2.txt | Adds a local build log file to the repo. |
| dongle-smartcontract/target/.rustc_info.json | Adds a build artifact under target/ to the repo. |
Comments suppressed due to low confidence (2)
dongle-smartcontract/src/review_registry.rs:59
update_reviewdoes not callreviewer.require_auth()and silently does nothing if the review is missing. Require reviewer auth and returnReviewNotFoundwhenStorageKey::Review(project_id, reviewer)is absent so callers can’t update others’ reviews and can detect failures.
.storage()
.persistent()
.get(&DataKey::UserReviews(reviewer.clone()))
.unwrap_or(soroban_sdk::Vec::new(&env));
user_reviews.push_back(project_id);
dongle-smartcontract/src/review_registry.rs:16
add_reviewnever callsreviewer.require_auth(), so anyone can submit a review on behalf of any address. Require the reviewer’s auth before storing the review/event.
#[contractimpl]
impl ReviewRegistry {
pub fn add_review(
env: Env,
project_id: u64,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
74
to
+78
| rating: u32, | ||
| comment_cid: Option<String>, | ||
| ) { | ||
| let key = StorageKey::Review(project_id, reviewer.clone()); | ||
|
|
There was a problem hiding this comment.
delete_review doesn’t call require_auth(), so anyone can delete another user’s review by passing their address. Require reviewer.require_auth() and return ReviewNotFound if the key is absent.
Comment on lines
+58
to
+69
| env.events() | ||
| .publish((VER_REQ, requester, project_id), ()); | ||
| } | ||
|
|
||
| pub fn publish_verification_approved_event(env: &Env, project_id: u64) { | ||
| env.events() | ||
| .publish((VER_APP, project_id), ()); | ||
| } | ||
|
|
||
| pub fn publish_verification_rejected_event(env: &Env, project_id: u64) { | ||
| env.events() | ||
| .publish((VER_REJ, project_id), ()); |
There was a problem hiding this comment.
Verification events publish empty data () here, which limits off-chain monitoring/auditability. Consider including at least timestamp and evidence CID (for requests) and the actor address (requester/admin) in the event payload.
Suggested change
| env.events() | |
| .publish((VER_REQ, requester, project_id), ()); | |
| } | |
| pub fn publish_verification_approved_event(env: &Env, project_id: u64) { | |
| env.events() | |
| .publish((VER_APP, project_id), ()); | |
| } | |
| pub fn publish_verification_rejected_event(env: &Env, project_id: u64) { | |
| env.events() | |
| .publish((VER_REJ, project_id), ()); | |
| let timestamp = env.ledger().timestamp(); | |
| env.events() | |
| .publish((VER_REQ, requester.clone(), project_id), (timestamp, requester)); | |
| } | |
| pub fn publish_verification_approved_event(env: &Env, project_id: u64) { | |
| let timestamp = env.ledger().timestamp(); | |
| let actor = env.invoker(); | |
| env.events() | |
| .publish((VER_APP, project_id), (timestamp, actor)); | |
| } | |
| pub fn publish_verification_rejected_event(env: &Env, project_id: u64) { | |
| let timestamp = env.ledger().timestamp(); | |
| let actor = env.invoker(); | |
| env.events() | |
| .publish((VER_REJ, project_id), (timestamp, actor)); |
Comment on lines
+193
to
+197
| VerificationRegistry::get_verification(&env, project_id) | ||
| } | ||
|
|
||
| // ========================================== | ||
| // FEE & TREASURY MANAGEMENT FUNCTIONS |
There was a problem hiding this comment.
withdraw_treasury exposes a caller-specified to address in the public contract API. If withdrawals are meant to go to a configured treasury destination, consider removing to from the contract interface and always using the stored treasury address (erroring if unset).
| // INITIALIZATION & ADMIN FUNCTIONS | ||
| // ========================================== | ||
|
|
||
| pub fn initialize(env: Env, admin: Address) -> Result<(), ContractError> { |
There was a problem hiding this comment.
initialize doesn’t call require_auth() on admin, so any invoker can initialize the contract and set an arbitrary admin address. Require admin.require_auth() (or require auth from the caller) before writing StorageKey::Admin.
Suggested change
| pub fn initialize(env: Env, admin: Address) -> Result<(), ContractError> { | |
| pub fn initialize(env: Env, admin: Address) -> Result<(), ContractError> { | |
| // Require the admin address to authorize being set as admin. | |
| admin.require_auth(&env); |
| @@ -1 +1 @@ | |||
| {"rustc_fingerprint":8223297098076279531,"outputs":{"7971740275564407648":{"success":true,"status":"","code":0,"stdout":"___.exe\nlib___.rlib\n___.dll\n___.dll\nlib___.a\n___.dll\nC:\\Users\\LENOVO\\.rustup\\toolchains\\stable-x86_64-pc-windows-gnu\noff\n___\ndebug_assertions\npanic=\"unwind\"\nproc_macro\ntarget_abi=\"\"\ntarget_arch=\"x86_64\"\ntarget_endian=\"little\"\ntarget_env=\"gnu\"\ntarget_family=\"windows\"\ntarget_feature=\"cmpxchg16b\"\ntarget_feature=\"fxsr\"\ntarget_feature=\"sse\"\ntarget_feature=\"sse2\"\ntarget_feature=\"sse3\"\ntarget_has_atomic=\"128\"\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_os=\"windows\"\ntarget_pointer_width=\"64\"\ntarget_vendor=\"pc\"\nwindows\n","stderr":""},"17747080675513052775":{"success":true,"status":"","code":0,"stdout":"rustc 1.93.1 (01f6ddf75 2026-02-11)\nbinary: rustc\ncommit-hash: 01f6ddf7588f42ae2d7eb0a2f21d44e8e96674cf\ncommit-date: 2026-02-11\nhost: x86_64-pc-windows-gnu\nrelease: 1.93.1\nLLVM version: 21.1.8\n","stderr":""},"11652014622397750202":{"success":true,"status":"","code":0,"stdout":"___.wasm\nlib___.rlib\n___.wasm\nlib___.a\nC:\\Users\\LENOVO\\.rustup\\toolchains\\stable-x86_64-pc-windows-gnu\noff\n___\ndebug_assertions\npanic=\"abort\"\nproc_macro\ntarget_abi=\"\"\ntarget_arch=\"wasm32\"\ntarget_endian=\"little\"\ntarget_env=\"\"\ntarget_family=\"wasm\"\ntarget_feature=\"bulk-memory\"\ntarget_feature=\"multivalue\"\ntarget_feature=\"mutable-globals\"\ntarget_feature=\"nontrapping-fptoint\"\ntarget_feature=\"reference-types\"\ntarget_feature=\"sign-ext\"\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_os=\"unknown\"\ntarget_pointer_width=\"32\"\ntarget_vendor=\"unknown\"\n","stderr":"warning: dropping unsupported crate type `dylib` for target `wasm32-unknown-unknown`\n\nwarning: dropping unsupported crate type `proc-macro` for target `wasm32-unknown-unknown`\n\nwarning: 2 warnings emitted\n\n"}},"successes":{}} No newline at end of file | |||
| {"rustc_fingerprint":5182176766396021987,"outputs":{"7971740275564407648":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n/home/robi/.rustup/toolchains/stable-x86_64-unknown-linux-gnu\noff\npacked\nunpacked\n___\ndebug_assertions\npanic=\"unwind\"\nproc_macro\ntarget_abi=\"\"\ntarget_arch=\"x86_64\"\ntarget_endian=\"little\"\ntarget_env=\"gnu\"\ntarget_family=\"unix\"\ntarget_feature=\"fxsr\"\ntarget_feature=\"sse\"\ntarget_feature=\"sse2\"\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_os=\"linux\"\ntarget_pointer_width=\"64\"\ntarget_vendor=\"unknown\"\nunix\n","stderr":""},"17747080675513052775":{"success":true,"status":"","code":0,"stdout":"rustc 1.93.1 (01f6ddf75 2026-02-11)\nbinary: rustc\ncommit-hash: 01f6ddf7588f42ae2d7eb0a2f21d44e8e96674cf\ncommit-date: 2026-02-11\nhost: x86_64-unknown-linux-gnu\nrelease: 1.93.1\nLLVM version: 21.1.8\n","stderr":""}},"successes":{}} No newline at end of file | |||
There was a problem hiding this comment.
This file is a build artifact under target/ and is environment-specific; it should not be committed. Add dongle-smartcontract/target/ to .gitignore and remove this file from the PR.
Comment on lines
+147
to
+149
| // First pay the fee | ||
| FeeManager::pay_fee(&env, requester.clone(), project_id, "verification")?; | ||
|
|
There was a problem hiding this comment.
This flow pays the verification fee but doesn’t check that requester is the project owner for project_id. If only project owners should request verification, fetch the project and verify project.owner == requester before paying/storing the request.
Suggested change
| // First pay the fee | |
| FeeManager::pay_fee(&env, requester.clone(), project_id, "verification")?; | |
| // Ensure that the requester is the owner of the given project. | |
| let project = ProjectRegistry::get_project(env.clone(), project_id) | |
| .ok_or(ContractError::ProjectNotFound)?; | |
| if project.owner != requester { | |
| return Err(ContractError::Unauthorized); | |
| } | |
| // First pay the fee | |
| FeeManager::pay_fee(&env, requester.clone(), project_id, "verification")?; |
Comment on lines
+28
to
+32
| timestamp: env.ledger().timestamp(), | ||
| }; | ||
|
|
||
| // If it's a new review, add reviewer to the project list | ||
| if !env.storage().persistent().has(&key) { |
There was a problem hiding this comment.
The review is written unconditionally, so an existing (project_id, reviewer) review gets overwritten (duplicate review). If duplicates should be rejected, return AlreadyReviewed when the key already exists and avoid overwriting.
Comment on lines
+21
to
+25
| let key = StorageKey::Review(project_id, reviewer.clone()); | ||
|
|
||
| let review = Review { | ||
| project_id, | ||
| reviewer: reviewer.clone(), |
There was a problem hiding this comment.
rating is persisted without validation (line 23). Since the contract defines InvalidRating and expects 1..=5, validate the range before storing/emitting the event.
Comment on lines
+102
to
+104
| return Err(ContractError::InsufficientBalance); | ||
| } | ||
|
|
There was a problem hiding this comment.
withdraw_treasury also casts amount: u128 to i128 via as in the token transfer. Add a bounds check to prevent overflow/truncation.
Comment on lines
59
to
+63
| } | ||
|
|
||
| pub fn list_pending_verifications( | ||
| _env: &Env, | ||
| pub fn reject_verification( | ||
| env: &Env, | ||
| project_id: u64, |
There was a problem hiding this comment.
reject_verification ignores the provided admin (it’s named _admin) and doesn’t require auth, so any caller can reject a verification. Require admin.require_auth() and verify it matches the stored admin (StorageKey::Admin).
Contributor
|
@sudo-robi fix all comments and resolve conflicts |
Contributor
|
@sudo-robi please fix conflicts |
Contributor
|
@sudo-robi pull, fix and push again |
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.
closes #21
feat: implement treasury tracking and withdrawal functionality
Summary
This PR implements robust treasury tracking and withdrawal functionality for the Dongle Smart Contract. It ensures that fees collected in various tokens (XLM, USDC, etc.) are securely stored, accurately tracked, and can be withdrawn by authorized admins to a designated treasury address.
Proposed Changes
Core Functionality
require_authchecks to ensure only the contract admin can initiate transfers.TREAS_WDevent for auditability.Errors & Storage
TreasuryAddressNotSeterror to handle cases where a withdrawal is attempted before a destination is configured.StorageKeydefinitions for consistent data management.Verification Results
Automated Tests
Added comprehensive unit tests in src/test_treasury.rs covering both success and failure scenarios. All 12 tests passed successfully:
test_insufficient_treasury_funds: Catches invalid withdrawal amounts.Test Output