diff --git a/src/v16_program.rs b/src/v16_program.rs index 6ce564a13..c76343b4d 100644 --- a/src/v16_program.rs +++ b/src/v16_program.rs @@ -55,7 +55,11 @@ pub mod constants { pub const HEADER_LEN: usize = 16; pub const WRAPPER_CONFIG_LEN: usize = 576; - pub const ASSET_ORACLE_PROFILE_LEN: usize = 400; + // GH#420: 400 -> 408 for the per-asset `creator_fee_claimable_atoms`. Safe + // because the profile lives inside the fixed 512-byte + // `ASSET_ORACLE_WRAPPER_LEN` slot and 112 of those bytes were spare — + // `MARKET_ASSET_SLOT_LEN` is unchanged and no offset moves. + pub const ASSET_ORACLE_PROFILE_LEN: usize = 408; pub const ASSET_ORACLE_WRAPPER_LEN: usize = 512; pub const MARKET_GROUP_LEN: usize = size_of::(); pub const MARKET_ASSET_SLOT_LEN: usize = size_of::>(); @@ -1371,6 +1375,30 @@ pub mod state { // (insurance/operator/backing/oracle) and itself, and can be burned (set to 0). Isolated: // it can never act on another asset. Set to the activator at creation. pub asset_admin: [u8; 32], + + /// GH#420: THIS asset's unclaimed creator share of trade fees, in collateral + /// atoms. Claimed via `WithdrawCreatorFee` (tag 90) by this asset's own + /// `asset_admin`. + /// + /// Previously every asset's creator cut accumulated into the single global + /// `WrapperConfigV16::creator_fee_claimable_atoms`, while the withdrawal + /// authority check named ASSET 0's admin only. In a multi-asset market that + /// meant the base deployer could drain fees earned on assets 1..N, and those + /// assets' creators could never claim their own. + /// + /// THERE IS NO LAYOUT COST. `ASSET_ORACLE_PROFILE_LEN` grows 400 -> 408, and + /// the profile is stored inside a fixed `ASSET_ORACLE_WRAPPER_LEN` (512) byte + /// slot — 112 bytes of it were spare, so `MARKET_ASSET_SLOT_LEN` is unchanged + /// and NO offset moves. This is the opposite of the config counter's + /// situation, where `WRAPPER_CONFIG_LEN` is exactly 576 and growing it would + /// shift `MARKET_GROUP_OFF` and brick every deployed market. + /// + /// Deployed markets have these bytes ZEROED (they were slot padding), so + /// after an in-place upgrade each asset reads 0 and accrues fresh — no + /// migration. The already-accrued GLOBAL balance is deliberately left in + /// `WrapperConfigV16::creator_fee_claimable_atoms` and stays claimable by + /// asset 0's admin, so nothing earned before this change is stranded. + pub creator_fee_claimable_atoms: u64, } /// Aggregate backing-domain accounting for an authority-controlled vault. @@ -2052,6 +2080,7 @@ pub mod state { oracle_leg_prices_e6: [0u64; ORACLE_LEG_CAP], oracle_leg_publish_times: [0i64; ORACLE_LEG_CAP], asset_admin: [0u8; 32], + creator_fee_claimable_atoms: 0, } } @@ -2089,6 +2118,7 @@ pub mod state { oracle_leg_prices_e6: config.oracle_leg_prices_e6, oracle_leg_publish_times: config.oracle_leg_publish_times, asset_admin: config.marketauth, + creator_fee_claimable_atoms: 0, } } @@ -4363,6 +4393,10 @@ pub mod ix { /// and a silent 0-atom transfer is a caller bug, not a claim. WithdrawCreatorFee { amount: u128, + /// GH#420: WHICH asset's creator fees. Appended after `amount`, so the + /// wire grows 1+16 -> 1+16+2 = 19 bytes and an old 17-byte caller fails + /// to decode rather than being silently treated as asset 0. + asset_index: u16, }, } @@ -4706,6 +4740,7 @@ pub mod ix { }, 90 => Self::WithdrawCreatorFee { amount: read_u128(&mut rest)?, + asset_index: read_u16(&mut rest)?, }, _ => return Err(ProgramError::InvalidInstructionData), }; @@ -5236,9 +5271,13 @@ pub mod ix { out.push(89); push_u16(&mut out, domain); } - Self::WithdrawCreatorFee { amount } => { + Self::WithdrawCreatorFee { + amount, + asset_index, + } => { out.push(90); push_u128(&mut out, amount); + push_u16(&mut out, asset_index); } } out @@ -7717,9 +7756,10 @@ pub mod processor { Instruction::ExpireBackingBucket { domain } => { handle_expire_backing_bucket(program_id, accounts, domain) } - Instruction::WithdrawCreatorFee { amount } => { - handle_withdraw_creator_fee(program_id, accounts, amount) - } + Instruction::WithdrawCreatorFee { + amount, + asset_index, + } => handle_withdraw_creator_fee(program_id, accounts, amount, asset_index), } } @@ -8275,7 +8315,19 @@ pub mod processor { // fit the 8 spare bytes of `_padding_split`. Overflow (either // the narrowing or the add) ERRORS the whole trade rather than // wrapping or saturating. - cfg.creator_fee_claimable_atoms = cfg + // GH#420: the creator cut accrues to THIS ASSET's profile, not to + // the market-wide config counter. The old global accumulator paid + // out against asset 0's `asset_admin` only, so in a multi-asset + // market the base deployer could drain fees earned on assets 1..N + // and those assets' creators could never claim their own. + // Mutate the `oracle_profile` ALREADY IN SCOPE rather than doing a + // fresh read/write here. That profile was read earlier in this + // function and is written back below; a separate write at this + // point is silently CLOBBERED by that later write-back, which is + // exactly what happened in the first version of this change and + // what `..._creator_fee_accrual_is_written_back_to_the_account_...` + // caught. Same pattern as the batch path. + oracle_profile.creator_fee_claimable_atoms = oracle_profile .creator_fee_claimable_atoms .checked_add( u64::try_from(creator_cut_total) @@ -8588,6 +8640,21 @@ pub mod processor { creator_cut_running_total = creator_cut_running_total .checked_add(split_leg.creator) .ok_or(PercolatorError::EngineArithmeticOverflow)?; + // GH#420: credit THIS leg's creator cut to THIS leg's asset + // profile. A batch can span several assets, so a single + // post-loop total cannot say which creator earned what — that + // is precisely how every asset's fees ended up in one pot + // payable to asset 0's admin. + // + // Persisted by the existing `write_oracle_profile_to_view` at + // the end of this same iteration; no extra write. + oracle_profile.creator_fee_claimable_atoms = oracle_profile + .creator_fee_claimable_atoms + .checked_add( + u64::try_from(split_leg.creator) + .map_err(|_| PercolatorError::EngineArithmeticOverflow)?, + ) + .ok_or(PercolatorError::EngineArithmeticOverflow)?; // NOTE (behaviour preserved): the creator leg used to be // credited only under `if taker_paid { .. } else if // maker_paid { .. }`, i.e. dropped when NEITHER side paid. @@ -8642,13 +8709,14 @@ pub mod processor { // counter is u64 because it had to fit the spare 8 bytes of // `_padding_split`. Overflow ERRORS the whole batch rather // than wrapping or saturating. - cfg.creator_fee_claimable_atoms = cfg - .creator_fee_claimable_atoms - .checked_add( - u64::try_from(creator_cut_running_total) - .map_err(|_| PercolatorError::EngineArithmeticOverflow)?, - ) - .ok_or(PercolatorError::EngineArithmeticOverflow)?; + // GH#420: intentionally NOT folded into the config counter here. + // The batch's creator cut is credited PER LEG to that leg's own + // asset profile inside the loop above, because a batch can span + // several assets and a single running total cannot say which + // creator earned what. `creator_cut_running_total` is retained + // solely as the write-back trigger below and as the value the + // conservation assertions compare against. + let _ = creator_cut_running_total; // CRITICAL: same write-back-forcing requirement as the // single-trade site -- a missed write-back here would // silently discard accrued fees for all four legs. @@ -11560,6 +11628,7 @@ pub mod processor { program_id: &Pubkey, accounts: &'a [AccountInfo<'a>], amount: u128, + asset_index: u16, ) -> ProgramResult { let authority = account(accounts, 0)?; let market_ai = account(accounts, 1)?; @@ -11607,8 +11676,20 @@ pub mod processor { // `UpdateAssetAuthority` -- the stake flow never touches it. It is the // one field that reliably tracks the creator through staking, and it // already gates other creator ops (e.g. RestartAssetOracle). - let asset0_profile = read_oracle_profile_from_view(&group, &cfg, 0)?; - if !live_authority_matches(&asset0_profile.asset_admin, authority.key) { + // GH#420: authorise against THIS ASSET's `asset_admin`, not asset 0's. + // + // Every asset's creator cut used to accrue into one market-wide counter + // while this check named asset 0 only, so in a multi-asset market the + // base deployer could withdraw fees earned on assets 1..N and those + // assets' creators could never claim their own. + // Range is enforced by the profile read itself (`read_asset_oracle_profile` + // bounds `asset_index` against the market's slot capacity), so there is no + // second bound here. An earlier draft added one against + // `config.max_market_slots`, which is NOT the same basis and rejected a + // legitimate asset 1 as InvalidInstruction — one invariant, one owner. + let mut claim_profile = + read_oracle_profile_from_view(&group, &cfg, asset_index as usize)?; + if !live_authority_matches(&claim_profile.asset_admin, authority.key) { return Err(PercolatorError::Unauthorized.into()); } verify_withdrawable_token_accounts( @@ -11628,7 +11709,25 @@ pub mod processor { // hence its own ordinal rather than the engine's // `EngineCounterUnderflow`, which stays reserved for the // fail-closed `checked_sub` below. - if amount > cfg.creator_fee_claimable_atoms as u128 { + // GH#420: the claimable pot is THIS asset's, plus — for asset 0 only — + // the pre-existing market-wide counter. + // + // Fees accrued BEFORE this change are all sitting in + // `cfg.creator_fee_claimable_atoms`, and asset 0's admin was the only + // party who could ever withdraw them. Leaving that balance claimable by + // asset 0 keeps the historical behaviour exactly as it was for the party + // who already had it, and strands nothing. Assets 1..N draw only from + // their own counter, which starts at zero on a deployed market because + // those bytes were slot padding. + let legacy_pot = if asset_index == 0 { + cfg.creator_fee_claimable_atoms + } else { + 0 + }; + let claimable = (claim_profile.creator_fee_claimable_atoms as u128) + .checked_add(legacy_pot as u128) + .ok_or(PercolatorError::EngineArithmeticOverflow)?; + if amount > claimable { return Err(PercolatorError::CreatorFeeOverClaim.into()); } let transfer_amount_u64 = amount_to_u64(amount)?; @@ -11642,13 +11741,30 @@ pub mod processor { group .withdraw_insurance_surplus_not_atomic(amount) .map_err(map_v16_error)?; - // `checked_sub` cannot fail after the clamp above; it is kept so a + // Debit THIS asset's counter first, then the legacy pot for whatever + // remains. Draining the per-asset counter first means a market that has + // migrated fully stops touching the legacy value at all, and the legacy + // balance can only ever shrink. + // + // `checked_sub` cannot fail after the clamp above; both are kept so a // future edit that weakens the clamp still fails closed rather than - // wrapping the counter to ~1.8e19 claimable atoms. - cfg.creator_fee_claimable_atoms = cfg + // wrapping a counter to ~1.8e19 claimable atoms. + let amount_u64 = amount as u64; + let from_asset = amount_u64.min(claim_profile.creator_fee_claimable_atoms); + claim_profile.creator_fee_claimable_atoms = claim_profile .creator_fee_claimable_atoms - .checked_sub(amount as u64) + .checked_sub(from_asset) .ok_or(PercolatorError::EngineCounterUnderflow)?; + let from_legacy = amount_u64 + .checked_sub(from_asset) + .ok_or(PercolatorError::EngineCounterUnderflow)?; + if from_legacy != 0 { + cfg.creator_fee_claimable_atoms = cfg + .creator_fee_claimable_atoms + .checked_sub(from_legacy) + .ok_or(PercolatorError::EngineCounterUnderflow)?; + } + write_oracle_profile_to_view(&mut group, asset_index as usize, &claim_profile)?; group.validate_shape().map_err(map_v16_error)?; (transfer_amount_u64, cfg) }; @@ -14118,6 +14234,7 @@ pub mod processor { insurance_authority: existing_profile.insurance_authority, insurance_operator: existing_profile.insurance_operator, asset_admin: existing_profile.asset_admin, + creator_fee_claimable_atoms: 0, backing_bucket_authority: existing_profile.backing_bucket_authority, oracle_authority: existing_profile.oracle_authority, max_staleness_secs, @@ -14240,6 +14357,7 @@ pub mod processor { insurance_authority: existing_profile.insurance_authority, insurance_operator: existing_profile.insurance_operator, asset_admin: existing_profile.asset_admin, + creator_fee_claimable_atoms: 0, backing_bucket_authority: existing_profile.backing_bucket_authority, oracle_authority: existing_profile.oracle_authority, max_staleness_secs: 0, @@ -14344,6 +14462,7 @@ pub mod processor { insurance_authority: existing_profile.insurance_authority, insurance_operator: existing_profile.insurance_operator, asset_admin: existing_profile.asset_admin, + creator_fee_claimable_atoms: 0, backing_bucket_authority: existing_profile.backing_bucket_authority, oracle_authority: existing_profile.oracle_authority, max_staleness_secs: 0, diff --git a/tests/v16_creator_fee_authority_guard.rs b/tests/v16_creator_fee_authority_guard.rs index cd9df6e59..c7946ece4 100644 --- a/tests/v16_creator_fee_authority_guard.rs +++ b/tests/v16_creator_fee_authority_guard.rs @@ -402,7 +402,10 @@ fn withdraw_creator_fee( let mut vault_auth = vault_authority_account(market); let mut token_program = token_program_account(); run_ix( - Instruction::WithdrawCreatorFee { amount }, + Instruction::WithdrawCreatorFee { + amount, + asset_index: 0, + }, &mut [ authority, market, diff --git a/tests/v16_creator_fee_isolation.rs b/tests/v16_creator_fee_isolation.rs index 0246ecdf7..adaa881ed 100644 --- a/tests/v16_creator_fee_isolation.rs +++ b/tests/v16_creator_fee_isolation.rs @@ -152,6 +152,11 @@ fn signer() -> TestAccount { TestAccount::new(Pubkey::new_unique(), Pubkey::new_unique(), 0).signer() } +/// A signer with a CHOSEN key — needed to sign as an asset's own `asset_admin`. +fn signer_with_key(key: Pubkey) -> TestAccount { + TestAccount::new(key, Pubkey::new_unique(), 0).signer() +} + fn market_account() -> TestAccount { let capacity = percolator_prog::constants::WRAPPER_MAX_PORTFOLIO_ASSETS as usize; TestAccount::new( @@ -266,8 +271,18 @@ fn run_ix(ix: Instruction, accounts: &mut [&mut TestAccount]) -> Result<(), Prog } fn default_init_market_ix() -> Instruction { + default_init_market_ix_with_assets(1) +} + +/// GH#420: the same fixture with a chosen asset count. +/// +/// The default is single-asset, which cannot express "asset 0's admin drains +/// asset 1's fees" at all — there is no asset 1, so the claim is rejected as +/// out-of-range and the test would look like it passed for the right reason +/// while proving nothing. +fn default_init_market_ix_with_assets(max_portfolio_assets: u16) -> Instruction { Instruction::InitMarket { - max_portfolio_assets: 1, + max_portfolio_assets, h_min: 0, h_max: 10, initial_price: 100, @@ -299,6 +314,17 @@ fn init_market(admin: &mut TestAccount, market: &mut TestAccount) -> Pubkey { mint_key } +fn init_market_two_assets(admin: &mut TestAccount, market: &mut TestAccount) -> Pubkey { + let mut mint = mint_account(); + let mint_key = mint.key; + run_ix( + default_init_market_ix_with_assets(2), + &mut [admin, market, &mut mint], + ) + .unwrap(); + mint_key +} + // ── The isolation tests ───────────────────────────────────────────────────── /// A nonzero insurance-withdraw cooldown is what makes tag 57 PERSIST the @@ -460,7 +486,10 @@ fn a_fully_drained_backstop_still_leaves_the_creator_claim_payable() { ); run_ix( - Instruction::WithdrawCreatorFee { amount: 100 }, + Instruction::WithdrawCreatorFee { + amount: 100, + asset_index: 0, + }, &mut [ &mut admin, &mut market, @@ -475,6 +504,101 @@ fn a_fully_drained_backstop_still_leaves_the_creator_claim_payable() { assert_eq!(cfg_end.creator_fee_claimable_atoms, 0); } +/// GH#420: asset 1's creator fees are claimable by ASSET 1's admin, and are NOT +/// drainable by asset 0's. +/// +/// This is the headline of the issue. Every asset's creator cut used to accumulate +/// into one market-wide counter while the withdrawal check named asset 0's +/// `asset_admin` only, so in a multi-asset market the base deployer could withdraw +/// fees earned on assets 1..N and those assets' creators could never claim theirs. +#[test] +fn asset0_admin_cannot_drain_asset1_creator_fees() { + install_clock_stub(); + let mut admin = signer(); + let mut market = market_account(); + let mint = init_market(&mut admin, &mut market); + + // Credit asset 1's creator pot directly; the accrual path is covered in + // v16_wrapper.rs, and this test is about WHO may withdraw it. + // Asset 1 needs a real `asset_admin`: a zeroed one matches NO signer + // (`live_authority_matches` rejects a zero authority), so the positive control + // below would fail for that reason rather than for the one under test. + let asset1_admin_key = Pubkey::new_unique(); + { + let mut p1 = state::read_asset_oracle_profile(&market.data, 1).unwrap(); + p1.creator_fee_claimable_atoms = 100; + p1.asset_admin = asset1_admin_key.to_bytes(); + state::write_asset_oracle_profile(&mut market.data, 1, &p1).unwrap(); + } + seed_both_pots(&mut market, 100, 150, 90); + + let mut dest = user_token_account(admin.key, mint, 0); + let mut vault = vault_token_account(&market, mint, 10_000); + let mut vault_auth = vault_authority_account(&market); + let mut token_program = token_program_account(); + + // Asset 0's admin claiming AGAINST ASSET 1 must be refused. Before GH#420 the + // authority check read asset 0's profile regardless of whose fees these were, + // so this succeeded and the atoms left with the wrong party. + let before = market.data.clone(); + let stolen = run_ix( + Instruction::WithdrawCreatorFee { + amount: 100, + asset_index: 1, + }, + &mut [ + &mut admin, + &mut market, + &mut dest, + &mut vault, + &mut vault_auth, + &mut token_program, + ], + ); + assert!( + stolen.is_err(), + "asset 0's admin must NOT be able to claim asset 1's creator fees" + ); + assert_eq!( + market.data, before, + "the refused claim must leave the market byte-identical" + ); + assert_eq!( + state::read_asset_oracle_profile(&market.data, 1) + .unwrap() + .creator_fee_claimable_atoms, + 100, + "asset 1's pot must be untouched" + ); + + // POSITIVE CONTROL: asset 1's OWN admin can claim it. Without this the + // rejection above would pass against a handler that refused everything. + let mut owner = signer_with_key(asset1_admin_key); + let mut dest1 = user_token_account(owner.key, mint, 0); + run_ix( + Instruction::WithdrawCreatorFee { + amount: 100, + asset_index: 1, + }, + &mut [ + &mut owner, + &mut market, + &mut dest1, + &mut vault, + &mut vault_auth, + &mut token_program, + ], + ) + .expect("asset 1's own admin must be able to claim asset 1's creator fees"); + assert_eq!( + state::read_asset_oracle_profile(&market.data, 1) + .unwrap() + .creator_fee_claimable_atoms, + 0, + "asset 1's pot must be drained by its own admin" + ); +} + /// The mirror of direction A, asserted here too because this binary is the only /// place both instructions can run against the same market: a creator claim /// must not shrink the backstop that tag 57 is entitled to spend afterwards. @@ -492,7 +616,10 @@ fn withdraw_creator_fee_leaves_the_backstop_fully_spendable_by_tag57() { let mut token_program = token_program_account(); run_ix( - Instruction::WithdrawCreatorFee { amount: 100 }, + Instruction::WithdrawCreatorFee { + amount: 100, + asset_index: 0, + }, &mut [ &mut admin, &mut market, diff --git a/tests/v16_fee_split.rs b/tests/v16_fee_split.rs index 1ea10cb05..810caaf02 100644 --- a/tests/v16_fee_split.rs +++ b/tests/v16_fee_split.rs @@ -286,6 +286,7 @@ fn withdraw_creator_fee_is_dispatch_tag_90_on_the_wire() { let encoded = Instruction::WithdrawCreatorFee { amount: 0x0102_0304_0506_0708_090a_0b0c_0d0e_0f10, + asset_index: 0x1234, } .encode(); assert_eq!( @@ -293,10 +294,20 @@ fn withdraw_creator_fee_is_dispatch_tag_90_on_the_wire() { "WithdrawCreatorFee must encode as dispatch tag 90 — the SDK, the keeper \ and any pre-signed transaction all hard-code this byte" ); + // GH#420: 17 -> 19 bytes. This IS a wire break, and a deliberate one — the + // creator claim now names WHICH asset's fees it is claiming, because a single + // market-wide counter could only ever pay one admin. `asset_index` is appended + // AFTER `amount`, so the tag byte and the u128 keep their offsets and an old + // 17-byte caller fails to DECODE rather than being silently read as asset 0. assert_eq!( encoded.len(), - 1 + 16, - "tag byte + a u128 amount; a length change is also a wire break" + 1 + 16 + 2, + "tag byte + u128 amount + u16 asset_index; a length change is a wire break" + ); + assert_eq!( + &encoded[17..19], + &0x1234u16.to_le_bytes(), + "asset_index is a little-endian u16 immediately after the amount" ); assert_eq!( &encoded[1..17], @@ -307,12 +318,26 @@ fn withdraw_creator_fee_is_dispatch_tag_90_on_the_wire() { // Decode direction, built from the literal byte rather than from encode(). let mut wire = vec![90u8]; wire.extend_from_slice(&7u128.to_le_bytes()); + wire.extend_from_slice(&3u16.to_le_bytes()); assert_eq!( Instruction::decode(&wire), - Ok(Instruction::WithdrawCreatorFee { amount: 7 }), + Ok(Instruction::WithdrawCreatorFee { + amount: 7, + asset_index: 3 + }), "byte 90 must dispatch to WithdrawCreatorFee" ); + // GH#420: the OLD 17-byte form must be REFUSED, not silently accepted as + // asset 0. A stale caller claiming against the wrong asset's pot is exactly + // the confusion this change exists to end. + let mut stale = vec![90u8]; + stale.extend_from_slice(&7u128.to_le_bytes()); + assert!( + Instruction::decode(&stale).is_err(), + "the pre-GH#420 17-byte payload must fail to decode" + ); + // And 90 must not have been taken from a neighbour: pin the two adjacent // fee-withdrawal tags this instruction was modelled on. assert_eq!( diff --git a/tests/v16_fork_bundles.rs b/tests/v16_fork_bundles.rs index b7c8bb0ef..71959f245 100644 --- a/tests/v16_fork_bundles.rs +++ b/tests/v16_fork_bundles.rs @@ -58,6 +58,7 @@ fn hybrid_profile(max_staleness_secs: u64) -> AssetOracleProfileV16 { oracle_leg_publish_times: [0i64; ORACLE_LEG_CAP], // v17: per-asset cold-storage admin key (collision matrix row N/A). asset_admin: [0u8; 32], + creator_fee_claimable_atoms: 0, } } diff --git a/tests/v16_wrapper.rs b/tests/v16_wrapper.rs index 84ca86e50..43590eaa3 100644 --- a/tests/v16_wrapper.rs +++ b/tests/v16_wrapper.rs @@ -384,6 +384,17 @@ fn vault_authority(market: &TestAccount) -> Pubkey { Pubkey::find_program_address(&[b"vault", market.key.as_ref()], &program_id()).0 } +/// GH#420: this asset's unclaimed creator fees. +/// +/// The counter moved from the market-wide `WrapperConfigV16` to each asset's own +/// `AssetOracleProfileV16`, because one global pot could only ever be paid out to +/// one admin — asset 0's — while every asset's trades fed it. +fn creator_claimable(market: &TestAccount, asset_index: usize) -> u64 { + state::read_asset_oracle_profile(&market.data, asset_index) + .unwrap() + .creator_fee_claimable_atoms +} + fn vault_token_account(market: &TestAccount, mint: Pubkey, amount: u64) -> TestAccount { TestAccount::new_with_data( canonical_vault_ata(&vault_authority(market), &mint), @@ -2041,7 +2052,8 @@ fn v16_wrapper_fee_redirect_policy_is_admin_gated_and_trade_fees_bypass_domain_b "the fixture must produce a nonzero creator leg, or the assertions below are no-ops" ); assert_eq!( - cfg_after_trade.creator_fee_claimable_atoms, expected_creator_cut as u64, + creator_claimable(&market, 0), + expected_creator_cut as u64, "the non-main asset's creator leg accrues to the claimable counter" ); assert_eq!( @@ -2141,7 +2153,8 @@ fn v16_wrapper_fee_redirect_policy_is_admin_gated_and_trade_fees_bypass_domain_b "the whole fee stays in insurance, unreachable through the domain-budget exit" ); assert_eq!( - cfg_end.creator_fee_claimable_atoms, expected_creator_cut as u64, + creator_claimable(&market, 0), + expected_creator_cut as u64, "and the creator's leg is still sitting on its own counter, claimable only via tag 90" ); } @@ -9748,6 +9761,7 @@ fn v16_wrapper_ewma_mark_profiles_reject_prices_above_engine_max() { oracle_leg_feeds: [[0u8; 32]; ORACLE_LEG_CAP], oracle_leg_prices_e6: [0u64; ORACLE_LEG_CAP], oracle_leg_publish_times: [0i64; ORACLE_LEG_CAP], + creator_fee_claimable_atoms: 0, }; assert!( state::validate_asset_oracle_profile(&profile).is_err(), @@ -17785,6 +17799,7 @@ fn setup_pinned_group_fresh_asset1(target_mark_e6: u64) -> (TestAccount, TestAcc backing_bucket_authority: admin.key.to_bytes(), oracle_authority: admin.key.to_bytes(), asset_admin: admin.key.to_bytes(), + creator_fee_claimable_atoms: 0, max_staleness_secs: 0, hybrid_soft_stale_slots: 0, mark_ewma_e6: 100, @@ -18001,6 +18016,7 @@ fn v16_wrapper_trade_fee_floor_uses_per_asset_dt_not_group_dt() { backing_bucket_authority: admin.key.to_bytes(), oracle_authority: admin.key.to_bytes(), asset_admin: admin.key.to_bytes(), + creator_fee_claimable_atoms: 0, max_staleness_secs: 0, hybrid_soft_stale_slots: 0, mark_ewma_e6: 100, @@ -18365,6 +18381,7 @@ fn v16_wrapper_protocol_fee_tradenocpi_skims_20pct_and_accrues_creator_leg_off_t ); let (cfg_before, group_before) = state::read_market(&market.data).unwrap(); + let creator_before = creator_claimable(&market, 0); assert_eq!(cfg_before.protocol_fee_accrued_atoms, 0); // account_a (long_owner/long_account) is the taker; size_q > 0 puts it in @@ -18417,7 +18434,7 @@ fn v16_wrapper_protocol_fee_tradenocpi_skims_20pct_and_accrues_creator_leg_off_t "fixture must produce a nonzero creator leg" ); assert_eq!( - cfg_after.creator_fee_claimable_atoms - cfg_before.creator_fee_claimable_atoms, + creator_claimable(&market, 0) - creator_before, expected_creator_cut as u64, "creator accrues exactly split_a.creator + split_b.creator into the claimable counter" ); @@ -18451,7 +18468,7 @@ fn v16_wrapper_protocol_fee_tradenocpi_skims_20pct_and_accrues_creator_leg_off_t // double-counted by the re-route. assert_eq!( expected_protocol_cut - + cfg_after.creator_fee_claimable_atoms as u128 + + creator_claimable(&market, 0) as u128 + expected_lp_cut + expected_insurance_cut, total_fee, @@ -18476,6 +18493,7 @@ fn v16_wrapper_protocol_fee_tradecpi_skims_20pct_and_accrues_creator_leg_off_the deposit(&mut owner_b, &mut market, &mut account_b, 10_000_000); let (cfg_before, group_before) = state::read_market(&market.data).unwrap(); + let creator_before = creator_claimable(&market, 0); // TradeCpi delegates to handle_trade_nocpi_zero_copy; account_a is always // the taker regardless of the matcher fill's sign convention. @@ -18514,7 +18532,7 @@ fn v16_wrapper_protocol_fee_tradecpi_skims_20pct_and_accrues_creator_leg_off_the "fixture must produce a nonzero creator leg" ); assert_eq!( - cfg_after.creator_fee_claimable_atoms - cfg_before.creator_fee_claimable_atoms, + creator_claimable(&market, 0) - creator_before, expected_creator_cut as u64, "creator accrues its configured share into the claimable counter on the CPI path too" ); @@ -18564,6 +18582,7 @@ fn v16_wrapper_protocol_fee_batchtradenocpi_skims_20pct_and_accrues_creator_leg_ ); let (cfg_before, group_before) = state::read_market(&market.data).unwrap(); + let creator_before = creator_claimable(&market, 0); run_ix( Instruction::BatchTradeNoCpi { @@ -18616,7 +18635,7 @@ fn v16_wrapper_protocol_fee_batchtradenocpi_skims_20pct_and_accrues_creator_leg_ "fixture must produce a nonzero creator leg" ); assert_eq!( - cfg_after.creator_fee_claimable_atoms - cfg_before.creator_fee_claimable_atoms, + creator_claimable(&market, 0) - creator_before, expected_creator_cut as u64, "batch loop must fold creator_cut_running_total into the claimable counter" ); @@ -18724,6 +18743,7 @@ fn v16_wrapper_protocol_fee_batchtradecpi_skims_20pct_and_accrues_creator_leg_of .unwrap(); let (cfg_before, group_before) = state::read_market(&market.data).unwrap(); + let creator_before = creator_claimable(&market, 0); let req_id = state::next_market_matcher_req_id(&market.data).unwrap(); let lp_account_id = { let bytes = delegate.key.to_bytes(); @@ -18776,7 +18796,7 @@ fn v16_wrapper_protocol_fee_batchtradecpi_skims_20pct_and_accrues_creator_leg_of // budget entirely). Kept current so un-ignoring this test, once a real // BPF/LiteSVM harness exists, does not start from a false expectation. assert_eq!( - cfg_after.creator_fee_claimable_atoms - cfg_before.creator_fee_claimable_atoms, + creator_claimable(&market, 0) - creator_before, expected_creator_cut as u64, "batch-CPI must accrue the creator leg to the claimable counter" ); @@ -19163,7 +19183,10 @@ fn withdraw_creator_fee( amount: u128, ) -> Result<(), ProgramError> { run_ix( - Instruction::WithdrawCreatorFee { amount }, + Instruction::WithdrawCreatorFee { + amount, + asset_index: 0, + }, &mut [authority, market, dest, vault, vault_auth, token_program], ) } @@ -19187,7 +19210,10 @@ fn withdraw_creator_fee_no_rollback( amount: u128, ) -> Result<(), ProgramError> { run_ix_no_rollback( - Instruction::WithdrawCreatorFee { amount }, + Instruction::WithdrawCreatorFee { + amount, + asset_index: 0, + }, &mut [authority, market, dest, vault, vault_auth, token_program], ) } @@ -19265,8 +19291,17 @@ fn v16_wrapper_creator_fee_accrual_is_written_back_to_the_account_and_accumulate // Raw slot in the market account: 16-byte header + 568-byte config prefix. const CLAIMABLE_OFF: usize = 16 + 568; + // GH#420: the counter moved from the market-wide config (bytes 568..776) to + // each asset's own profile, so this reads asset 0's. + // + // The test's point is preserved: `read_asset_oracle_profile` parses out of + // `market.data`, the RAW account bytes, so a missed write-back still reads + // back as 0/stale here exactly as it did before. What changed is WHERE the + // bytes live, not whether this proves they were persisted. let raw_counter = |data: &[u8]| -> u64 { - u64::from_le_bytes(data[CLAIMABLE_OFF..CLAIMABLE_OFF + 8].try_into().unwrap()) + state::read_asset_oracle_profile(data, 0) + .unwrap() + .creator_fee_claimable_atoms }; for expected_trades in 1..=2u128 { @@ -19295,7 +19330,8 @@ fn v16_wrapper_creator_fee_accrual_is_written_back_to_the_account_and_accumulate ); let (cfg_now, _) = state::read_market(&market.data).unwrap(); assert_eq!( - cfg_now.creator_fee_claimable_atoms, expected, + creator_claimable(&market, 0), + expected, "the parsed view must agree with the raw bytes" ); } @@ -19326,9 +19362,13 @@ fn v16_wrapper_creator_fee_accrual_overflow_rejects_the_trade_instead_of_wrappin 10_000_000, ); { - let (mut cfg, group) = state::read_market(&market.data).unwrap(); - cfg.creator_fee_claimable_atoms = u64::MAX; - state::write_market(&mut market.data, &cfg, &group).unwrap(); + // GH#420: saturate ASSET 0's counter, which is where the accrual now + // lands. Seeding the config counter would leave the asset counter at 0, + // the add would succeed, and this test would silently stop testing + // overflow at all. + let mut profile = state::read_asset_oracle_profile(&market.data, 0).unwrap(); + profile.creator_fee_claimable_atoms = u64::MAX; + state::write_asset_oracle_profile(&mut market.data, 0, &profile).unwrap(); } let before = market.data.clone(); @@ -19356,9 +19396,8 @@ fn v16_wrapper_creator_fee_accrual_overflow_rejects_the_trade_instead_of_wrappin market.data, before, "the rejected trade must not mutate the market" ); - let (cfg_after, _) = state::read_market(&market.data).unwrap(); assert_eq!( - cfg_after.creator_fee_claimable_atoms, + creator_claimable(&market, 0), u64::MAX, "the counter must be exactly u64::MAX still -- not wrapped to a small value" ); @@ -19387,9 +19426,13 @@ fn v16_wrapper_creator_fee_batch_accrual_overflow_rejects_the_batch_instead_of_w 10_000_000, ); { - let (mut cfg, group) = state::read_market(&market.data).unwrap(); - cfg.creator_fee_claimable_atoms = u64::MAX; - state::write_market(&mut market.data, &cfg, &group).unwrap(); + // GH#420: saturate ASSET 0's counter, which is where the accrual now + // lands. Seeding the config counter would leave the asset counter at 0, + // the add would succeed, and this test would silently stop testing + // overflow at all. + let mut profile = state::read_asset_oracle_profile(&market.data, 0).unwrap(); + profile.creator_fee_claimable_atoms = u64::MAX; + state::write_asset_oracle_profile(&mut market.data, 0, &profile).unwrap(); } let before = market.data.clone(); @@ -19419,8 +19462,8 @@ fn v16_wrapper_creator_fee_batch_accrual_overflow_rejects_the_batch_instead_of_w market.data, before, "the rejected batch must not mutate the market" ); - let (cfg_after, _) = state::read_market(&market.data).unwrap(); - assert_eq!(cfg_after.creator_fee_claimable_atoms, u64::MAX); + // GH#420: the saturated counter is asset 0's, not the config's. + assert_eq!(creator_claimable(&market, 0), u64::MAX); } /// A claim BELOW capacity pays out and decrements by exactly `amount`, and the @@ -19966,7 +20009,8 @@ fn v16_wrapper_creator_fee_end_to_end_trade_accrues_then_creator_claims_exactly_ let earned = total_fee * cfg_before.creator_share_bps as u128 / 10_000; assert_ne!(earned, 0); let (cfg_accrued, _) = state::read_market(&market.data).unwrap(); - assert_eq!(cfg_accrued.creator_fee_claimable_atoms as u128, earned); + // GH#420: the accrual now lands in asset 0's own profile, not the config. + assert_eq!(creator_claimable(&market, 0) as u128, earned); let mut dest = user_token_account(admin.key, mint, 0); let mut vault = vault_token_account(&market, mint, 1_000_000); @@ -20450,6 +20494,7 @@ fn v16_wrapper_creator_fee_batch_multi_leg_accrues_the_sum_of_every_leg() { ); let (cfg_before, group_before) = state::read_market(&market.data).unwrap(); + let creator_before = creator_claimable(&market, 0); assert_eq!(cfg_before.creator_fee_claimable_atoms, 0); // Unequal sizes => unequal per-leg creator cuts => the SUM is distinguishable @@ -20506,10 +20551,27 @@ fn v16_wrapper_creator_fee_batch_multi_leg_accrues_the_sum_of_every_leg() { this test exists to catch" ); + // GH#420: the legs are on DIFFERENT assets, so each asset's creator gets its + // own leg's cut. That is the whole point of the change — one shared pot could + // only ever be paid out to asset 0's admin, so asset 1's creator earned + // `creator_leg1` and could never claim it. + assert_eq!( + creator_claimable(&market, 0) - creator_before, + creator_leg0 as u64, + "asset 0 must receive exactly ITS leg's creator cut" + ); + assert_eq!( + creator_claimable(&market, 1), + creator_leg1 as u64, + "asset 1 must receive exactly ITS leg's creator cut — the bug was that this \ + landed in asset 0's pot" + ); + // CONSERVATION, kept from the original assertion: nothing is created or lost + // by splitting the pot, so the two assets together still hold the batch total. assert_eq!( - cfg_after.creator_fee_claimable_atoms - cfg_before.creator_fee_claimable_atoms, + (creator_claimable(&market, 0) - creator_before) + creator_claimable(&market, 1), creator_sum as u64, - "the batch fold must credit the SUM of every leg's creator cut, not just one leg" + "the per-asset credits must still sum to every leg's creator cut" ); // The other three legs are folded the same way, so pin them on the same // multi-leg batch: a mis-folded running total in any of them is the same bug.