Skip to content

fix: harden get_active_listings input validation & edge cases (closes #120) #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Obiajulu-gif wants to merge 2 commits into
base: testnet-implementation
Choose a base branch
from
Open

fix: harden get_active_listings input validation & edge cases (closes #120) #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
78368a3
#120: harden get_active_listings input validation & edge cases
Obiajulu-gif Jul 25, 2026
f11ae95
Merge branch 'testnet-implementation' into fix/issue-120-split
Obiajulu-gif Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion contracts/marketplace/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,35 @@ impl MarketplaceContract {
.ok_or(MarketplaceError::ListingNotFound)
}

/// Return up to `limit` active listings, skipping the first `start` entries
/// of the active-listings index.
///
/// Input validation / edge-case handling (issue #120):
/// - `limit == 0`: a request for zero items is trivially satisfied, so we
/// return an empty vec immediately rather than treating it as an error.
/// - `start` beyond the number of active listings: the index iteration
/// simply skips every entry and yields an empty vec — no panic.
/// - Stale index entry (an id in `ActiveListings` whose `Listing(id)` record
/// was removed from persistent storage): skipped gracefully via the
/// `if let Some(l)` guard.
/// - An id still present in the index but whose listing has `active == false`:
/// filtered out by the `if l.active` check.
///
/// Every edge case degrades gracefully to an empty/partial result, so there
/// is no genuine failure condition to signal. The return type stays
/// `Vec<Listing>` (rather than `Result<..>`) to avoid needless API churn for
/// callers.
pub fn get_active_listings(env: Env, start: u64, limit: u32) -> Vec<Listing> {
let mut result: Vec<Listing> = Vec::new(&env);
// A request for zero items is trivially satisfied with an empty vec.
if limit == 0 {
return result;
}
let active_ids: Vec<u64> = env
.storage()
.instance()
.get(&DataKey::ActiveListings)
.unwrap_or_else(|| Vec::new(&env));
let mut result: Vec<Listing> = Vec::new(&env);
let mut count: u32 = 0;
for (i, id) in active_ids.iter().enumerate() {
if (i as u64) < start {
Expand Down
211 changes: 87 additions & 124 deletions contracts/marketplace/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,130 +216,6 @@ fn test_buy_bot_pays_seller_minus_fee_and_transfers_bot() {
assert_eq!(h.mkt.get_active_listings(&0, &100).len(), 0);
}

#[test]
fn test_buy_bot_success() {
let h = setup();
let seller = Address::generate(&h.env);
let buyer = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);

h.token.mint(&buyer, &100_0000000_i128);

let listing_id = h
.mkt
.list_bot(&seller, &bot_id, &100_0000000_i128, &h.token.address);
assert_eq!(listing_id, 1);

h.mkt.buy_bot(&buyer, &listing_id);

let listing = h.mkt.get_listing(&listing_id);
assert!(!listing.active);

let bot = h.bot.get_bot(&bot_id);
assert_eq!(bot.owner, buyer);

// 2.5% fee to admin (2_5000000), 97.5% to seller (97_5000000)
assert_eq!(h.token.balance(&seller), 97_5000000_i128);
assert_eq!(h.token.balance(&h.admin), 2_5000000_i128);
assert_eq!(h.token.balance(&buyer), 0_i128);
}

#[test]
fn test_buy_bot_listing_not_found() {
let h = setup();
let buyer = Address::generate(&h.env);
assert_eq!(
h.mkt.try_buy_bot(&buyer, &999_u64),
Err(Ok(MarketplaceError::ListingNotFound))
);
}

#[test]
fn test_buy_bot_inactive_listing_fails() {
let h = setup();
let seller = Address::generate(&h.env);
let buyer1 = Address::generate(&h.env);
let buyer2 = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);

h.token.mint(&buyer1, &100_0000000_i128);
h.token.mint(&buyer2, &100_0000000_i128);

let listing_id = h
.mkt
.list_bot(&seller, &bot_id, &100_0000000_i128, &h.token.address);

h.mkt.buy_bot(&buyer1, &listing_id);

// Second purchase attempt must fail with ListingNotActive
assert_eq!(
h.mkt.try_buy_bot(&buyer2, &listing_id),
Err(Ok(MarketplaceError::ListingNotActive))
);
}

#[test]
fn test_buy_bot_insufficient_funds_fails() {
let h = setup();
let seller = Address::generate(&h.env);
let buyer = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);

// Buyer has insufficient balance (10 tokens vs 100 price)
h.token.mint(&buyer, &10_0000000_i128);

let listing_id = h
.mkt
.list_bot(&seller, &bot_id, &100_0000000_i128, &h.token.address);

assert_eq!(
h.mkt.try_buy_bot(&buyer, &listing_id),
Err(Ok(MarketplaceError::PaymentFailed))
);
}

#[test]
fn test_buy_bot_self_purchase_fails() {
let h = setup();
let seller = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);
let price = 100_0000000_i128;
h.token.mint(&seller, &price);

let listing_id = h.mkt.list_bot(&seller, &bot_id, &price, &h.token.address);

// A seller cannot buy back their own listing.
assert_eq!(
h.mkt.try_buy_bot(&seller, &listing_id),
Err(Ok(MarketplaceError::Unauthorized))
);
// Listing is unaffected and the bot remains escrowed.
assert!(h.mkt.get_listing(&listing_id).active);
assert_eq!(h.bot.get_bot(&bot_id).owner, h.mkt.address);
}

#[test]
fn test_buy_bot_extreme_price_overflow_fails() {
let h = setup();
let seller = Address::generate(&h.env);
let buyer = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);

// A price this large makes the 2.5% fee computation (price * 25) overflow
// i128. This must surface as an error, not a panic.
let listing_id = h
.mkt
.list_bot(&seller, &bot_id, &i128::MAX, &h.token.address);

assert_eq!(
h.mkt.try_buy_bot(&buyer, &listing_id),
Err(Ok(MarketplaceError::Overflow))
);
// Listing is unaffected and the bot remains escrowed.
assert!(h.mkt.get_listing(&listing_id).active);
assert_eq!(h.bot.get_bot(&bot_id).owner, h.mkt.address);
}

