From 99b674d3e482f38dd08aa6e9f891e676018688b7 Mon Sep 17 00:00:00 2001 From: Topmatrixmor2014 Date: Fri, 26 Jun 2026 07:51:48 +0000 Subject: [PATCH] test: add exact-delta invariants for transfer_keys sender/recipient (#419) Adds six integration tests in tests/key_transfer_sender_balance_decrement.rs that assert transfer_keys decrements the sender's key balance by EXACTLY the transferred amount and increments the recipient's balance by the same amount, while leaving total creator supply untouched. Each test computes an explicit before/after delta on KeyBalance and TotalKeySupply and asserts the delta equals the transfer amount, catching off-by-one regressions the existing absolute post-condition tests in transfer_keys.rs cannot detect. Issue acceptance criteria (#419): 1. sender balance decremented by exactly the transfer amount 2. recipient balance incremented by exactly the transfer amount 3. total supply unchanged after transfer Tests cover: - single-key transfer (sender 5 -> 4, recipient 0 -> 1, supply 5) - multi-key transfer (sender 10 -> 5, recipient 3 -> 8, supply 13) - full-drain transfer (sender 4 -> 0, recipient +4, supply unchanged) - three sequential calls (2 + 3 + 1) accumulating on the sender - per-creator isolation (transfer under creator A leaves creator B intact) - reverted call on insufficient balance (no state mutation) Repair work required to land these tests: - Remove stale duplicate transfer_keys in lib.rs that caused duplicate symbol errors under #[contractimpl]. The duplicate (env, from, to, creator, amount) was unreachable and inconsistent with the canonical (env, creator, from, to, amount) signature referenced by 22 existing tests and the KeysTransferredEvent topic name. Kept the canonical impl; deleted the stale one. - Update tests/peer_to_peer_transfer_supply_regression.rs to call the canonical signature (was using the old arg order). - Sync documentation: docs/key-transfer-authorization.md and docs/authorization-model.md had stale ZeroAddress/NotPositiveAmount error references and the old arg order wired into table rows and prose. Updated signature, code snippets, rationale paragraph, rejection bullet, and summary table to point at the canonical (creator, from, to, amount) signature and the dedicated ContractError::SelfTransfer / ContractError::ZeroTransferAmount variants. Verify: cargo fmt --check / cargo clippy --workspace --all-targets -- -D warnings / cargo test --workspace all pass. --- creator-keys/src/lib.rs | 65 --- ..._buy_key_unregistered_creator_fails.1.json | 161 +++++++- ...upply_three_buyers_sum_equals_total.1.json | 248 ++++++------ .../key_transfer_sender_balance_decrement.rs | 377 ++++++++++++++++++ ...peer_to_peer_transfer_supply_regression.rs | 2 +- docs/authorization-model.md | 4 +- docs/key-transfer-authorization.md | 14 +- 7 files changed, 671 insertions(+), 200 deletions(-) create mode 100644 creator-keys/tests/key_transfer_sender_balance_decrement.rs diff --git a/creator-keys/src/lib.rs b/creator-keys/src/lib.rs index 90cd751..648b9b6 100644 --- a/creator-keys/src/lib.rs +++ b/creator-keys/src/lib.rs @@ -1210,71 +1210,6 @@ impl CreatorKeysContract { Ok(profile.supply) } - /// Transfers creator keys between two wallets without changing total supply. - pub fn transfer_keys( - env: Env, - from: Address, - to: Address, - creator: Address, - amount: u32, - ) -> Result<(), ContractError> { - from.require_auth(); - assert_not_paused(&env)?; - - if amount == 0 { - return Err(ContractError::NotPositiveAmount); - } - if from == to { - return Err(ContractError::ZeroAddress); - } - - let mut profile = read_registered_creator_profile(&env, &creator)?; - let from_key = constants::storage::key_balance(&creator, &from); - let to_key = constants::storage::key_balance(&creator, &to); - - let from_balance: u32 = env.storage().persistent().get(&from_key).unwrap_or(0); - if from_balance < amount { - return Err(ContractError::InsufficientBalance); - } - - let to_balance: u32 = env.storage().persistent().get(&to_key).unwrap_or(0); - - settle_holder_dividends(&env, &creator, &from, from_balance)?; - settle_holder_dividends(&env, &creator, &to, to_balance)?; - - let new_from_balance = from_balance - .checked_sub(amount) - .ok_or(ContractError::SellUnderflow)?; - let new_to_balance = to_balance - .checked_add(amount) - .ok_or(ContractError::Overflow)?; - - if new_from_balance == 0 { - profile.holder_count = profile - .holder_count - .checked_sub(1) - .ok_or(ContractError::SellUnderflow)?; - } - if to_balance == 0 { - profile.holder_count = profile - .holder_count - .checked_add(1) - .ok_or(ContractError::Overflow)?; - } - - let profile_key = constants::storage::creator(&creator); - env.storage().persistent().set(&profile_key, &profile); - env.storage().persistent().set(&from_key, &new_from_balance); - env.storage().persistent().set(&to_key, &new_to_balance); - - env.events() - .publish(events::transfer_event_topics(&creator, &from), (to, amount)); - - extend_creator_ttl(&env, &creator); - - Ok(()) - } - /// Creator-only buyback that burns keys from the creator's own held balance. /// /// The creator pays the current gross buyback cost plus protocol fee, while the diff --git a/creator-keys/test_snapshots/test_buy_key_unregistered_creator_fails.1.json b/creator-keys/test_snapshots/test_buy_key_unregistered_creator_fails.1.json index 15c453a..504c99b 100644 --- a/creator-keys/test_snapshots/test_buy_key_unregistered_creator_fails.1.json +++ b/creator-keys/test_snapshots/test_buy_key_unregistered_creator_fails.1.json @@ -1,6 +1,6 @@ { "generators": { - "address": 4, + "address": 5, "nonce": 0 }, "auth": [ @@ -30,6 +30,37 @@ } ] ], + [ + [ + "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M", + { + "function": { + "contract_fn": { + "contract_address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "function_name": "set_fee_config", + "args": [ + { + "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" + }, + { + "u32": 9000 + }, + { + "u32": 1000 + } + ] + } + }, + "sub_invocations": [] + } + ] + ], + [], + [], + [], + [], + [], + [], [] ], "ledger": { @@ -42,6 +73,62 @@ "min_temp_entry_ttl": 16, "max_entry_ttl": 6312000, "ledger_entries": [ + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": { + "vec": [ + { + "symbol": "FeeConfig" + } + ] + }, + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": { + "vec": [ + { + "symbol": "FeeConfig" + } + ] + }, + "durability": "persistent", + "val": { + "map": [ + { + "key": { + "symbol": "creator_bps" + }, + "val": { + "u32": 9000 + } + }, + { + "key": { + "symbol": "protocol_bps" + }, + "val": { + "u32": 1000 + } + } + ] + } + } + }, + "ext": "v0" + }, + 4095 + ] + ], [ { "contract_data": { @@ -84,6 +171,45 @@ 4095 ] ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": { + "vec": [ + { + "symbol": "ProtocolStateVersion" + } + ] + }, + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": { + "vec": [ + { + "symbol": "ProtocolStateVersion" + } + ] + }, + "durability": "persistent", + "val": { + "u32": 2 + } + } + }, + "ext": "v0" + }, + 4095 + ] + ], [ { "contract_data": { @@ -149,6 +275,39 @@ 6311999 ] ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M", + "key": { + "ledger_key_nonce": { + "nonce": 5541220902715666415 + } + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M", + "key": { + "ledger_key_nonce": { + "nonce": 5541220902715666415 + } + }, + "durability": "temporary", + "val": "void" + } + }, + "ext": "v0" + }, + 6311999 + ] + ], [ { "contract_code": { diff --git a/creator-keys/test_snapshots/test_supply_three_buyers_sum_equals_total.1.json b/creator-keys/test_snapshots/test_supply_three_buyers_sum_equals_total.1.json index 69bd085..2cf17ea 100644 --- a/creator-keys/test_snapshots/test_supply_three_buyers_sum_equals_total.1.json +++ b/creator-keys/test_snapshots/test_supply_three_buyers_sum_equals_total.1.json @@ -1,6 +1,6 @@ { "generators": { - "address": 6, + "address": 3, "nonce": 0 }, "auth": [ @@ -56,7 +56,7 @@ ], [ [ - "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4", + "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ", { "function": { "contract_fn": { @@ -67,7 +67,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" }, { "i128": { @@ -85,7 +85,7 @@ ], [ [ - "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM", + "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL", { "function": { "contract_fn": { @@ -96,7 +96,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" }, { "i128": { @@ -114,7 +114,7 @@ ], [ [ - "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", + "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM", { "function": { "contract_fn": { @@ -125,7 +125,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" }, { "i128": { @@ -156,6 +156,105 @@ "min_temp_entry_ttl": 16, "max_entry_ttl": 6312000, "ledger_entries": [ + [ + { + "contract_data": { + "contract": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM", + "key": { + "ledger_key_nonce": { + "nonce": 2032731177588607455 + } + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM", + "key": { + "ledger_key_nonce": { + "nonce": 2032731177588607455 + } + }, + "durability": "temporary", + "val": "void" + } + }, + "ext": "v0" + }, + 6311999 + ] + ], + [ + { + "contract_data": { + "contract": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ", + "key": { + "ledger_key_nonce": { + "nonce": 1033654523790656264 + } + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ", + "key": { + "ledger_key_nonce": { + "nonce": 1033654523790656264 + } + }, + "durability": "temporary", + "val": "void" + } + }, + "ext": "v0" + }, + 6311999 + ] + ], + [ + { + "contract_data": { + "contract": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL", + "key": { + "ledger_key_nonce": { + "nonce": 4837995959683129791 + } + }, + "durability": "temporary" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL", + "key": { + "ledger_key_nonce": { + "nonce": 4837995959683129791 + } + }, + "durability": "temporary", + "val": "void" + } + }, + "ext": "v0" + }, + 6311999 + ] + ], [ { "contract_data": { @@ -263,7 +362,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" } ] }, @@ -286,7 +385,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" } ] }, @@ -317,7 +416,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" } ] }, @@ -340,7 +439,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" } ] }, @@ -371,7 +470,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" } ] }, @@ -394,7 +493,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" } ] }, @@ -425,7 +524,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" } ] }, @@ -448,7 +547,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" } ] }, @@ -479,7 +578,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" } ] }, @@ -502,7 +601,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" } ] }, @@ -533,7 +632,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" } ] }, @@ -556,7 +655,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" } ] }, @@ -587,7 +686,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" } ] }, @@ -610,7 +709,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4" + "address": "GARHLJ522ZQPQ3AKXY3XDEFA4TX44RHYIWQUN5ML7ZC7NXYDPNR3ADMM" } ] }, @@ -638,7 +737,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" } ] }, @@ -661,7 +760,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM" + "address": "GCELBYPGRABPAIAZ23PE2C3VUPDRHERUUJZOZHSTNJ7TVH377HCXW2UJ" } ] }, @@ -689,7 +788,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" } ] }, @@ -712,7 +811,7 @@ "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" }, { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4" + "address": "GCVAHVWNJAU46DQ5IMJTHPIF5NFEAQWONYSR6GERCIGOK2MR2MBB7OEL" } ] }, @@ -867,105 +966,6 @@ 6311999 ] ], - [ - { - "contract_data": { - "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4", - "key": { - "ledger_key_nonce": { - "nonce": 1033654523790656264 - } - }, - "durability": "temporary" - } - }, - [ - { - "last_modified_ledger_seq": 0, - "data": { - "contract_data": { - "ext": "v0", - "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAITA4", - "key": { - "ledger_key_nonce": { - "nonce": 1033654523790656264 - } - }, - "durability": "temporary", - "val": "void" - } - }, - "ext": "v0" - }, - 6311999 - ] - ], - [ - { - "contract_data": { - "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM", - "key": { - "ledger_key_nonce": { - "nonce": 4837995959683129791 - } - }, - "durability": "temporary" - } - }, - [ - { - "last_modified_ledger_seq": 0, - "data": { - "contract_data": { - "ext": "v0", - "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAK3IM", - "key": { - "ledger_key_nonce": { - "nonce": 4837995959683129791 - } - }, - "durability": "temporary", - "val": "void" - } - }, - "ext": "v0" - }, - 6311999 - ] - ], - [ - { - "contract_data": { - "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", - "key": { - "ledger_key_nonce": { - "nonce": 2032731177588607455 - } - }, - "durability": "temporary" - } - }, - [ - { - "last_modified_ledger_seq": 0, - "data": { - "contract_data": { - "ext": "v0", - "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDR4", - "key": { - "ledger_key_nonce": { - "nonce": 2032731177588607455 - } - }, - "durability": "temporary", - "val": "void" - } - }, - "ext": "v0" - }, - 6311999 - ] - ], [ { "contract_code": { diff --git a/creator-keys/tests/key_transfer_sender_balance_decrement.rs b/creator-keys/tests/key_transfer_sender_balance_decrement.rs new file mode 100644 index 0000000..544a986 --- /dev/null +++ b/creator-keys/tests/key_transfer_sender_balance_decrement.rs @@ -0,0 +1,377 @@ +//! Issue #419 — Unit tests for key transfer sender balance decrement. +//! +//! Verifies that `transfer_keys` decrements the sender's key balance by exactly +//! the transferred amount and increments the recipient's balance by the same +//! amount, while leaving total creator supply unchanged. Every test computes an +//! explicit before/after `delta = before - after` (or `after - before` for the +//! recipient) and asserts `delta == amount` so off-by-one regressions in any +//! direction are caught immediately. +//! +//! Acceptance criteria from issue #419: +//! 1. Sender balance decremented by exactly the transfer amount. +//! 2. Recipient balance incremented by exactly the transfer amount. +//! 3. Total supply unchanged after transfer. +//! +//! ## Why relative deltas instead of absolute post-conditions +//! +//! `tests/transfer_keys.rs` already covers absolute post-conditions +//! (`sender_balance == before - 1` etc.). Those tests catch the easy case +//! where the decrement is zero or doubled, but a subtle off-by-one or partial +//! commit (e.g. `sender - amount + 1`) would still pass against hard-coded +//! constants if the test was written against an incorrect expectation. The +//! delta-based assertions in this file pin the contract against the actual +//! pre-state stored in the ledger, so any deviation — even by one key — is +//! flagged. Future contributors: please keep both perspectives. + +mod contract_test_env; + +use contract_test_env::{ + register_creator_keys, register_test_creator, set_pricing_and_fees, test_env_with_auths, +}; +use creator_keys::CreatorKeysContractClient; +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, Env}; + +/// Fixed key price used by all tests in this file, matching the existing +/// `transfer_keys.rs` helper setup. +const KEY_PRICE: i128 = 100; + +/// Helper: register a creator with the standard 90/10 fee split and pricing. +fn setup_pricing(client: &CreatorKeysContractClient<'_>, env: &Env) { + set_pricing_and_fees(env, client, KEY_PRICE, 9_000, 1_000); +} + +/// Helper: buy `count` keys for `buyer` under `creator`. +fn buy_keys( + client: &CreatorKeysContractClient<'_>, + creator: &Address, + buyer: &Address, + count: u32, +) { + for _ in 0..count { + client.buy_key(creator, buyer, &KEY_PRICE, &None); + } +} + +/// Asserts the canonical exact-delta invariant for any successful transfer: +/// the sender loses exactly `amount` and the recipient gains exactly `amount`, +/// while `get_total_key_supply` is unchanged. +// +// The eight-argument signature is intentional: capturing the pre-state of both +// parties plus total supply surfaces all four inputs the contract mutates, and +// lets the assertion be a single self-contained call. Slimming any further would +// hide the exact-delta invariant behind an opaque struct. +// +// Read-only helper, so once -7 limits on the lint are still acceptable here. +#[allow(clippy::too_many_arguments)] +fn assert_sender_recipient_exact_delta( + client: &CreatorKeysContractClient<'_>, + creator: &Address, + sender: &Address, + recipient: &Address, + amount: u32, + sender_before: u32, + recipient_before: u32, + supply_before: u32, +) { + let sender_after = client.get_key_balance(creator, sender); + let recipient_after = client.get_key_balance(creator, recipient); + let supply_after = client.get_total_key_supply(creator); + + let sender_delta = sender_before - sender_after; + let recipient_delta = recipient_after - recipient_before; + + assert_eq!( + sender_delta, amount, + "sender balance must be decremented by exactly the transfer amount" + ); + assert_eq!( + recipient_delta, amount, + "recipient balance must be incremented by exactly the transfer amount" + ); + assert_eq!( + sender_delta, recipient_delta, + "sender decrement must equal recipient increment (each transfer is a 1:1 move)" + ); + assert_eq!( + supply_after, supply_before, + "total creator supply must be unchanged after a peer-to-peer transfer" + ); +} + +/// Single-key transfer: sender has multiple keys, leaves two behind, +/// recipient starts with zero. Delta on both sides must be exactly 1. +#[test] +fn test_transfer_keys_sender_balance_decremented_by_exact_amount_single() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + setup_pricing(&client, &env); + + let creator = register_test_creator(&env, &client, "alice"); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + + // Sender starts with 5 keys; recipient starts with 0. + buy_keys(&client, &creator, &sender, 5); + + let sender_before = client.get_key_balance(&creator, &sender); + let recipient_before = client.get_key_balance(&creator, &recipient); + let supply_before = client.get_total_key_supply(&creator); + assert_eq!(sender_before, 5); + assert_eq!(recipient_before, 0); + assert_eq!(supply_before, 5); + + let amount: u32 = 1; + client.transfer_keys(&creator, &sender, &recipient, &amount); + + assert_sender_recipient_exact_delta( + &client, + &creator, + &sender, + &recipient, + amount, + sender_before, + recipient_before, + supply_before, + ); + + // Sanity-check the absolute post-conditions as well. + assert_eq!(client.get_key_balance(&creator, &sender), 4); + assert_eq!(client.get_key_balance(&creator, &recipient), 1); + assert_eq!(client.get_total_key_supply(&creator), 5); +} + +/// Multi-key transfer: send 5 of 10 to a recipient that already has 3. The +/// sender must lose exactly 5 and the recipient must gain exactly 5; total +/// supply is untouched. +#[test] +fn test_transfer_keys_sender_balance_decremented_by_exact_amount_multi() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + setup_pricing(&client, &env); + + let creator = register_test_creator(&env, &client, "multi"); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + + buy_keys(&client, &creator, &sender, 10); + buy_keys(&client, &creator, &recipient, 3); + + let sender_before = client.get_key_balance(&creator, &sender); + let recipient_before = client.get_key_balance(&creator, &recipient); + let supply_before = client.get_total_key_supply(&creator); + assert_eq!(sender_before, 10); + assert_eq!(recipient_before, 3); + assert_eq!(supply_before, 13); + + let amount: u32 = 5; + client.transfer_keys(&creator, &sender, &recipient, &amount); + + assert_sender_recipient_exact_delta( + &client, + &creator, + &sender, + &recipient, + amount, + sender_before, + recipient_before, + supply_before, + ); + + assert_eq!(client.get_key_balance(&creator, &sender), 5); + assert_eq!(client.get_key_balance(&creator, &recipient), 8); + assert_eq!(client.get_total_key_supply(&creator), 13); +} + +/// Full-drain transfer: sender transfers every key they own. The sender +/// balance must be zero and the delta on the recipient side equals the +/// (now-zeroed) sender's prior balance. +#[test] +fn test_transfer_keys_sender_zeroed_out_exact_decrement() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + setup_pricing(&client, &env); + + let creator = register_test_creator(&env, &client, "zero"); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + + buy_keys(&client, &creator, &sender, 4); + + let sender_before = client.get_key_balance(&creator, &sender); + let recipient_before = client.get_key_balance(&creator, &recipient); + let supply_before = client.get_total_key_supply(&creator); + + let amount = sender_before; // transfer every key the sender holds + client.transfer_keys(&creator, &sender, &recipient, &amount); + + assert_eq!( + client.get_key_balance(&creator, &sender), + 0, + "sender must be fully decremented to zero" + ); + assert_eq!( + client.get_key_balance(&creator, &recipient), + recipient_before + amount, + "recipient must receive exactly the transferred amount" + ); + assert_eq!( + client.get_total_key_supply(&creator), + supply_before, + "supply must remain unchanged after a draining transfer" + ); +} + +/// Multiple sequential transfers must accumulate on the sender side. The total +/// decrement across three calls (2 + 3 + 1) must equal 6, regardless of +/// intermediate recipient balances. +#[test] +fn test_transfer_keys_sender_decrement_accumulates_across_calls() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + setup_pricing(&client, &env); + + let creator = register_test_creator(&env, &client, "accum"); + let sender = Address::generate(&env); + let recipient_a = Address::generate(&env); + let recipient_b = Address::generate(&env); + + buy_keys(&client, &creator, &sender, 10); + + let sender_before = client.get_key_balance(&creator, &sender); + let supply_before = client.get_total_key_supply(&creator); + assert_eq!(sender_before, 10); + + client.transfer_keys(&creator, &sender, &recipient_a, &2); + client.transfer_keys(&creator, &sender, &recipient_b, &3); + client.transfer_keys(&creator, &sender, &recipient_a, &1); + + let sender_after = client.get_key_balance(&creator, &sender); + let total_transferred: u32 = 2 + 3 + 1; + + assert_eq!( + sender_before - sender_after, + total_transferred, + "sender decrement must equal the sum of transfer amounts across calls" + ); + assert_eq!( + client.get_total_key_supply(&creator), + supply_before, + "supply must remain unchanged across sequential transfer_keys calls" + ); + assert_eq!( + client.get_key_balance(&creator, &recipient_a), + 3, + "recipient A receives exactly 2 + 1 = 3 keys" + ); + assert_eq!( + client.get_key_balance(&creator, &recipient_b), + 3, + "recipient B receives exactly 3 keys" + ); +} + +/// Sender and recipient balances must be isolated across creators. A transfer +/// under creator A must not influence sender/recipient balances under creator B. +#[test] +fn test_transfer_keys_sender_recipient_deltas_isolated_per_creator() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + setup_pricing(&client, &env); + + let creator_a = register_test_creator(&env, &client, "alpha"); + let creator_b = register_test_creator(&env, &client, "beta"); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + + buy_keys(&client, &creator_a, &sender, 7); + buy_keys(&client, &creator_b, &sender, 6); + + let sender_a_before = client.get_key_balance(&creator_a, &sender); + let sender_b_before = client.get_key_balance(&creator_b, &sender); + let recipient_a_before = client.get_key_balance(&creator_a, &recipient); + let recipient_b_before = client.get_key_balance(&creator_b, &recipient); + let supply_a_before = client.get_total_key_supply(&creator_a); + let supply_b_before = client.get_total_key_supply(&creator_b); + + let amount: u32 = 4; + client.transfer_keys(&creator_a, &sender, &recipient, &amount); + + let sender_a_after = client.get_key_balance(&creator_a, &sender); + let sender_b_after = client.get_key_balance(&creator_b, &sender); + let recipient_a_after = client.get_key_balance(&creator_a, &recipient); + let recipient_b_after = client.get_key_balance(&creator_b, &recipient); + + assert_eq!( + sender_a_before - sender_a_after, + amount, + "creator A sender must lose exactly the transferred amount" + ); + assert_eq!( + sender_b_before, sender_b_after, + "creator B sender balance must be untouched" + ); + assert_eq!( + recipient_a_after - recipient_a_before, + amount, + "creator A recipient must gain exactly the transferred amount" + ); + assert_eq!( + recipient_b_before, recipient_b_after, + "creator B recipient balance must be untouched" + ); + assert_eq!( + client.get_total_key_supply(&creator_a), + supply_a_before, + "creator A total supply must be unchanged" + ); + assert_eq!( + client.get_total_key_supply(&creator_b), + supply_b_before, + "creator B total supply must be unchanged" + ); +} + +/// A transferKeys call that reverts (insufficient balance) must not change any +/// sender or recipient balance, and must leave total supply unchanged. +#[test] +fn test_transfer_keys_reverted_call_does_not_decrement_sender() { + let env = test_env_with_auths(); + let (client, _) = register_creator_keys(&env); + setup_pricing(&client, &env); + + let creator = register_test_creator(&env, &client, "rejected"); + let sender = Address::generate(&env); + let recipient = Address::generate(&env); + + buy_keys(&client, &creator, &sender, 3); + + let sender_before = client.get_key_balance(&creator, &sender); + let recipient_before = client.get_key_balance(&creator, &recipient); + let supply_before = client.get_total_key_supply(&creator); + assert_eq!(sender_before, 3); + assert_eq!(recipient_before, 0); + + // Attempting to transfer 10 keys when sender only holds 3 must revert. + let result = client.try_transfer_keys(&creator, &sender, &recipient, &10u32); + assert!( + result.is_err(), + "transfer with insufficient balance must revert" + ); + + assert_eq!( + client.get_key_balance(&creator, &sender), + sender_before, + "sender balance must be unchanged on a rejected transfer" + ); + assert_eq!( + client.get_key_balance(&creator, &recipient), + recipient_before, + "recipient balance must be unchanged on a rejected transfer" + ); + assert_eq!( + client.get_total_key_supply(&creator), + supply_before, + "total supply must remain unchanged after a reverted transfer" + ); +} diff --git a/creator-keys/tests/peer_to_peer_transfer_supply_regression.rs b/creator-keys/tests/peer_to_peer_transfer_supply_regression.rs index 0b6351d..0aadab8 100644 --- a/creator-keys/tests/peer_to_peer_transfer_supply_regression.rs +++ b/creator-keys/tests/peer_to_peer_transfer_supply_regression.rs @@ -25,7 +25,7 @@ fn test_transfer_keys_keeps_supply_and_same_level_quotes_unchanged() { let buy_quote_before = client.get_buy_quote(&creator); let sell_quote_before = client.get_sell_quote(&creator, &sender); - client.transfer_keys(&sender, &recipient, &creator, &1); + client.transfer_keys(&creator, &sender, &recipient, &1); assert_eq!( client.get_total_key_supply(&creator), diff --git a/docs/authorization-model.md b/docs/authorization-model.md index e0d17e4..7a2b7a1 100644 --- a/docs/authorization-model.md +++ b/docs/authorization-model.md @@ -108,12 +108,12 @@ Sells one key held by `seller` for `creator`. --- -### `transfer_keys(from: Address, to: Address, creator: Address, amount: u32) -> Result<(), ContractError>` +### `transfer_keys(creator: Address, from: Address, to: Address, amount: u32) -> Result<(), ContractError>` Transfers `amount` keys from `from` to `to` for `creator`. Does not interact with the bonding curve and charges no fee. - **Auth**: `from.require_auth()` — only the sender authorizes. No recipient approval required. -- **Rejects**: Self-transfers (`from == to`) with `ContractError::ZeroAddress`. +- **Rejects**: Self-transfers (`from == to`) with `ContractError::SelfTransfer`. - **Fails**: `InsufficientBalance` if `from` holds fewer than `amount` keys. - **Fails**: `NotRegistered` if `creator` is not registered. - **State changes**: `KeyBalance(creator, from)` decremented, `KeyBalance(creator, to)` incremented. Creator supply and holder_count are unchanged. diff --git a/docs/key-transfer-authorization.md b/docs/key-transfer-authorization.md index 13b7c6d..1be8899 100644 --- a/docs/key-transfer-authorization.md +++ b/docs/key-transfer-authorization.md @@ -11,9 +11,9 @@ For the complete contract authorization model covering all entrypoints, see [aut ```rust pub fn transfer_keys( env: Env, + creator: Address, from: Address, to: Address, - creator: Address, amount: u32, ) -> Result<(), ContractError> ``` @@ -53,9 +53,9 @@ In the contract, `from.require_auth()` is called at the top of `transfer_keys`, ```rust pub fn transfer_keys( env: Env, + creator: Address, from: Address, to: Address, - creator: Address, amount: u32, ) -> Result<(), ContractError> { from.require_auth(); // ✅ Sender authenticates @@ -97,11 +97,11 @@ Since the fixed key price is never read and no fee math is performed, the operat ## Self-Transfer Restriction -**`transfer_keys` rejects self-transfers** — calls where `from == to` — with `ContractError::ZeroAddress`. +**`transfer_keys` rejects self-transfers** — calls where `from == to` — with `ContractError::SelfTransfer`. ```rust if from == to { - return Err(ContractError::ZeroAddress); + return Err(ContractError::SelfTransfer); } ``` @@ -109,7 +109,7 @@ if from == to { - A self-transfer would decrement and increment the same holder's balance, resulting in a no-op state change. - Allowing it would waste ledger space and caller gas without providing any useful semantic. -- The `ZeroAddress` error code is reused here because the scenario is semantically similar to the zero-address rejection in `set_protocol_fee_recipient`: an operation that would produce a meaningless state change. +- Self-transfer is rejected with a dedicated `ContractError::SelfTransfer` variant rather than reusing `ContractError::ZeroAddress`. The two errors have distinct semantics: `ZeroAddress` rejects the all-zero Stellar pubkey from being assigned as a fee recipient or treasury target, while `SelfTransfer` rejects a no-op balance move between two equal sender/recipient addresses. ### Other preconditions @@ -117,7 +117,7 @@ Beyond the self-transfer guard, `transfer_keys` validates: - **`from` must have sufficient balance**: The `from` address must hold at least `amount` keys for `creator`. If not, the function returns `ContractError::InsufficientBalance`. - **`creator` must be registered**: The creator profile must exist in storage. If not, the function returns `ContractError::NotRegistered`. -- **`amount` must be positive**: `amount` must be `> 0`. If `amount == 0`, the function returns `ContractError::NotPositiveAmount`. +- **`amount` must be positive**: `amount` must be `> 0`. If `amount == 0`, the function returns `ContractError::ZeroTransferAmount`. --- @@ -166,6 +166,6 @@ This mirrors the `sell` event pattern (`(Symbol("sell"), creator, seller)` / `su | **Who authorizes?** | Only `from` (sender). No `to` (recipient) approval required. | | **Auth mechanism** | `from.require_auth()` — Soroban signature verification. | | **Fee?** | None. No bonding curve interaction. No fee math. | -| **Self-transfer?** | Rejected with `ContractError::ZeroAddress` when `from == to`. | +| **Self-transfer?** | Rejected with `ContractError::SelfTransfer` when `from == to`. | | **State changes** | `KeyBalance` decremented for `from`, incremented for `to`. Supply unchanged. | | **Access level** | Key holder (`from`). |