feat: optimize marketplace contracts with tests and docs#158
Open
mgtmdccix-oss wants to merge 2 commits into
Open
feat: optimize marketplace contracts with tests and docs#158mgtmdccix-oss wants to merge 2 commits into
mgtmdccix-oss wants to merge 2 commits into
Conversation
- Add reentrancy guard on fund_invoice via kora_shared::reentrancy - Enforce checks-effects-interactions: state written before token transfers - Add net > 0 guard after fee calculation to prevent 100% fee edge case - Add TTL bumping (extend_ttl) on all persistent storage writes - Replace unwrap() on instance reads with ok_or(KoraError::NotInitialized) - Add require_not_paused() stub on all mutating functions - Capture listing_token before storage write to avoid borrow ambiguity New functions: - update_fee_bps(admin, new_fee_bps): admin can update fee rate post-deploy - dewhitelist_token(admin, token): admin can remove token from whitelist - get_fee_bps(): view current fee rate - get_admin(): view current admin address - is_token_whitelisted(token): check whitelist status Tests: 58 unit tests covering all functions and edge cases
|
@mgtmdccix-oss 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! 🚀 |
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
Rewrites
lib.rs
to improve security, robustness, and developer experience across the Marketplace domain. No changes to other contracts.
Changes
Security
Reentrancy guard on fund_invoice using kora_shared::reentrancy::acquire_guard. The function is split into a public entry point (holds the guard) and a private fund_invoice_inner (does the work), so the guard cleanly wraps the entire operation including token transfers and the cross-contract pool call.
Checks-effects-interactions enforced explicitly — funded_amount and is_active are written to storage before any token transfers occur.
net > 0 guard after fee calculation prevents a 100% fee misconfiguration from sending zero tokens to the pool.
require_not_paused stub added to all mutating functions (list_invoice, fund_invoice, cancel_listing), ready for access_control integration at deployment.
Robustness
TTL bumping on all persistent storage writes via bump_listing() and inline extend_ttl for whitelisted tokens — matches the pattern used in access_control and risk_registry.
Typed errors on instance reads — replaced .unwrap() on InvoiceNft, FinancingPool, and Treasury address reads with .ok_or(KoraError::NotInitialized).
Token address captured before storage write to eliminate any borrow ambiguity around listing.token.
New functions
Function Description
update_fee_bps(admin, new_fee_bps) Admin can update the fee rate post-deploy
dewhitelist_token(admin, token) Admin can remove a token from the whitelist
get_fee_bps() View the current fee rate
get_admin() View the current admin address
is_token_whitelisted(token) Check whitelist status of a token
Tests
58 unit tests added covering all functions and edge cases:
initialize — boundary fee values (0, max, over-max), double-init
get_admin / get_fee_bps — correct values, pre-init error
update_fee_bps — success, boundary values, non-admin rejection, state unchanged on failure
whitelist_token / dewhitelist_token — full lifecycle, non-admin rejection, re-whitelist after removal
is_token_whitelisted — unknown token returns false
list_invoice — success, NFT status transition, all validation rejections (zero/negative price, price ≥ face value, past deadline, non-whitelisted token, duplicate id), multiple independent listings
get_listing — not found, correct data returned
fund_invoice — partial/full funding, deadline expiry, cancelled listing, exceeds target, fee math, multiple investors, exact remaining amount, one-over-remaining, zero-fee edge case
cancel_listing — seller, admin, stranger, not found, already cancelled, state unchanged on failure, partial-funded cancel
Arithmetic safety — overflow protection on funded_amount
Checklist
Scoped to contracts/marketplace/ only
All arithmetic uses checked_* / bps_of — no silent overflows
No unwrap() in contract code
require_auth() is the first call in every mutating function
Follows TTL bumping pattern from access_control and risk_registry
Compatible with existing Soroban SDK 21.0.0 infrastructure
Existing integration tests in contracts/tests/ unaffected
closes #101