#[test]
fn test_cancel_listing_returns_bot_to_seller() {
let h = setup();
Expand Down Expand Up @@ -431,3 +307,90 @@ fn test_cancel_listing_by_non_seller_fails() {
// Listing still active
assert!(h.mkt.get_listing(&listing_id).active);
}

// ── get_active_listings edge cases (#120) ───────────────────────────────────

// #120: limit == 0 returns an empty vec (a request for zero items), never a panic
#[test]
fn test_get_active_listings_zero_limit_returns_empty() {
let h = setup();
let seller = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);
h.mkt
.list_bot(&seller, &bot_id, &10_0000000_i128, &h.token.address);
// There is one active listing, but a limit of 0 yields nothing.
assert_eq!(h.mkt.get_active_listings(&0, &0).len(), 0);
}

// #120: start beyond the number of active listings returns an empty vec
#[test]
fn test_get_active_listings_start_beyond_count_returns_empty() {
let h = setup();
let seller = Address::generate(&h.env);
let bot_id = h.bot.mint_basic(&seller);
h.mkt
.list_bot(&seller, &bot_id, &10_0000000_i128, &h.token.address);
// Only one active listing (index 0); starting at 5 skips everything.
assert_eq!(h.mkt.get_active_listings(&5, &100).len(), 0);
}

// #120: a stale active-id (index entry present but the Listing record was
// removed) is skipped gracefully rather than causing a panic.
#[test]
fn test_get_active_listings_skips_stale_index_entry() {
let h = setup();
let seller = Address::generate(&h.env);
let id1 = h.bot.mint_basic(&seller);
let id2 = h.bot.mint_basic(&seller);
let l1 = h
.mkt
.list_bot(&seller, &id1, &10_0000000_i128, &h.token.address);
h.mkt
.list_bot(&seller, &id2, &20_0000000_i128, &h.token.address);
assert_eq!(h.mkt.get_active_listings(&0, &100).len(), 2);

// Remove the persistent Listing record for l1 while leaving it in the
// ActiveListings index — simulating a stale index entry.
h.env.as_contract(&h.mkt.address, || {
h.env
.storage()
.persistent()
.remove(&DataKey::Listing(l1));
});

// Only the still-present listing is returned; no panic on the stale id.
let listings = h.mkt.get_active_listings(&0, &100);
assert_eq!(listings.len(), 1);
assert_eq!(listings.get(0).unwrap().id, 2);
}

// #120: an id present in the active-ids index but whose listing is marked
// inactive is filtered out.
#[test]
fn test_get_active_listings_filters_inactive_still_in_index() {
let h = setup();
let seller = Address::generate(&h.env);
let id1 = h.bot.mint_basic(&seller);
let l1 = h
.mkt
.list_bot(&seller, &id1, &10_0000000_i128, &h.token.address);
assert_eq!(h.mkt.get_active_listings(&0, &100).len(), 1);

// Flip the listing to inactive but leave it in the ActiveListings index.
h.env.as_contract(&h.mkt.address, || {
let mut listing: Listing = h
.env
.storage()
.persistent()
.get(&DataKey::Listing(l1))
.unwrap();
listing.active = false;
h.env
.storage()
.persistent()
.set(&DataKey::Listing(l1), &listing);
});

// The inactive listing is filtered out despite remaining in the index.
assert_eq!(h.mkt.get_active_listings(&0, &100).len(), 0);
}