Skip to content

fix(globe-wallet): disambiguate spend limit storage keys by AssetInfo and add migration path (#82) - #94

Closed
s6pa1rta3n-lab wants to merge 1 commit into
Orbit-Wal:mainfrom
s6pa1rta3n-lab:fix-issue
Closed

fix(globe-wallet): disambiguate spend limit storage keys by AssetInfo and add migration path (#82)#94
s6pa1rta3n-lab wants to merge 1 commit into
Orbit-Wal:mainfrom
s6pa1rta3n-lab:fix-issue

Conversation

@s6pa1rta3n-lab

@s6pa1rta3n-lab s6pa1rta3n-lab commented Aug 29, 2026

Copy link
Copy Markdown

Summary

Resolves #82.

In contracts/globe-wallet/src/lib.rs, DataKey::SpendLimit and DataKey::DailySpent previously used (Address, String) keyed solely by asset code name (e.g. "USDC"), causing spend limit collisions between different assets sharing identical ticker codes issued by different accounts.

This PR implements the disambiguation fix in contracts/globe-wallet/src/lib.rs, allows same-code different-issuer assets in add_asset, enforces user asset whitelist presence on spend limit configuration and spends, and provides backward-compatible migration endpoints for existing persisted keys.

Location Note: Spend limit and asset whitelisting logic resides exclusively in contracts/globe-wallet/src/lib.rs where user wallets and daily spend budgets are tracked; contracts/token-wrapper/src/lib.rs was intentionally left untouched.


Key Changes

  • Disambiguated Storage Keys: Updated DataKey::SpendLimit(Address, AssetInfo) and DataKey::DailySpent(Address, AssetInfo) to key by full AssetInfo rather than raw ticker string. Added LegacySpendLimit and LegacyDailySpent variants for migration.
  • Spend Limit API: Updated set_spend_limit, get_spend_limit, and record_spend to accept AssetInfo. Validates that the asset is present in the caller's UserAssets whitelist via require_asset_registered, rejecting unregistered assets with WalletError::AssetNotFound.
  • Duplicate Asset Whitelist Check: Updated add_asset duplicate check to match codes_match_case_insensitive(&existing.code, &asset.code) && existing.issuer == asset.issuer, allowing same-code assets from different issuers while rejecting true duplicates.
  • Clean Asset Removal: Updated remove_asset to purge both new AssetInfo-keyed and legacy storage entries.
  • Migration Endpoints:
    • migrate_user_spend_limits: Admin-authorized migration of a user's legacy spend limit and daily spend record from legacy key to disambiguated key. Includes allow_overwrite flag (returns WalletError::MigrationError if target is already configured and overwrite is false).
    • batch_migrate_spend_limits: Admin-authorized batch migration for multiple user wallets.
  • Comprehensive Unit & Integration Tests:
    • test_same_code_different_issuer_allowed: Verifies assets sharing ticker codes but with distinct issuers can be registered simultaneously.
    • test_disambiguated_spend_limits: Proves two same-code, different-issuer assets receive independent daily spend limits and tracking.
    • test_unregistered_asset_rejected_for_spend_limit_and_spend: Verifies unregistered assets are rejected for both spend limit setting and spending.
    • test_migration_from_legacy_key: Verifies migration from legacy (Symbol, user, code) and LegacySpendLimit storage format to new AssetInfo keys.
    • test_batch_migration: Verifies batch migration across multiple accounts.
    • test_migration_no_overwrite: Verifies allow_overwrite=false preserves existing configuration without corruption.
    • Updated all existing unit tests and reentrancy test record_spend_reentrancy.rs.

Test Verification

running 75 tests
test tests::test_add_asset_overlong_code_fails ... ok
test tests::test_add_asset_empty_code_fails ... ok
test tests::test_accept_by_wrong_address_fails ... ok
test tests::test_add_asset_case_variant_duplicate_fails ... ok
test tests::test_add_duplicate_guardian_fails ... ok
test tests::test_add_and_get_assets ... ok
test tests::test_add_duplicate_asset_fails ... ok
test tests::test_add_and_list_guardians ... ok
test tests::test_cancel_admin_transfer ... ok
test tests::test_cannot_initiate_second_recovery_while_one_pending ... ok
test tests::test_initialize ... ok
test tests::test_daily_spent_survives_temporary_ttl_eviction ... ok
test tests::test_double_approval_rejected ... ok
test tests::test_admin_can_cancel_recovery_even_after_quorum ... ok
test tests::test_batch_migration ... ok
test tests::test_execute_recovery_rejects_new_admin_same_as_current_admin ... ok
test tests::test_disambiguated_spend_limits ... ok
test tests::test_migrate_user_assets_within_limit_does_nothing ... ok
test tests::test_migration_from_legacy_key ... 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_no_limit_allows_any_spend ... ok
test tests::test_migration_no_overwrite ... ok
test tests::test_pending_admin_cleared_after_accept_admin ... ok
test tests::test_propose_upgrade_accepts_any_hash_without_validation ... ok
test tests::test_max_guardians_limit ... ok
test tests::test_propose_upgrade_requires_admin ... ok
test tests::test_non_native_code_without_issuer_is_underspecified_and_rejected ... ok
test tests::test_propose_without_accept_keeps_admin_unchanged ... ok
test tests::test_non_admin_cannot_add_guardian ... ok
test tests::test_record_spend_boundary_first_second_of_new_day_resets ... ok
test tests::test_record_spend_boundary_drift_awareness ... ok
test tests::test_record_spend_exact_day_boundary ... ok
test tests::test_raise_spend_then_lower_limit ... ok
test tests::test_max_assets_limit ... ok
test tests::test_migrate_user_assets_trims_excess ... ok
test tests::test_record_spend_boundary_last_second_of_day_accumulates ... ok
test tests::test_record_spend_negative_amount_fails ... ok
test tests::test_record_spend_exceeds_limit_fails ... ok
test tests::test_record_spend_overflow_does_not_poison_later_calls ... ok
test tests::test_record_spend_within_limit ... ok
test tests::test_record_spend_bucket_is_integer_division ... ok
test tests::test_record_spend_zero_amount_fails ... ok
test tests::test_remove_asset ... ok
test tests::test_execute_upgrade_with_never_uploaded_hash_traps - should panic ... ok
test tests::test_record_spend_negative_amount_cannot_bypass_daily_limit ... ok
test tests::test_recovery_rejects_non_guardian ... ok
test tests::test_record_spend_rejected_negative_amount_does_not_mutate_state ... ok
test tests::test_recovery_happy_path_2_of_3 ... ok
test tests::test_remove_nonexistent_asset_fails ... ok
test tests::test_recovery_clears_any_in_flight_normal_admin_transfer ... ok
test tests::test_propose_and_execute_upgrade - should panic ... ok
test tests::test_require_admin_not_initialized ... ok
test tests::test_remove_guardian_below_threshold_fails ... ok
test tests::test_same_code_different_issuer_allowed ... ok
test tests::test_removed_guardian_approval_no_longer_counts_toward_quorum ... ok
test tests::test_removed_guardian_cannot_initiate_recovery ... ok
test tests::test_remove_guardian_dequorated_proposal_can_requorum_with_fresh_timelock ... ok
test tests::test_revoke_recovery_approval_rejects_non_guardian ... ok
test tests::test_set_recovery_config_rejects_single_guardian_threshold ... ok
test tests::test_revoking_approval_below_threshold_resets_timelock ... ok
test tests::test_remove_guardian_who_never_approved_leaves_proposal_untouched ... ok
test tests::test_set_recovery_config_requires_min_guardians ... ok
test tests::test_revoke_recovery_approval_rejects_removed_guardian ... ok
test tests::test_unregistered_asset_rejected_for_spend_limit_and_spend ... ok
test tests::test_set_recovery_config_rejects_threshold_above_guardian_count ... ok
test tests::test_upgrade_propose_double_fails ... ok
test tests::test_transfer_admin ... ok
test tests::test_spend_limit_ttl_extension_after_long_idle_period ... ok
test tests::test_upgrade_rejects_hash_mismatch ... ok
test tests::test_set_recovery_config_rejected_while_recovery_pending ... ok
test tests::test_spend_limit_set_and_get ... ok
test tests::test_upgrade_requires_admin_and_ready_time ... ok
test tests::test_user_assets_ttl_extension_after_long_idle_period ... ok

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

     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.25s

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

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

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

Payout Routing

  • EVM (Base/Arbitrum/Polygon/ETH): 0xF46C9F6d70C50BF81ef3588AB523a90a594a2F89
  • Stellar: GCL6OXAMLD75BMTINA6EMRUDWK5THQUSHMYNLSNBCJAPZJHNYJTUNIBC

@s6pa1rta3n-lab s6pa1rta3n-lab changed the title fix(globe-wallet): enforce minimum delay for upgrade proposals and guardian recovery timelocks (#84) fix(globe-wallet): disambiguate spend limit storage keys by AssetInfo and add migration path (#82) Aug 29, 2026
@s6pa1rta3n-lab
s6pa1rta3n-lab force-pushed the fix-issue branch 7 times, most recently from c2618d7 to ce27fba Compare August 29, 2026 17:16
…with token allowlist and CEI ordering

- Add admin-curated token allowlist via `set_token_allowed` and `is_token_allowed` (with `TokenNotAllowed = 1034`) to reject untrusted token contract addresses.
- Add `GlobeWallet::send` entrypoint wiring `globe-wallet` to `token-wrapper::transfer_from` with strict Checks-Effects-Interactions (CEI) ordering.
- Commit spend bookkeeping to persistent storage before calling external contract code.
- Add unit tests verifying token allowlist admin enforcement, wired transfer settlement, daily limit enforcement, and rollback on malicious re-entrant token callbacks.
- Update architecture design and reentrancy documentation with threat model analysis and platform invariants.

Closes Orbit-Wal#92
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Issue #8's fix landed in the wrong contract — the SpendLimit/DailySpent code/issuer collision it describes is still live on main

1 participant