fix(token-wrapper): key Allowance storage by (owner, spender, token_id) to support multi-asset allowances (#85)

[s6pa1rta3n-lab]

Summary & Root Cause

Fixes #85.

Root Cause

In contracts/token-wrapper/src/lib.rs, DataKey::Allowance was previously keyed only by (Address, Address) representing (owner, spender). The token_id parameter was not included in storage key definition, nor was it passed to approve or allowance. Consequently, when an owner called approve for the same spender across a second token asset, the new allowance record completely replaced and destroyed the existing allowance for the first token.

Fix

• Updated DataKey::Allowance storage enum to Allowance(Address, Address, Address) representing (owner, spender, token_id).
• Updated TokenWrapper::approve to accept token_id: Address as the third parameter (owner, spender, token_id, amount, expiry_ledger) and emit (owner, spender, token_id, amount, expiry_ledger) in the approved event.
• Updated TokenWrapper::allowance to accept token_id: Address (owner, spender, token_id).
• Updated TokenWrapper::transfer_from to construct storage key using (from, spender, token_id).
• Updated unit tests across token-wrapper and added unit tests proving independent multi-asset allowances for the same (owner, spender) pair.
• Updated architecture and README documentation to match the updated API signatures.

---

Migration & Compatibility Decision

Decision: Pre-existing (owner, spender)-keyed allowances are intentionally invalidated by the schema change and are treated as non-existent (returning default zero allowance upon query). Integrators must re-issue approve calls for each specific asset.

Rationale:

1. Under the legacy storage layout, the contract stored DataKey::Allowance(owner, spender) with value Allowance { amount, expiry_ledger }. Neither the key nor value record tracked which token_id the allowance was originally intended for.
2. Because token_id was never persisted in state, there is no unambiguous on-chain mechanism to map legacy allowance records to a particular token address without relying on off-chain indexing or arbitrary assumptions.
3. Automatically preserving legacy allowances across arbitrary tokens would introduce severe security vulnerabilities (e.g. enabling a spender to drain high-value assets using an allowance granted for a different test asset). Invalidation and requiring explicit per-asset approve invocations guarantees security and correctness.

---

Definition of Done Checklist

• DataKey::Allowance includes token_id; approve, allowance, transfer_from all take/use it. (Addressed in contracts/token-wrapper/src/lib.rs)
• Test proving two different tokens' allowances for the same (owner, spender) pair are now independent (approving one doesn't touch the other). (Added test_approving_second_token_does_not_destroy_first_tokens_allowance and test_allowances_for_different_tokens_are_independent)
• Existing single-token tests (test_approve_overwrites_previous_allowance, etc.) updated to reflect same-token overwrite is still the correct behavior — only cross-token overwrite was the bug. (Updated test suite and docstrings in contracts/token-wrapper/src/lib.rs)
• Migration/compat decision for pre-existing (owner, spender)-keyed allowances written out explicitly in the PR description. (Detailed above in Migration & Compatibility Decision section)
• cargo test --workspace output pasted. (Pasted below)

---

Test Suite Output (cargo test --workspace)

warning: use of deprecated associated function `GlobeWallet::spec_xdr_transfer_admin`: Use propose_admin and accept_admin instead
   --> contracts/globe-wallet/src/lib.rs:201:1
    |
201 | #[contractimpl]
    | ^^^^^^^^^^^^^^^
    |
    = note: `#[warn(deprecated)]` on by default
    = note: this warning originates in the attribute macro `soroban_sdk::contractspecfn` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: use of deprecated method `GlobeWalletClient::<'a>::transfer_admin`: Use propose_admin and accept_admin instead
    --> contracts/globe-wallet/src/lib.rs:1588:16
     |
1588 |         client.transfer_admin(&admin, &new_admin);
     |                ^^^^^^^^^^^^^^

