From 38bfeaeab14a417695a4ea2fac341504ffdb0040 Mon Sep 17 00:00:00 2001 From: ayinde38 Date: Tue, 25 Aug 2026 12:09:02 +0100 Subject: [PATCH] test(creator-keys): cover treasury rotation, sell TTL, blank handle, holder count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the four test suites requested in #740, #741, #742 and #743, plus the one behaviour change #742 needs to have something to assert against. protocol_fee_recipient_update_persistence.rs (#740) Covers update_protocol_fee_recipient: the rotated address is what lands on the ledger, the superseded one is gone, fees accrued after the rotation are payable to the new recipient, and both guards (non-admin, zero address) reject without disturbing the stored value. ttl_extension_on_sell.rs (#741) The buy path's TTL extension was covered; the sell path only had a single "TTL went up" assertion. Adds the full set: a successful sell restores a complete CREATOR_TTL_LEDGERS window, repeated sells reset that window from the current ledger rather than stacking on the remainder, and a sell that reverts — on slippage, or with no balance — extends nothing. display_name_empty_registration.rs (#742) A blank handle previously surfaced as HandleTooShort (empty, single space) or InvalidHandleCharacter (three spaces), neither of which tells the caller what actually went wrong. Adds ContractError::DisplayNameEmpty = 40 — appended per docs/error-extension-guide.md, so no existing discriminant moves — and checks it ahead of the length and character rules. The other handle rules are unchanged and the tests pin that: "aa" is still HandleTooShort and "alice bob" is still InvalidHandleCharacter. empty_handle_registration_regression.rs (#301) asserted HandleTooShort for the empty string and now asserts DisplayNameEmpty. The rejection and the "no state written" invariant it guards are unchanged. holder_count_buy_sell_sequence.rs (#743) Existing coverage has three wallets holding one key each, which a key counter would also pass. Adds the case that separates the two: wallets holding several keys, where partial sells must leave the count alone and only the final key may decrement it. Also covers re-entry after a full exit. Every step cross-checks the view against supply and per-wallet balances. docs/error-codes.md documents the new variant. --- creator-keys/src/lib.rs | 34 ++- .../tests/display_name_empty_registration.rs | 211 +++++++++++++++ .../empty_handle_registration_regression.rs | 6 +- .../tests/holder_count_buy_sell_sequence.rs | 250 ++++++++++++++++++ ...otocol_fee_recipient_update_persistence.rs | 193 ++++++++++++++ creator-keys/tests/ttl_extension_on_sell.rs | 191 +++++++++++++ docs/error-codes.md | 2 + 7 files changed, 875 insertions(+), 12 deletions(-) create mode 100644 creator-keys/tests/display_name_empty_registration.rs create mode 100644 creator-keys/tests/holder_count_buy_sell_sequence.rs create mode 100644 creator-keys/tests/protocol_fee_recipient_update_persistence.rs create mode 100644 creator-keys/tests/ttl_extension_on_sell.rs diff --git a/creator-keys/src/lib.rs b/creator-keys/src/lib.rs index 764be7a9..ef464912 100644 --- a/creator-keys/src/lib.rs +++ b/creator-keys/src/lib.rs @@ -86,6 +86,7 @@ pub enum ContractError { WalletBlacklisted = 37, SchemaVersionTooOld = 38, SchemaVersionUnsupported = 39, + DisplayNameEmpty = 40, } pub mod fee { @@ -1035,21 +1036,31 @@ fn is_valid_handle_byte(byte: u8) -> bool { byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_' } +/// Validates a creator's display handle. +/// +/// A blank handle — empty, or nothing but ASCII whitespace — is reported as +/// [`ContractError::DisplayNameEmpty`] ahead of the length and character rules, +/// so a caller that simply omitted the field gets that back rather than the +/// generic "too short". The over-length check runs first because the handle +/// bytes are read into a fixed `HANDLE_LEN_MAX` buffer. fn validate_creator_handle(handle: &String) -> Result<(), ContractError> { let len = handle.len(); - if len < HANDLE_LEN_MIN { - return Err(ContractError::HandleTooShort); - } if len > HANDLE_LEN_MAX { return Err(ContractError::HandleTooLong); } let mut bytes = [0u8; HANDLE_LEN_MAX as usize]; handle.copy_into_slice(&mut bytes[..len as usize]); - if bytes[..len as usize] - .iter() - .any(|byte| !is_valid_handle_byte(*byte)) - { + let handle_bytes = &bytes[..len as usize]; + + // An empty slice satisfies `all`, so this covers the empty-string case too. + if handle_bytes.iter().all(|byte| byte.is_ascii_whitespace()) { + return Err(ContractError::DisplayNameEmpty); + } + if len < HANDLE_LEN_MIN { + return Err(ContractError::HandleTooShort); + } + if handle_bytes.iter().any(|byte| !is_valid_handle_byte(*byte)) { return Err(ContractError::InvalidHandleCharacter); } @@ -1608,10 +1619,11 @@ impl CreatorKeysContract { /// - `creator`: must authorize the call (`require_auth`). A profile must not /// already exist for this address, otherwise /// [`ContractError::AlreadyRegistered`]. - /// - `handle`: validated by [`validate_creator_handle`] — below the minimum - /// length returns [`ContractError::HandleTooShort`], above the maximum - /// returns [`ContractError::HandleTooLong`], and any disallowed byte - /// returns [`ContractError::InvalidHandleCharacter`]. + /// - `handle`: validated by [`validate_creator_handle`] — a blank handle + /// (empty or whitespace-only) returns [`ContractError::DisplayNameEmpty`], + /// below the minimum length returns [`ContractError::HandleTooShort`], + /// above the maximum returns [`ContractError::HandleTooLong`], and any + /// disallowed byte returns [`ContractError::InvalidHandleCharacter`]. /// - `locked_allocation`: optional time-locked key allocation for creator self-vesting. /// If provided, `unlock_ledger` must be strictly greater than current ledger. /// - `max_supply`: optional maximum supply cap. If provided, must be greater than zero. diff --git a/creator-keys/tests/display_name_empty_registration.rs b/creator-keys/tests/display_name_empty_registration.rs new file mode 100644 index 00000000..14ac8680 --- /dev/null +++ b/creator-keys/tests/display_name_empty_registration.rs @@ -0,0 +1,211 @@ +//! Unit tests for the blank display-name guard on creator registration (#742). +//! +//! A registration whose display handle is empty — or nothing but whitespace, which +//! is the same thing from a user's point of view — is rejected with +//! [`ContractError::DisplayNameEmpty`]. The guard runs ahead of the length and +//! character rules so the caller gets the precise reason back, and it runs before +//! any storage write so a rejected registration leaves no trace. +//! +//! `empty_handle_registration_regression.rs` covers the empty-string case as a +//! regression; this file pins the guard's *ordering* against the other handle +//! rules and the no-partial-state invariant. + +mod contract_test_env; + +use contract_test_env::{register_creator_keys, test_env_with_auths}; +use creator_keys::{ContractError, CreatorKeysContractClient, HANDLE_LEN_MIN}; +use soroban_sdk::{testutils::Address as _, Address, Env, String}; + +/// Register a fresh address with `handle`, returning that address and whether +/// the call succeeded (`Ok`) or the contract error it failed with (`Err`). +fn register_with_handle( + env: &Env, + client: &CreatorKeysContractClient<'_>, + handle: &str, +) -> (Address, Result<(), ContractError>) { + let creator = Address::generate(env); + let outcome = match client.try_register_creator( + &creator_keys::RegisterCreatorParams { + creator: creator.clone(), + handle: String::from_str(env, handle), + }, + &None, + &None, + &None, + &None, + &None, + &None, + ) { + Ok(Ok(())) => Ok(()), + Ok(Err(_)) => panic!("register_creator returned an undecodable success value"), + Err(Ok(error)) => Err(error), + Err(Err(invoke_error)) => { + panic!("register_creator failed with a non-contract error: {invoke_error:?}") + } + }; + (creator, outcome) +} + +/// A rejected registration must leave nothing behind: no profile, no derived +/// view state. +fn assert_no_creator_state(client: &CreatorKeysContractClient<'_>, creator: &Address) { + assert!( + !client.is_creator_registered(creator), + "no creator profile should exist after a rejected registration" + ); + assert_eq!( + client.get_creator_holder_count(creator), + 0, + "no holder count should be written after a rejected registration" + ); + assert_eq!( + client.get_total_key_supply(creator), + 0, + "no supply should be written after a rejected registration" + ); +} + +#[test] +fn empty_display_name_is_rejected() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let (creator, result) = register_with_handle(&env, &client, ""); + assert_eq!( + result, + Err(ContractError::DisplayNameEmpty), + "an empty display name should be rejected as blank" + ); + assert_no_creator_state(&client, &creator); +} + +#[test] +fn single_whitespace_display_name_is_rejected() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let (creator, result) = register_with_handle(&env, &client, " "); + assert_eq!( + result, + Err(ContractError::DisplayNameEmpty), + "a single space is a blank display name, not a short one" + ); + assert_no_creator_state(&client, &creator); +} + +/// Whitespace-only handles long enough to clear the minimum length still fail as +/// blank rather than falling through to the character check — this is what makes +/// the guard's position in the validator observable. +#[test] +fn whitespace_only_display_name_is_blank_not_an_invalid_character() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + for handle in [" ", "\t\t\t\t", " \t \t "] { + let (creator, result) = register_with_handle(&env, &client, handle); + assert_eq!( + result, + Err(ContractError::DisplayNameEmpty), + "whitespace-only handle {handle:?} should be rejected as blank, \ + not as an invalid character" + ); + assert_no_creator_state(&client, &creator); + } +} + +/// The blank guard must not swallow the other handle rules: a short but +/// non-blank handle still reports `HandleTooShort`. +#[test] +fn short_non_blank_display_name_still_reports_too_short() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let short = "a".repeat((HANDLE_LEN_MIN - 1) as usize); + let (creator, result) = register_with_handle(&env, &client, &short); + assert_eq!( + result, + Err(ContractError::HandleTooShort), + "a non-blank handle below the minimum length is still HandleTooShort" + ); + assert_no_creator_state(&client, &creator); +} + +/// Likewise a handle containing a disallowed character among real content is an +/// `InvalidHandleCharacter`, not a blank name. +#[test] +fn non_blank_display_name_with_bad_characters_still_reports_invalid_character() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let (creator, result) = register_with_handle(&env, &client, "alice bob"); + assert_eq!( + result, + Err(ContractError::InvalidHandleCharacter), + "whitespace mixed with real content is an invalid character, not a blank name" + ); + assert_no_creator_state(&client, &creator); +} + +#[test] +fn valid_display_name_registers_successfully() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let (creator, result) = register_with_handle(&env, &client, "alice"); + assert_eq!( + result, + Ok(()), + "a valid non-empty display name should register" + ); + assert!( + client.is_creator_registered(&creator), + "the creator profile should exist after a successful registration" + ); + assert_eq!( + client.get_creator(&creator).handle, + String::from_str(&env, "alice"), + "the stored handle should match the registered one" + ); +} + +/// A blank name rejected first must not block a later valid registration by the +/// same address — the guard writes nothing, so the address is still free. +#[test] +fn rejected_blank_registration_leaves_the_address_registrable() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + + let creator = Address::generate(&env); + let blank = client.try_register_creator( + &creator_keys::RegisterCreatorParams { + creator: creator.clone(), + handle: String::from_str(&env, " "), + }, + &None, + &None, + &None, + &None, + &None, + &None, + ); + assert_eq!(blank, Err(Ok(ContractError::DisplayNameEmpty))); + + let retry = client.try_register_creator( + &creator_keys::RegisterCreatorParams { + creator: creator.clone(), + handle: String::from_str(&env, "alice"), + }, + &None, + &None, + &None, + &None, + &None, + &None, + ); + assert_eq!( + retry, + Ok(Ok(())), + "the address should still be registrable after a rejected blank handle" + ); + assert!(client.is_creator_registered(&creator)); +} diff --git a/creator-keys/tests/empty_handle_registration_regression.rs b/creator-keys/tests/empty_handle_registration_regression.rs index cc48747c..fbe92a83 100644 --- a/creator-keys/tests/empty_handle_registration_regression.rs +++ b/creator-keys/tests/empty_handle_registration_regression.rs @@ -2,6 +2,10 @@ //! //! Confirms that the shortest possible invalid input (empty string) is caught by the //! handle validation guard and that no creator state is written as a result. +//! +//! The expected variant moved from `HandleTooShort` to the more specific +//! `DisplayNameEmpty` when the blank-handle guard was added in #742; the +//! rejection itself, and the "no state written" invariant, are unchanged. use creator_keys::{ContractError, CreatorKeysContract, CreatorKeysContractClient}; use soroban_sdk::{testutils::Address as _, Address, Env, String}; @@ -28,6 +32,6 @@ fn test_register_creator_rejects_empty_handle() { &None, ); - assert_eq!(result, Err(Ok(ContractError::HandleTooShort))); + assert_eq!(result, Err(Ok(ContractError::DisplayNameEmpty))); assert!(!client.is_creator_registered(&creator)); } diff --git a/creator-keys/tests/holder_count_buy_sell_sequence.rs b/creator-keys/tests/holder_count_buy_sell_sequence.rs new file mode 100644 index 00000000..de459457 --- /dev/null +++ b/creator-keys/tests/holder_count_buy_sell_sequence.rs @@ -0,0 +1,250 @@ +//! Unit tests for the holder-count view across a full buy/sell sequence (#743). +//! +//! `get_creator_holder_count` must report the number of *unique wallets* currently +//! holding at least one key — not the number of keys, and not the number of wallets +//! that have ever traded. `holder_count_multiple_buyers.rs` covers three wallets +//! holding one key each; the case that distinguishes a unique-wallet counter from a +//! key counter is a wallet holding *several* keys, where only the sale of the last +//! key may decrement the count. +//! +//! Every step also asserts the view against `get_creator_supply` and the per-wallet +//! balance, so the count cannot drift away from the state it is derived from. + +mod contract_test_env; + +use contract_test_env::{register_creator_keys, register_test_creator, set_key_price_for_tests}; +use soroban_sdk::{testutils::Address as _, Address, Env}; + +const KEY_PRICE: i128 = 100; + +/// Assert the holder count, total supply and both wallets' balances in one step. +fn assert_state( + client: &creator_keys::CreatorKeysContractClient<'_>, + creator: &Address, + expected_holders: u32, + expected_supply: u32, + wallets: &[(&Address, u32)], + step: &str, +) { + assert_eq!( + client.get_creator_holder_count(creator), + expected_holders, + "holder count mismatch at step: {step}" + ); + assert_eq!( + client.get_creator_supply(creator), + expected_supply, + "supply mismatch at step: {step}" + ); + for (wallet, expected_balance) in wallets { + assert_eq!( + client.get_key_balance(creator, wallet), + *expected_balance, + "wallet balance mismatch at step: {step}" + ); + } +} + +fn setup( + env: &Env, +) -> ( + creator_keys::CreatorKeysContractClient<'_>, + Address, + Address, + Address, +) { + let (client, _contract_id) = register_creator_keys(env); + set_key_price_for_tests(env, &client, KEY_PRICE); + let creator = register_test_creator(env, &client, "alice"); + (client, creator, Address::generate(env), Address::generate(env)) +} + +#[test] +fn holder_count_is_zero_before_any_buys() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, creator, wallet_a, _wallet_b) = setup(&env); + + assert_state( + &client, + &creator, + 0, + 0, + &[(&wallet_a, 0)], + "freshly registered creator", + ); +} + +#[test] +fn holder_count_tracks_two_wallets_through_buys_and_full_exits() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, creator, wallet_a, wallet_b) = setup(&env); + + assert_state( + &client, + &creator, + 0, + 0, + &[(&wallet_a, 0), (&wallet_b, 0)], + "before any buys", + ); + + // Two distinct wallets buy; each first buy is a new holder. + client.buy_key(&creator, &wallet_a, &KEY_PRICE, &None); + assert_state( + &client, + &creator, + 1, + 1, + &[(&wallet_a, 1), (&wallet_b, 0)], + "wallet A's first buy", + ); + + client.buy_key(&creator, &wallet_b, &KEY_PRICE, &None); + assert_state( + &client, + &creator, + 2, + 2, + &[(&wallet_a, 1), (&wallet_b, 1)], + "wallet B's first buy", + ); + + // Wallet A sells its only key: a full exit, so the count drops to 1. + client.sell_key(&creator, &wallet_a, &None); + assert_state( + &client, + &creator, + 1, + 1, + &[(&wallet_a, 0), (&wallet_b, 1)], + "wallet A sells all of its keys", + ); + + // Wallet B follows: no holders left. + client.sell_key(&creator, &wallet_b, &None); + assert_state( + &client, + &creator, + 0, + 0, + &[(&wallet_a, 0), (&wallet_b, 0)], + "wallet B sells all of its keys", + ); +} + +/// The discriminating case: a wallet with several keys stays a holder until its +/// balance actually reaches zero. +#[test] +fn partial_sells_do_not_decrement_the_holder_count() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, creator, wallet_a, wallet_b) = setup(&env); + + // Wallet A takes three keys, wallet B takes two. + for _ in 0..3 { + client.buy_key(&creator, &wallet_a, &KEY_PRICE, &None); + } + for _ in 0..2 { + client.buy_key(&creator, &wallet_b, &KEY_PRICE, &None); + } + assert_state( + &client, + &creator, + 2, + 5, + &[(&wallet_a, 3), (&wallet_b, 2)], + "two wallets holding multiple keys", + ); + + // Selling down wallet A one key at a time leaves the count at 2 until the + // last key goes. If the count tracked keys instead of wallets, the first of + // these assertions would fail. + client.sell_key(&creator, &wallet_a, &None); + assert_state( + &client, + &creator, + 2, + 4, + &[(&wallet_a, 2), (&wallet_b, 2)], + "wallet A partial sell (2 keys left)", + ); + + client.sell_key(&creator, &wallet_a, &None); + assert_state( + &client, + &creator, + 2, + 3, + &[(&wallet_a, 1), (&wallet_b, 2)], + "wallet A partial sell (1 key left)", + ); + + client.sell_key(&creator, &wallet_a, &None); + assert_state( + &client, + &creator, + 1, + 2, + &[(&wallet_a, 0), (&wallet_b, 2)], + "wallet A's final key sold", + ); + + // Wallet B exits the same way. + client.sell_key(&creator, &wallet_b, &None); + assert_state( + &client, + &creator, + 1, + 1, + &[(&wallet_a, 0), (&wallet_b, 1)], + "wallet B partial sell", + ); + + client.sell_key(&creator, &wallet_b, &None); + assert_state( + &client, + &creator, + 0, + 0, + &[(&wallet_a, 0), (&wallet_b, 0)], + "all holders exited", + ); +} + +/// A wallet that buys again after a full exit is counted once more, and repeat +/// buys by an existing holder never double-count. +#[test] +fn repeat_buys_and_re_entry_are_counted_once_per_wallet() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, creator, wallet_a, wallet_b) = setup(&env); + + client.buy_key(&creator, &wallet_a, &KEY_PRICE, &None); + client.buy_key(&creator, &wallet_a, &KEY_PRICE, &None); + assert_state( + &client, + &creator, + 1, + 2, + &[(&wallet_a, 2)], + "a second buy by the same wallet is not a second holder", + ); + + client.sell_key(&creator, &wallet_a, &None); + client.sell_key(&creator, &wallet_a, &None); + assert_state(&client, &creator, 0, 0, &[(&wallet_a, 0)], "wallet A exited"); + + // Re-entry counts again. + client.buy_key(&creator, &wallet_a, &KEY_PRICE, &None); + client.buy_key(&creator, &wallet_b, &KEY_PRICE, &None); + assert_state( + &client, + &creator, + 2, + 2, + &[(&wallet_a, 1), (&wallet_b, 1)], + "wallet A re-entered alongside wallet B", + ); +} diff --git a/creator-keys/tests/protocol_fee_recipient_update_persistence.rs b/creator-keys/tests/protocol_fee_recipient_update_persistence.rs new file mode 100644 index 00000000..7501acb4 --- /dev/null +++ b/creator-keys/tests/protocol_fee_recipient_update_persistence.rs @@ -0,0 +1,193 @@ +//! Persistence tests for the protocol fee recipient (treasury) update entrypoint (#740). +//! +//! `update_protocol_fee_recipient` rotates the address that protocol-side trade +//! fees accrue to. Existing coverage checks that the read view reflects an update +//! (`protocol_fee_recipient.rs`) and that the `set_` entrypoint rejects the zero +//! address (`set_protocol_fee_recipient.rs`). What is not covered is the update +//! entrypoint's own guard rails and the downstream effect of a rotation: +//! +//! - the new address is what is actually persisted, and the old one is gone +//! - fees accrued *after* the rotation are attributed to the new recipient +//! - a non-admin caller is rejected with [`ContractError::Unauthorized`] +//! - the zero address is rejected with [`ContractError::ZeroAddress`] and the +//! previously stored recipient survives the rejected call + +mod contract_test_env; + +use contract_test_env::{register_creator_keys, register_test_creator, test_env_with_auths}; +use creator_keys::{constants, ContractError, CreatorKeysContractClient}; +use soroban_sdk::{testutils::Address as _, Address, Env, String}; + +const KEY_PRICE: i128 = 1_000; +const CREATOR_BPS: u32 = 9_000; +const PROTOCOL_BPS: u32 = 1_000; + +/// The Stellar all-zero account address, rejected by the address validators. +fn zero_address(env: &Env) -> Address { + Address::from_string(&String::from_str( + env, + "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + )) +} + +/// Read the raw persisted recipient straight out of contract storage. +/// +/// The view function is the normal way in, but reading the storage key directly +/// is what lets these tests distinguish "the view returns the new address" from +/// "the new address is what is on the ledger". +fn stored_recipient(env: &Env, contract_id: &Address) -> Option
{ + env.as_contract(contract_id, || { + env.storage() + .persistent() + .get(&constants::storage::PROTOCOL_FEE_RECIPIENT) + }) +} + +/// Register the contract with an admin, a fee split and an initial recipient. +fn setup(env: &Env) -> (CreatorKeysContractClient<'_>, Address, Address, Address) { + let (client, contract_id) = register_creator_keys(env); + let admin = Address::generate(env); + client.set_protocol_admin(&admin, &admin); + client.set_key_price(&admin, &KEY_PRICE); + client.set_fee_config(&admin, &CREATOR_BPS, &PROTOCOL_BPS); + + let original_recipient = Address::generate(env); + client.set_protocol_fee_recipient(&admin, &original_recipient); + + (client, contract_id, admin, original_recipient) +} + +#[test] +fn update_persists_new_recipient_in_contract_state() { + let env = test_env_with_auths(); + let (client, contract_id, admin, original) = setup(&env); + + assert_eq!( + stored_recipient(&env, &contract_id), + Some(original.clone()), + "precondition: the original recipient should be on the ledger" + ); + + let new_recipient = Address::generate(&env); + let result = client.try_update_protocol_fee_recipient(&admin, &new_recipient); + assert_eq!(result, Ok(Ok(())), "admin rotation should succeed"); + + assert_eq!( + stored_recipient(&env, &contract_id), + Some(new_recipient), + "the new recipient should be the persisted value" + ); +} + +#[test] +fn view_returns_the_updated_recipient() { + let env = test_env_with_auths(); + let (client, _contract_id, admin, _original) = setup(&env); + + let new_recipient = Address::generate(&env); + client.update_protocol_fee_recipient(&admin, &new_recipient); + + assert_eq!( + client.get_protocol_fee_recipient(), + Some(new_recipient), + "get_protocol_fee_recipient should report the rotated address" + ); +} + +#[test] +fn old_recipient_is_absent_from_state_after_update() { + let env = test_env_with_auths(); + let (client, contract_id, admin, original) = setup(&env); + + let new_recipient = Address::generate(&env); + client.update_protocol_fee_recipient(&admin, &new_recipient); + + let stored = stored_recipient(&env, &contract_id).expect("recipient should still be set"); + assert_ne!( + stored, original, + "the superseded recipient must not remain on the ledger" + ); + assert_ne!( + client.get_protocol_fee_recipient(), + Some(original), + "the view must not report the superseded recipient" + ); +} + +/// The point of the rotation is where the money goes next, so assert on the +/// accrued balance rather than only on the stored address. +#[test] +fn fees_accrued_after_the_update_are_attributed_to_the_new_recipient() { + let env = test_env_with_auths(); + let (client, _contract_id, admin, _original) = setup(&env); + let creator = register_test_creator(&env, &client, "alice"); + let holder = Address::generate(&env); + + // Accrue some protocol fees against the original recipient. + client.buy_key(&creator, &holder, &KEY_PRICE, &None); + client.sell_key(&creator, &holder, &None); + let balance_before_rotation = client.get_protocol_recipient_balance(); + assert!( + balance_before_rotation > 0, + "precondition: a round trip should accrue protocol fees, got {balance_before_rotation}" + ); + + let new_recipient = Address::generate(&env); + client.update_protocol_fee_recipient(&admin, &new_recipient); + + // A second round trip after the rotation must keep accruing, and the + // recipient of record for that accrual is now the new address. + client.buy_key(&creator, &holder, &KEY_PRICE, &None); + client.sell_key(&creator, &holder, &None); + + assert!( + client.get_protocol_recipient_balance() > balance_before_rotation, + "post-rotation trades should keep accruing protocol fees" + ); + assert_eq!( + client.get_protocol_fee_recipient(), + Some(new_recipient), + "the accrued balance should be payable to the rotated recipient" + ); +} + +#[test] +fn non_admin_caller_is_rejected_as_unauthorized() { + let env = test_env_with_auths(); + let (client, contract_id, _admin, original) = setup(&env); + + let impostor = Address::generate(&env); + let attacker_recipient = Address::generate(&env); + + let result = client.try_update_protocol_fee_recipient(&impostor, &attacker_recipient); + assert_eq!( + result, + Err(Ok(ContractError::Unauthorized)), + "only the protocol admin may rotate the fee recipient" + ); + + assert_eq!( + stored_recipient(&env, &contract_id), + Some(original), + "a rejected rotation must leave the stored recipient untouched" + ); +} + +#[test] +fn zero_address_is_rejected_and_leaves_the_recipient_intact() { + let env = test_env_with_auths(); + let (client, contract_id, admin, original) = setup(&env); + + let result = client.try_update_protocol_fee_recipient(&admin, &zero_address(&env)); + assert_eq!( + result, + Err(Ok(ContractError::ZeroAddress)), + "rotating to the zero address would silently burn protocol fees" + ); + + assert_eq!( + stored_recipient(&env, &contract_id), + Some(original), + "a rejected rotation must leave the stored recipient untouched" + ); +} diff --git a/creator-keys/tests/ttl_extension_on_sell.rs b/creator-keys/tests/ttl_extension_on_sell.rs new file mode 100644 index 00000000..ec4950a2 --- /dev/null +++ b/creator-keys/tests/ttl_extension_on_sell.rs @@ -0,0 +1,191 @@ +//! Integration tests for persistent-storage TTL extension during sell transactions (#741). +//! +//! A creator's profile entry must survive active trading. `buy_key` extends the +//! creator's persistent TTL and is covered by `ttl_extension_on_buy.rs`; the sell +//! path calls the same `extend_creator_ttl` helper and needs the same guarantees: +//! every successful sell must push the entry back to a full window, repeated sells +//! must *reset* the window rather than stack it, and a reverted sell must not +//! extend anything. +//! +//! These tests assert on the real remaining TTL read back out of the test +//! ledger, so removing the `extend_creator_ttl` call from `sell_key` fails them. + +mod contract_test_env; + +use contract_test_env::{register_creator_keys, register_test_creator, set_key_price_for_tests}; +use creator_keys::constants::storage; +use creator_keys::{CREATOR_TTL_LEDGERS, TTL_EXTENSION_THRESHOLD}; +use soroban_sdk::testutils::storage::Persistent; +use soroban_sdk::testutils::Ledger; +use soroban_sdk::{testutils::Address as _, Address, Env}; + +const KEY_PRICE: i128 = 100; + +/// Remaining TTL, in ledgers, on the creator's persistent profile entry. +fn creator_ttl_remaining(env: &Env, contract_id: &Address, creator: &Address) -> u32 { + let key = storage::creator(creator); + env.as_contract(contract_id, || env.storage().persistent().get_ttl(&key)) +} + +/// Advance the ledger sequence by `ledgers`, draining TTL from live entries. +fn advance_ledgers(env: &Env, ledgers: u32) { + let mut ledger = env.ledger().get(); + ledger.sequence_number += ledgers; + env.ledger().set(ledger); +} + +fn setup( + env: &Env, +) -> ( + creator_keys::CreatorKeysContractClient<'_>, + Address, + Address, + Address, +) { + let (client, contract_id) = register_creator_keys(env); + // The default test env archives the contract instance after ~4095 ledgers. + // These tests deliberately jump far into the future to drain the creator + // entry's TTL, so the instance needs the full window to stay invocable. + env.deployer().extend_ttl( + contract_id.clone(), + CREATOR_TTL_LEDGERS, + CREATOR_TTL_LEDGERS, + ); + set_key_price_for_tests(env, &client, KEY_PRICE); + let creator = register_test_creator(env, &client, "alice"); + let holder = Address::generate(env); + (client, contract_id, creator, holder) +} + +/// After a successful sell the creator entry must be back at (or above) a full +/// extension window — not merely "a bit higher than it was". +#[test] +fn sell_restores_creator_ttl_to_the_full_window() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, contract_id, creator, holder) = setup(&env); + + // Two keys so the seller still holds one afterwards and the profile entry + // is not removed as part of a full exit. + client.buy_key(&creator, &holder, &KEY_PRICE, &None); + client.buy_key(&creator, &holder, &KEY_PRICE, &None); + + // Drain the TTL down to almost nothing. + let ttl_before_advance = creator_ttl_remaining(&env, &contract_id, &creator); + advance_ledgers(&env, ttl_before_advance.saturating_sub(1).max(1)); + + let ttl_before_sell = creator_ttl_remaining(&env, &contract_id, &creator); + assert!( + ttl_before_sell < TTL_EXTENSION_THRESHOLD, + "precondition: TTL should be drained below the extension threshold, got {ttl_before_sell}" + ); + + let result = client.try_sell_key(&creator, &holder, &None); + assert_eq!(result, Ok(Ok(1)), "sell should succeed"); + + let ttl_after_sell = creator_ttl_remaining(&env, &contract_id, &creator); + assert!( + ttl_after_sell >= CREATOR_TTL_LEDGERS, + "TTL after a sell must reach the full window: \ + before={ttl_before_sell} after={ttl_after_sell} window={CREATOR_TTL_LEDGERS}" + ); +} + +/// The extension is `current_ledger + CREATOR_TTL_LEDGERS`, an absolute target. +/// A second sell must therefore land on the same window measured from the new +/// ledger — the remaining TTL must not be two windows deep. +#[test] +fn repeated_sells_reset_the_ttl_window_rather_than_accumulate() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, contract_id, creator, holder) = setup(&env); + + for _ in 0..3 { + client.buy_key(&creator, &holder, &KEY_PRICE, &None); + } + + let ttl_initial = creator_ttl_remaining(&env, &contract_id, &creator); + advance_ledgers(&env, ttl_initial.saturating_sub(1).max(1)); + + client.sell_key(&creator, &holder, &None); + let ttl_after_first = creator_ttl_remaining(&env, &contract_id, &creator); + + // Burn a chunk of the freshly granted window, then sell again. + let elapsed = CREATOR_TTL_LEDGERS / 4; + advance_ledgers(&env, elapsed); + let ttl_after_elapsing = creator_ttl_remaining(&env, &contract_id, &creator); + assert!( + ttl_after_elapsing < ttl_after_first, + "precondition: advancing the ledger should have consumed TTL" + ); + + client.sell_key(&creator, &holder, &None); + let ttl_after_second = creator_ttl_remaining(&env, &contract_id, &creator); + + assert!( + ttl_after_second >= CREATOR_TTL_LEDGERS, + "the second sell should restore a full window: {ttl_after_second}" + ); + assert!( + ttl_after_second <= CREATOR_TTL_LEDGERS + elapsed, + "the window must be reset from the current ledger, not stacked on the \ + remaining TTL: after_second={ttl_after_second} window={CREATOR_TTL_LEDGERS}" + ); +} + +/// A sell that reverts must leave the TTL exactly where it was — a failed +/// transaction rolls back its storage effects, extension included. +#[test] +fn failed_sell_does_not_extend_ttl() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, contract_id, creator, holder) = setup(&env); + + client.buy_key(&creator, &holder, &KEY_PRICE, &None); + + let ttl_initial = creator_ttl_remaining(&env, &contract_id, &creator); + advance_ledgers(&env, ttl_initial.saturating_sub(1).max(1)); + + let ttl_before_failed_sell = creator_ttl_remaining(&env, &contract_id, &creator); + + // `min_proceeds` above the achievable payout trips the slippage guard, so + // the sell reverts after the entrypoint has been entered. + let result = client.try_sell_key(&creator, &holder, &Some(KEY_PRICE * 100)); + assert!( + result.is_err() || matches!(result, Ok(Err(_))), + "sell should revert on slippage" + ); + + assert_eq!( + creator_ttl_remaining(&env, &contract_id, &creator), + ttl_before_failed_sell, + "a reverted sell must not extend the creator TTL" + ); +} + +/// A sell by a wallet holding no keys reverts before reaching the extension +/// call at the end of `sell_key`. +#[test] +fn sell_without_balance_does_not_extend_ttl() { + let env = soroban_sdk::Env::default(); + env.mock_all_auths(); + let (client, contract_id, creator, _holder) = setup(&env); + + let ttl_initial = creator_ttl_remaining(&env, &contract_id, &creator); + advance_ledgers(&env, ttl_initial.saturating_sub(1).max(1)); + + let ttl_before = creator_ttl_remaining(&env, &contract_id, &creator); + + let stranger = Address::generate(&env); + let result = client.try_sell_key(&creator, &stranger, &None); + assert!( + result.is_err() || matches!(result, Ok(Err(_))), + "selling with no balance should revert" + ); + + assert_eq!( + creator_ttl_remaining(&env, &contract_id, &creator), + ttl_before, + "a sell that never reaches the extension call must not extend the TTL" + ); +} diff --git a/docs/error-codes.md b/docs/error-codes.md index ed640795..84722926 100644 --- a/docs/error-codes.md +++ b/docs/error-codes.md @@ -45,6 +45,7 @@ Defined in [`creator-keys/src/lib.rs`](../creator-keys/src/lib.rs#L50-L83) as `p | `31` | `WhitelistOnly` | Buyer address is not in creator whitelist during whitelist window | Triggered in [`check_whitelist`](../creator-keys/src/lib.rs#L683) when whitelist is active and buyer is not allowed. | | `32` | `WhitelistTooLarge` | Whitelist configuration address count exceeds maximum limit | Triggered in [`validate_whitelist_config`](../creator-keys/src/lib.rs#L637) when address count `> MAX_WHITELIST_SIZE`. | | `33` | `AirdropRecipientLimitExceeded` | Airdrop recipient list length exceeds max limit per transaction | Triggered in [`airdrop_keys`](../creator-keys/src/lib.rs#L1730) when `recipients.len() > MAX_AIRDROP_RECIPIENT_LIMIT`. | +| `40` | `DisplayNameEmpty` | Creator display handle is blank (empty string or ASCII whitespace only) | Triggered in [`validate_creator_handle`](../creator-keys/src/lib.rs) before the length and character checks when the handle contains no non-whitespace bytes. | --- @@ -136,6 +137,7 @@ try { - `AlreadyRegistered` (Code 1) guards against re-registering an existing creator. Off-chain apps should call `is_registered(creator)` or `get_creator(creator)` prior to registration. - `NotRegistered` (Code 2) applies to trades, quotes, and management. Callers must register creators prior to key trading. - `HandleTooShort` (12), `HandleTooLong` (13), and `InvalidHandleCharacter` (14) are deterministic handle validation checks. Validate handles client-side (`/^[a-z0-9_]{3,32}$/`) before submission. +- `DisplayNameEmpty` (40) is checked ahead of (12) and (14): a handle that is empty or entirely ASCII whitespace reports this rather than `HandleTooShort` or `InvalidHandleCharacter`. ### Fees and Pricing - `FeeConfigNotSet` (7) and `KeyPriceNotSet` (5) are initialization gates. Detect these and inform users that pricing/fees are not yet configured.