From 9fd635b2cdda8eae72602afe1817657554a45086 Mon Sep 17 00:00:00 2001 From: vjuliaife Date: Sat, 25 Jul 2026 00:48:01 -0700 Subject: [PATCH] Fix referral TTL squatting, tier downgrade reversion, and impact rounding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - referral_storage: add self-service renew_code so a code owner can keep their CodeOwner entry's TTL alive without depending on trader activity, closing the window where a dormant code's TTL lapses and a different address can register the same code string and inherit any trader still linked to it. - referral_storage: add admin-only set_referrer_volume so a manual set_referrer_tier downgrade can be paired with a volume reset, since increment_referrer_volume's auto-upgrade otherwise re-evaluates the unchanged, still-large cumulative volume on the next trade and silently reverts the downgrade. - pricing_utils: round the token-unit conversion of a negative price impact (a charge) up in magnitude instead of down, matching how fee amounts round up elsewhere so the protocol never under-collects. apply_swap_impact_value, apply_position_impact_value, and the inline conversion in get_swap_output_amount now share one conversion helper that floors payouts and ceils charges. - docs/ttl-strategy.md: correct the blanket "persistent storage is never extend_ttl'd" claim — order_handler, data_store's bytes32-set helpers, and referral_storage all call it on specific keys — and precisely list which related keys in those same contracts still aren't covered. --- contracts/referral_storage/src/lib.rs | 183 ++++++++++++++++++++++++++ docs/ttl-strategy.md | 107 ++++++++++++--- libs/pricing_utils/src/lib.rs | 21 ++- 3 files changed, 290 insertions(+), 21 deletions(-) diff --git a/contracts/referral_storage/src/lib.rs b/contracts/referral_storage/src/lib.rs index 8d9c684..65b2aad 100644 --- a/contracts/referral_storage/src/lib.rs +++ b/contracts/referral_storage/src/lib.rs @@ -285,6 +285,32 @@ impl ReferralStorage { .publish_event(&CodeOwnershipTransferred { code, from, to }); } + /// Self-service keep-alive for a registered code (issue #445). + /// + /// `register_code` only sets the `CodeOwner` entry's initial TTL, and that + /// TTL is otherwise only bumped as a side effect of a trader interacting + /// with the code. A code nobody is actively trading with can therefore run + /// out its TTL; once archived, `register_code`'s existence check reads as + /// "unregistered" and a different address can claim the same code string, + /// silently taking over any trader still linked to it. Calling this lets + /// the current owner extend the TTL at will, independent of trader + /// activity. Only the current owner may call it. + pub fn renew_code(env: Env, caller: Address, code: Bytes) { + caller.require_auth(); + let key = ReferralKey::CodeOwner(code); + let owner: Address = env + .storage() + .persistent() + .get(&key) + .unwrap_or_else(|| panic_with_error!(&env, Error::CodeNotFound)); + if owner != caller { + panic_with_error!(&env, Error::NotCodeOwner); + } + env.storage() + .persistent() + .extend_ttl(&key, MIN_BUMP_THRESHOLD, PERSISTENT_BUMP_TARGET); + } + /// Return the owner address for a given referral code, or None if unregistered. pub fn get_code_owner(env: Env, code: Bytes) -> Option
{ env.storage() @@ -375,6 +401,32 @@ impl ReferralStorage { .unwrap_or(0u128) } + /// Admin-only override for a referrer's stored lifetime volume (issue #444). + /// + /// `increment_referrer_volume`'s auto-upgrade re-evaluates tier thresholds + /// purely from this value, which otherwise only ever grows. A manual + /// `set_referrer_tier` downgrade is therefore not durable on its own: the + /// very next trade reads the unchanged, still-large cumulative volume and + /// silently re-upgrades the referrer right back to (or above) the tier an + /// admin just removed. Call this alongside a manual tier change to make + /// the downgrade (or any other volume correction) stick. + pub fn set_referrer_volume(env: Env, admin: Address, referrer: Address, volume_usd: u128) { + admin.require_auth(); + let stored_admin: Address = env + .storage() + .instance() + .get(&InstanceKey::Admin) + .unwrap_or_else(|| panic_with_error!(&env, Error::NotInitialized)); + if admin != stored_admin { + panic_with_error!(&env, Error::Unauthorized); + } + let vol_key = ReferralKey::ReferrerVolume(referrer); + env.storage().persistent().set(&vol_key, &volume_usd); + env.storage() + .persistent() + .extend_ttl(&vol_key, MIN_BUMP_THRESHOLD, PERSISTENT_BUMP_TARGET); + } + /// Called by the authorized order_handler after each trade settlement. /// Increments `referrer`'s lifetime volume and auto-upgrades their tier if /// the new cumulative total crosses any tier threshold (tier only goes up). @@ -801,6 +853,53 @@ mod tests { assert_eq!(discount, 500); } + // ─── Issue #445: self-service code renewal ─────────────────────────────── + + /// The current owner can renew their own code without reverting. + #[test] + fn renew_code_owner_succeeds() { + let w = setup(); + let alice = Address::generate(&w.env); + let code = make_code(&w.env, 0x10); + client(&w).register_code(&alice, &code); + client(&w).renew_code(&alice, &code); + assert_eq!(client(&w).get_code_owner(&code), Some(alice)); + } + + /// A non-owner attempting to renew someone else's code must revert with NotCodeOwner. + #[test] + fn renew_code_non_owner_rejected() { + let w = setup(); + let alice = Address::generate(&w.env); + let mallory = Address::generate(&w.env); + let code = make_code(&w.env, 0x11); + client(&w).register_code(&alice, &code); + + let result = client(&w).try_renew_code(&mallory, &code); + assert_eq!( + result, + Err(Ok(soroban_sdk::Error::from_contract_error( + Error::NotCodeOwner as u32 + ))) + ); + } + + /// Renewing a code that was never registered must revert with CodeNotFound. + #[test] + fn renew_code_unregistered_rejected() { + let w = setup(); + let alice = Address::generate(&w.env); + let code = make_code(&w.env, 0x12); + + let result = client(&w).try_renew_code(&alice, &code); + assert_eq!( + result, + Err(Ok(soroban_sdk::Error::from_contract_error( + Error::CodeNotFound as u32 + ))) + ); + } + // ─── Issue #236: referral code length and character set validation ──────── /// Empty code must revert with EmptyCode. @@ -972,4 +1071,88 @@ mod tests { client(&w).increment_referrer_volume(&order_handler, &referrer, &1u128); assert_eq!(client(&w).get_referrer_cumulative_volume(&referrer), 2_001u128); } + + // ─── Issue #444: manual tier downgrade must be durable ─────────────────── + + /// Without a volume reset, the very next trade after a manual admin + /// downgrade silently re-upgrades the referrer — this is the bug being + /// fixed, captured so a regression is caught if the auto-upgrade path + /// changes. + #[test] + fn manual_downgrade_alone_is_reverted_by_next_trade() { + let w = setup(); + let referrer = Address::generate(&w.env); + let order_handler = Address::generate(&w.env); + + client(&w).set_order_handler(&w.admin, &order_handler); + client(&w).set_tier_upgrade_threshold(&w.admin, &1u32, &1_000u128); + + client(&w).increment_referrer_volume(&order_handler, &referrer, &1_000u128); + client(&w).set_referrer_tier(&w.admin, &referrer, &0u32); + + // Cumulative volume (1_000) still clears the tier-1 threshold, so an + // ordinary trade re-upgrades the referrer even though admin just + // downgraded them. + client(&w).increment_referrer_volume(&order_handler, &referrer, &1u128); + let code = Bytes::from_slice(&w.env, b"REFCODE2"); + let trader = Address::generate(&w.env); + client(&w).register_code(&referrer, &code); + client(&w).set_tier_config( + &w.admin, + &1u32, + &TierConfig { total_rebate_bps: 2_000, discount_share_bps: 5_000 }, + ); + client(&w).set_trader_referral_code(&trader, &code); + assert_eq!( + client(&w).get_trader_discount_bps(&trader), + 1_000, + "tier 1 discount must be back in effect: downgrade was reverted by the next trade" + ); + } + + /// Pairing the admin downgrade with `set_referrer_volume` makes it stick: + /// the next trade no longer re-crosses the threshold, so the referrer + /// stays at the admin-assigned tier. + #[test] + fn manual_downgrade_with_volume_reset_is_durable() { + let w = setup(); + let referrer = Address::generate(&w.env); + let order_handler = Address::generate(&w.env); + + client(&w).set_order_handler(&w.admin, &order_handler); + client(&w).set_tier_upgrade_threshold(&w.admin, &1u32, &1_000u128); + + client(&w).increment_referrer_volume(&order_handler, &referrer, &1_000u128); + client(&w).set_referrer_tier(&w.admin, &referrer, &0u32); + client(&w).set_referrer_volume(&w.admin, &referrer, &0u128); + + client(&w).increment_referrer_volume(&order_handler, &referrer, &1u128); + let code = Bytes::from_slice(&w.env, b"REFCODE3"); + let trader = Address::generate(&w.env); + client(&w).register_code(&referrer, &code); + client(&w).set_tier_config( + &w.admin, + &1u32, + &TierConfig { total_rebate_bps: 2_000, discount_share_bps: 5_000 }, + ); + client(&w).set_trader_referral_code(&trader, &code); + assert_eq!( + client(&w).get_trader_discount_bps(&trader), + 0, + "tier 0 must hold: volume reset stops the auto-upgrade from re-crossing the threshold" + ); + assert_eq!(client(&w).get_referrer_cumulative_volume(&referrer), 1u128); + } + + /// A non-admin caller must not be able to override a referrer's stored volume. + #[test] + #[should_panic] + fn set_referrer_volume_non_admin_reverts() { + let w = setup(); + let referrer = Address::generate(&w.env); + let impostor = Address::generate(&w.env); + ReferralStorageClient::new(&w.env, &w.handler).set_referrer_volume( + &impostor, &referrer, &0u128, + ); + } } diff --git a/docs/ttl-strategy.md b/docs/ttl-strategy.md index fb4affc..a798a49 100644 --- a/docs/ttl-strategy.md +++ b/docs/ttl-strategy.md @@ -5,12 +5,19 @@ sequences. Rent is paid when entries are created, enlarged, or extended. This document describes the policy implemented by the current code, not an intended future policy. -> **Current-state warning:** the protocol does not yet renew instance storage or -> long-lived persistent entries. There are no `MIN_POSITION_TTL`, +> **Current-state warning:** the protocol does not yet renew instance storage, +> and most long-lived persistent entries still receive only the network's +> default TTL at write time. There are no `MIN_POSITION_TTL`, > `MAX_POSITION_TTL`, `bump_core_ttl`, or `bump_persistent_ttl` definitions in -> the Soroban workspace. Only oracle prices and token allowances explicitly -> extend TTL. Operators must monitor the remaining TTL of contract instances and -> critical persistent entries until a renewal policy is implemented. +> the Soroban workspace, and no repository-wide renewal policy. Beyond oracle +> prices and token allowances, `order_handler`, `data_store`, and +> `referral_storage` each define their own local +> `PERSISTENT_BUMP_TARGET`/`MIN_BUMP_THRESHOLD` pair and call `extend_ttl` on +> specific keys — see "Persistent storage" below for exactly which ones, and +> which related keys in those same contracts are still uncovered. Operators +> must monitor the remaining TTL of contract instances and every persistent +> entry without a documented bump until a repository-wide renewal policy +> exists. ## Storage tiers used by the protocol @@ -22,7 +29,7 @@ a protocol upgrade. Do not hard-code them in keepers. | Tier | Current uses | Cost/lifetime characteristics | Current bump policy | |---|---|---|---| | Instance | Initialization flag, admin, role store, data store, oracle and vault/handler addresses; market-token metadata; test-token pause state; selected `data_store` configuration values | One contract-instance ledger entry backs all instance keys. Efficient for small, frequently read shared configuration, but growing it increases the size/rent of the shared entry. Its TTL is tied to the contract instance. | **None.** No production entrypoint calls `env.storage().instance().extend_ttl(...)`. | -| Persistent | Positions and orders in `order_handler`; pending deposits and withdrawals; `data_store` pool amounts, OI, factors, market metadata and index sets; roles, referrals, balances and token supply | Each key has an independent TTL and can be archived. Appropriate for durable protocol/user state, but every key must be monitored and renewed independently. | **None.** The current contracts do not call `persistent().extend_ttl(...)`. Reads and writes must not be assumed to renew entries automatically. | +| Persistent | Positions and orders in `order_handler`; pending deposits and withdrawals; `data_store` pool amounts, OI, factors, market metadata and index sets; roles, referrals, balances and token supply | Each key has an independent TTL and can be archived. Appropriate for durable protocol/user state, but every key must be monitored and renewed independently. | **Partial.** `order_handler` calls `extend_ttl` on `Order`, `OrderExpiry`, and `OrderFrozen`. `data_store`'s bytes32-set helpers (`add_bytes32_to_set`, `remove_bytes32_from_set`, and the bytes32-set getters) call it on the set entry itself, which covers the `order_list_key`/`account_order_list_key` and `position_list_key`/`account_position_list_key` enumeration indexes. `referral_storage` calls it on `CodeOwner`, `TraderCode`, `ReferrerTier`, `TierConfig`, `ReferrerVolume`, and `TierUpgradeThreshold`. Positions themselves, deposits, withdrawals, roles, token balances, `data_store`'s scalar/address-set accounting (pool amounts, OI, factors, prices), and a few specific paths noted under "Persistent storage" below are **not** covered. Reads and writes must not be assumed to renew an entry unless the exact code path calls `extend_ttl`. | | Temporary | Signed oracle prices; SEP-41 allowances in `market_token` and `test_token` | Lowest intended lifetime. Expired temporary entries are permanently removed and cannot be restored, which is desirable for stale prices and allowances. | Oracle prices are extended to 120 ledgers. Allowances are extended to their caller-supplied expiration ledger. | The unused `libs/storage_ttl` crate is not part of the root Cargo workspace and @@ -43,8 +50,13 @@ cadence. Ledger close time varies, so ledgers—not minutes—are authoritative. `MIN_POSITION_TTL` and `MAX_POSITION_TTL` are **not defined**. Positions receive the network's minimum persistent TTL when first written and are not explicitly -extended afterward. The same is true for orders, deposits, withdrawals, -balances, roles, and most `data_store` values. +extended afterward — `order_handler`'s only position-TTL entrypoint, +`bump_position_ttl`, rewrites the stored value with a plain `set()` but never +calls `extend_ttl`. The same is true for deposits, withdrawals, balances, +roles, and most `data_store` values. Orders and referral records are the +exception: see "Persistent storage" below for the specific keys `order_handler` +and `referral_storage` do extend, and which related keys in those same +contracts they don't. The effective wall-clock time for a network value is approximately: @@ -80,8 +92,52 @@ share one ledger entry and one lifetime. ### Persistent storage -There is currently no bump for durable keys. A future helper must accept the -exact typed storage key and extend the same entry that was read or written: +Most durable keys still have no bump — see "Current gaps" below — but three +contracts already extend specific keys inline, each with its own local +`PERSISTENT_BUMP_TARGET` (~30 days) / `MIN_BUMP_THRESHOLD` (~15 days) pair +rather than a shared helper: + +- `order_handler` extends `OrderStorageKey::Order` on `create_order` and + `update_order`, `OrderStorageKey::OrderExpiry` on `create_order` when the + caller supplies an expiry, and `OrderStorageKey::OrderFrozen` on + `freeze_order`. `PositionStorageKey::Position` is **not** extended this way: + the only position-TTL entrypoint, `bump_position_ttl`, rewrites the value + with a plain `set()` rather than calling `extend_ttl`, and the read-only + `get_order`/`get_position` getters don't renew anything either. +- `data_store`'s `DataKey::B32Set` helpers (`add_bytes32_to_set`, + `remove_bytes32_from_set`, `get_bytes32_set_count`, `get_bytes32_set_at`, + `contains_bytes32`) extend the set entry itself on every call. Every caller + that adds/removes an order or position to/from an enumeration index goes + through these — `order_handler`'s `order_list_key`/`account_order_list_key` + and `increase_position_utils`/`decrease_position_utils`'s + `position_list_key`/`account_position_list_key` are all covered this way. + The equivalent `DataKey::AddrSet` helpers (`add_address_to_set`, + `remove_address_from_set`) do **not** call `extend_ttl`, nor do any of + `data_store`'s scalar getters/setters (`get_u128`/`set_u128`, + `apply_delta_to_u128`, `get_address`/`set_address`, etc.) — pool amounts, + OI, factors, and other `data_store` accounting values have no bump. +- `referral_storage` extends `CodeOwner`, `TraderCode`, `ReferrerTier`, + `TierConfig`, and `ReferrerVolume` on their write paths and again on read + paths that touch them (`get_trader_referrer`, `get_trader_discount_bps`, + `renew_code`), and extends `TierUpgradeThreshold` for every tier the + auto-upgrade scan in `increment_referrer_volume` reads. It does **not** + extend `CodeOwner`'s TTL inside `transfer_code_ownership`, so a code that + changes hands but is never subsequently read or renewed keeps whatever TTL + it already had; the plain `get_trader_referral_code`/`get_code_owner` + getters likewise don't renew anything. + +Example of the pattern used by all three contracts: + +```rust,ignore +env.storage().persistent().extend_ttl( + &OrderStorageKey::Order(key), + MIN_BUMP_THRESHOLD, + PERSISTENT_BUMP_TARGET, +); +``` + +A future repository-wide helper must still accept the exact typed storage key +and extend the same entry that was read or written: ```rust,ignore env.storage().persistent().extend_ttl( @@ -92,10 +148,13 @@ env.storage().persistent().extend_ttl( ``` It must be called for every durable entry touched by a successful operation, -including both the primary object and its enumeration/index keys. Extending a -position does not extend its account-position index in `data_store`, and vice -versa. This is why a generic helper cannot infer the full set of entries to -renew. +including both the primary object and its enumeration/index keys — they are +independent entries with independent TTLs. Today a position open/close does +extend the `data_store` account-position index (via `add_bytes32_to_set`/ +`remove_bytes32_from_set`), but that same operation does **not** extend the +position's own `PositionStorageKey::Position` entry, and nothing about the +index write implies the primary object is covered too. This is why a generic +helper cannot infer the full set of entries to renew from any single key. ### Temporary storage @@ -227,10 +286,22 @@ The documentation exposes, but does not silently fix, these implementation gaps: 1. No automatic instance-storage renewal. -2. No persistent TTL renewal for positions or their indexes. -3. No persistent TTL renewal for orders, deposits, withdrawals, roles, token - balances, or `data_store` accounting. -4. No repository-wide minimum/maximum TTL policy constants. +2. No persistent TTL renewal for positions themselves + (`PositionStorageKey::Position`). The `data_store` account-position and + position-list indexes that reference them **are** renewed, as a side + effect of `add_bytes32_to_set`/`remove_bytes32_from_set` on every position + open/close — but the primary position entry is not, so the index can + outlive the object it points to. +3. No persistent TTL renewal for deposits, withdrawals, roles, token + balances, or most `data_store` accounting (pool amounts, OI, factors, + prices) and its `AddrSet` helpers. Orders, `data_store`'s `B32Set` + enumeration indexes, and referral-code/tier/volume records are a partial + exception — see "Persistent storage" above for exactly which keys + `order_handler`, `data_store`, and `referral_storage` cover, and which + related keys in those same contracts they don't. +4. No repository-wide minimum/maximum TTL policy constants — `order_handler`, + `data_store`, and `referral_storage` each hard-code their own, identical + `PERSISTENT_BUMP_TARGET`/`MIN_BUMP_THRESHOLD` pair rather than sharing one. 5. No on-chain TTL query/maintenance entrypoint or bundled off-chain monitor. Adding those mechanisms changes runtime behavior and rent costs and should be a diff --git a/libs/pricing_utils/src/lib.rs b/libs/pricing_utils/src/lib.rs index f768c16..f8d50de 100644 --- a/libs/pricing_utils/src/lib.rs +++ b/libs/pricing_utils/src/lib.rs @@ -76,6 +76,21 @@ fn compute_impact_usd( } } +// ─── USD → token conversion for signed price impact ────────────────────────── + +/// Convert a signed USD price impact to token units, rounding correctly for +/// the sign: a positive impact (payout to the trader) floors, while a +/// negative impact (a charge) takes the ceiling of its magnitude before +/// re-applying the sign — mirroring how fee amounts always round up so the +/// protocol never under-collects. +fn convert_impact_usd_to_tokens(env: &Env, impact_usd: i128, token_price: i128) -> i128 { + if impact_usd >= 0 { + mul_div_wide(env, impact_usd, TOKEN_PRECISION, token_price) + } else { + -mul_div_wide_up(env, -impact_usd, TOKEN_PRECISION, token_price) + } +} + // ─── Swap price impact ──────────────────────────────────────────────────────── #[allow(clippy::too_many_arguments)] @@ -139,7 +154,7 @@ pub fn apply_swap_impact_value( return 0; } // Convert USD impact to token amount - let impact_amount = mul_div_wide(env, impact_usd, TOKEN_PRECISION, token_price); + let impact_amount = convert_impact_usd_to_tokens(env, impact_usd, token_price); // Positive impact → paid from pool (reduce pool); negative → paid into pool (increase pool) let delta = -impact_amount; @@ -186,7 +201,7 @@ pub fn get_swap_output_amount( env, data_store, market, token_in, token_out, amount_in, price_in, price_out, ); let impact_amount = if price_out > 0 { - mul_div_wide(env, impact_usd, TOKEN_PRECISION, price_out) + convert_impact_usd_to_tokens(env, impact_usd, price_out) } else { 0 }; @@ -268,7 +283,7 @@ pub fn apply_position_impact_value( if impact_usd == 0 || index_token_price == 0 { return 0; } - let impact_amount = mul_div_wide(env, impact_usd, TOKEN_PRECISION, index_token_price); + let impact_amount = convert_impact_usd_to_tokens(env, impact_usd, index_token_price); let delta = -impact_amount; // positive impact → pool shrinks; negative → pool grows DataStoreClient::new(env, data_store).apply_delta_to_u128( caller,