warning: use of deprecated method `GlobeWalletClient::<'a>::try_transfer_admin`: Use propose_admin and accept_admin instead
    --> contracts/globe-wallet/src/lib.rs:1630:20
     |
1630 |             client.try_transfer_admin(&caller, &caller),
     |                    ^^^^^^^^^^^^^^^^^^

warning: unused variable: `env`
    --> contracts/globe-wallet/src/lib.rs:1949:14
     |
1949 |         let (env, _admin, guardians, client) = setup_with_guardians(3);
     |              ^^^ help: if this is intentional, prefix it with an underscore: `_env`
     |
     = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: function `make_code` is never used
    --> contracts/globe-wallet/src/lib.rs:1265:8
     |
1265 |     fn make_code(env: &Env, n: u32) -> String {
     |        ^^^^^^^^^
     |
     = note: `#[warn(dead_code)]` (part of `#[warn(unused)]`) on by default

warning: function `fill_to_max` is never used
    --> contracts/globe-wallet/src/lib.rs:1269:8
     |
1269 |     fn fill_to_max(env: &Env, client: &GlobeWalletClient, user: &Address) {
     |        ^^^^^^^^^^^

warning: `globe-wallet` (lib) generated 1 warning
warning: `globe-wallet` (lib test) generated 6 warnings (1 duplicate) (run `cargo fix --lib -p globe-wallet --tests` to apply 1 suggestion)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.62s
     Running unittests src/lib.rs (target/debug/deps/globe_wallet-c063bb43cfb6b03b)

running 69 tests
test tests::test_add_asset_overlong_code_fails ... ok
test tests::test_add_asset_empty_code_fails ... ok
test tests::test_add_duplicate_guardian_fails ... ok
test tests::test_add_asset_case_variant_duplicate_fails ... ok
test tests::test_add_duplicate_asset_fails ... ok
test tests::test_accept_by_wrong_address_fails ... ok
test tests::test_add_and_list_guardians ... ok
test tests::test_add_and_get_assets ... ok
test tests::test_daily_spent_survives_temporary_ttl_eviction ... ok
test tests::test_initialize ... ok
test tests::test_cancel_admin_transfer ... ok
test tests::test_double_approval_rejected ... ok
test tests::test_cannot_initiate_second_recovery_while_one_pending ... ok
test tests::test_execute_recovery_rejects_new_admin_same_as_current_admin ... ok
test tests::test_admin_can_cancel_recovery_even_after_quorum ... ok
test tests::test_migrate_user_assets_within_limit_does_nothing ... ok
test tests::test_no_limit_allows_any_spend ... ok
test tests::test_initialize_twice_fails ... ok
test tests::test_migrate_user_assets_requires_admin ... ok
test tests::test_native_code_with_issuer_is_contradictory_and_rejected ... ok
test tests::test_non_admin_cannot_add_guardian ... ok
test tests::test_propose_upgrade_accepts_any_hash_without_validation ... ok
test tests::test_propose_upgrade_requires_admin ... ok
test tests::test_pending_admin_cleared_after_accept_admin ... ok
test tests::test_propose_without_accept_keeps_admin_unchanged ... ok
test tests::test_record_spend_boundary_drift_awareness ... ok
test tests::test_record_spend_boundary_first_second_of_new_day_resets ... ok
test tests::test_max_guardians_limit ... ok
test tests::test_non_native_code_without_issuer_is_underspecified_and_rejected ... ok
test tests::test_record_spend_boundary_last_second_of_day_accumulates ... ok
test tests::test_record_spend_bucket_is_integer_division ... ok
test tests::test_record_spend_exact_day_boundary ... ok
test tests::test_record_spend_exceeds_limit_fails ... ok
test tests::test_raise_spend_then_lower_limit ... ok
test tests::test_record_spend_negative_amount_fails ... ok
test tests::test_migrate_user_assets_trims_excess ... ok
test tests::test_record_spend_negative_amount_cannot_bypass_daily_limit ... ok
test tests::test_record_spend_rejected_negative_amount_does_not_mutate_state ... ok
test tests::test_record_spend_within_limit ... ok
test tests::test_record_spend_overflow_does_not_poison_later_calls ... ok
test tests::test_remove_asset ... ok
test tests::test_record_spend_zero_amount_fails ... ok
test tests::test_remove_guardian_below_threshold_fails ... ok
test tests::test_recovery_rejects_non_guardian ... ok
test tests::test_remove_nonexistent_asset_fails ... ok
test tests::test_recovery_clears_any_in_flight_normal_admin_transfer ... ok
test tests::test_remove_guardian_dequorated_proposal_can_requorum_with_fresh_timelock ... ok
test tests::test_execute_upgrade_with_never_uploaded_hash_traps - should panic ... ok
test tests::test_recovery_happy_path_2_of_3 ... ok
test tests::test_require_admin_not_initialized ... ok
test tests::test_revoke_recovery_approval_rejects_non_guardian ... ok
test tests::test_propose_and_execute_upgrade - should panic ... ok
test tests::test_remove_guardian_who_never_approved_leaves_proposal_untouched ... ok
test tests::test_removed_guardian_cannot_initiate_recovery ... ok
test tests::test_removed_guardian_approval_no_longer_counts_toward_quorum ... ok
test tests::test_set_recovery_config_requires_min_guardians ... ok
test tests::test_spend_limit_set_and_get ... ok
test tests::test_revoking_approval_below_threshold_resets_timelock ... ok
test tests::test_set_recovery_config_rejects_single_guardian_threshold ... ok
test tests::test_revoke_recovery_approval_rejects_removed_guardian ... ok
test tests::test_upgrade_propose_double_fails ... ok
test tests::test_upgrade_rejects_hash_mismatch ... ok
test tests::test_spend_limit_ttl_extension_after_long_idle_period ... ok
test tests::test_transfer_admin ... ok
test tests::test_upgrade_requires_admin_and_ready_time ... ok
test tests::test_user_assets_ttl_extension_after_long_idle_period ... ok
test tests::test_set_recovery_config_rejects_threshold_above_guardian_count ... ok
test tests::test_set_recovery_config_rejected_while_recovery_pending ... ok
test tests::test_max_assets_limit ... ok

test result: ok. 69 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.86s

     Running tests/record_spend_reentrancy.rs (target/debug/deps/record_spend_reentrancy-9f6bb289bf4090dc)

running 1 test
test two_spends_in_one_host_invocation_accumulate ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.13s

     Running unittests src/lib.rs (target/debug/deps/token_wrapper-11699b1bfd8edd1b)

running 13 tests
test tests::test_approve_past_expiry_fails ... ok
test tests::test_approve_negative_amount_fails ... ok
test tests::test_allowance_unset_pair_returns_zero ... ok
test tests::test_approve_and_allowance ... ok
test tests::test_transfer_from_expired_allowance_fails ... ok
test tests::test_transfer_from_insufficient_allowance_fails ... ok
test tests::test_approving_second_token_does_not_destroy_first_tokens_allowance ... ok
test tests::test_transfer_from_happy_path ... ok
test tests::test_approve_overwrites_previous_allowance ... ok
test tests::test_allowances_for_different_tokens_are_independent ... ok
test tests::test_transfer_from_zero_amount_fails ... ok
test tests::test_transfer_from_succeeds_exactly_at_expiry_ledger ... ok
test tests::test_transfer_from_rolls_back_allowance_when_underlying_transfer_fails ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.02s

   Doc-tests globe_wallet

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests token_wrapper

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

---

Payout Routing

• Base / Arbitrum / Polygon / ETH: 0xF46C9F6d70C50BF81ef3588AB523a90a594a2F89
• Stellar: GCL6OXAMLD75BMTINA6EMRUDWK5THQUSHMYNLSNBCJAPZJHNYJTUNIBC