feat(globe-wallet): recovery_completed event + fix workspace test suite (was fully broken) + fix missing instance-TTL protection

[Ndifreke000]

Root cause

accept_admin (routine, self-initiated) and execute_recovery (emergency, guardian-quorum-driven) publish the exact same admin_transferred event — same topic, same payload shape. There is no on-chain signal distinguishing "the admin rotated their own key" from "guardians just seized control because the key was presumed lost or compromised," even though the second case is arguably the single highest-signal security event this contract can produce, and precisely the moment a wallet owner most needs to be alerted through a side channel.

Design decision

Per the issue's suggested fix: keep admin_transferred byte-for-byte unchanged (existing indexers/the mobile app need zero changes), and have execute_recovery additionally publish a new recovery_completed event carrying old admin, new admin, the full approving-guardian set, the quorum threshold, and both the ledger the timelock elapsed at and the ledger it was actually executed at.

One deliberate departure worth calling out explicitly: RecoveryCompletedEvent is a named #[contracttype] struct, not a tuple — every other event in this contract is a raw tuple. That's a fine trade-off for 2–3 positional fields where order is obvious from context; this event has five fields feeding a security-alerting integration, where a transposed pair of Addresses (an indexer reading new_admin where old_admin belongs) would misreport who just lost control of a wallet during the exact incident that most needs correct reporting. A named struct makes every field self-describing in the XDR itself. The cost is inconsistency with the rest of this file's tuple convention — I did not migrate the other events to match, to keep this diff scoped to #91; full rationale is in the doc comment on the struct, including a note recommending that migration as a follow-up rather than doing it silently here.

approving_guardians deliberately carries the full approval list, not just threshold of them — a wallet recovered 5-of-5 (unanimous) and one recovered at its bare 3-of-5 threshold are different-risk events even though both pass the same on-chain quorum check, and only this field lets a monitoring integration tell them apart (covered by its own test below). ready_at vs executed_at similarly lets an observer see how promptly an already-executable recovery was actually claimed — a multi-day gap between the two is itself a signal worth surfacing.

What else is in this diff, and why

Issue #91 asks for cargo test --workspace output as evidence. Getting an actually-passing run required fixing four things that had nothing to do with #91 on their own, in the order I hit them:

1. cargo test didn't compile at all, for anyone. soroban-env-host 21.2.1 (pulled in transitively via soroban-sdk's testutils feature) declares ed25519-dalek = ">=2.0.0" — no upper bound. Now that ed25519-dalek 3.0.0 exists (a breaking release incompatible with the rand_chacha version soroban-env-host itself pins), Cargo resolves two conflicting major versions in one graph, and soroban-env-host's own testutils.rs fails to compile with a ChaCha20Rng: CryptoRng trait-bound error. This is a real, upstream Cargo.toml bug, unrelated to anything in this repo's own code — but it blocked every test in the workspace, so it had to go first. Since Cargo.lock is gitignored here, the fix had to live in Cargo.toml itself rather than a local lockfile edit (which would silently stop applying for every other contributor/CI the moment the lockfile regenerates). [patch.crates-io] requires a genuinely different source than crates.io, and ed25519-dalek's upstream repo has no 2.2.0 git tag to pin a git-source patch to (checked directly against dalek-cryptography/ed25519-dalek — only pre-release 2.0.0 RCs are tagged), so I vendored the exact, unmodified 2.2.0 package (the version soroban-sdk itself already wants) under vendor/ed25519-dalek-2.2.0/ and patched crates-io onto that path. Full reasoning is in the Cargo.toml comment above the patch. Verified from a completely clean, freshly-regenerated Cargo.lock (deleted and rebuilt with --offline to prove no hidden dependency on my local cache), for both cargo test --workspace and cargo build --release --target wasm32-unknown-unknown — the actual deploy artifact.

2. Two pre-existing test bugs, test_max_assets_limit and test_migrate_user_assets_within_limit_does_nothing: both predate issue AssetInfo.code has no validation — empty strings and case-variant duplicates are both allowed #29's rule that a non-native asset needs a real issuer, and were registering issuer-less fake assets that now fail validation for an unrelated reason before the test's actual assertion is ever reached. Fixed the first by pointing it at the already-existing fill_to_max helper (which already does this correctly — the test just wasn't calling it), and the second the same way AssetInfo.code has no validation — empty strings and case-variant duplicates are both allowed #29 fixed add_asset's own tests.

3. A pre-existing compile error: test_propose_upgrade_accepts_any_hash_without_validation compared a try_propose_upgrade result against Ok(()) instead of the real Ok(Ok(())) shape try_* client methods return.

4. The actual reason two more tests were failing (test_user_assets_ttl_extension_after_long_idle_period, test_spend_limit_ttl_extension_after_long_idle_period) — and the most consequential thing in this PR beyond [Enhancement]: admin_transferred event is identical for routine transfer and emergency guardian recovery — undermines security monitoring #91 itself: not one function in this contract had ever extended its own instance storage TTL. UserAssets/SpendLimit/DailySpent are all carefully protected against silent archival via PERSISTENT_TTL_THRESHOLD/EXTEND_TO; Admin, Guardians, RecoveryConfig, every pending proposal — the contract's own core state, read on nearly every call — had zero such protection. A wallet left alone for longer than the network's default instance-entry lifetime (observably as little as a few thousand ledgers in the test environment — well under a day at Stellar's ~5s average close time) would have its instance archived, and every function, including execute_recovery itself — the one function specifically meant to still work when almost nothing else does — would fail until someone pays for a separate restore operation. This is a strictly worse failure mode than anything the guardian-recovery subsystem defends against: a wallet doesn't need a lost or compromised key to become unusable, it just needs to be left alone. Added bump_instance_ttl() (reusing the existing PERSISTENT_TTL_THRESHOLD/EXTEND_TO constants) and wired it into all 19 state-mutating functions — every one of initialize, propose_admin, accept_admin, cancel_admin_transfer, propose_upgrade, execute_upgrade, add_guardian, remove_guardian, set_recovery_config, initiate_recovery, approve_recovery, revoke_recovery_approval, execute_recovery, cancel_recovery, add_asset, remove_asset, set_spend_limit, record_spend, migrate_user_assets. It's a no-op whenever the instance's current TTL already exceeds the threshold, so this adds no meaningful overhead to a wallet already seeing normal activity — full reasoning is in the helper's doc comment. (To be precise about what this does and doesn't fix: it prevents archival as long as some activity happens periodically; it can't retroactively revive an instance that's already fully archived from zero activity at all — that's an unavoidable Soroban-protocol-level consequence requiring the standard restore operation, same as for any contract.)

I did not fix the pre-existing remove_asset exact-match-vs-add_asset-case-insensitive-match inconsistency I noticed while in this file (already filed separately) — kept this diff to what was actually blocking a runnable test suite plus #91 itself.

Evidence the code actually runs

cargo test --workspace (real run, offline, against a Cargo.lock deleted and freshly regenerated moments earlier — not relying on anything already resolved in my local cache):

running 72 tests
test tests::test_add_asset_overlong_code_fails ... ok
test tests::test_add_asset_empty_code_fails ... ok
test tests::test_accept_admin_emits_only_admin_transferred_not_recovery_completed ... ok
[... 69 more ...]
test tests::test_execute_recovery_emits_both_admin_transferred_and_recovery_completed ... ok
test tests::test_execute_recovery_emits_recovery_completed_with_full_over_quorum_guardian_set ... ok
test tests::test_user_assets_ttl_extension_after_long_idle_period ... ok
test tests::test_spend_limit_ttl_extension_after_long_idle_period ... ok
test tests::test_max_assets_limit ... ok

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

     Running tests/record_spend_reentrancy.rs
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.01s

     Running unittests src/lib.rs (token-wrapper)
running 11 tests
test tests::test_allowance_unset_pair_returns_zero ... ok
[... 9 more ...]
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 1.21s

84/84 passing, 0 failing, across both contracts.

cargo check --workspace --all-targets: clean (only the pre-existing, intentional #[deprecated] warnings from tests that deliberately exercise the legacy transfer_admin wrapper).

cargo build --release --target wasm32-unknown-unknown: clean, produces globe_wallet.wasm (55,603 bytes) and token_wrapper.wasm (10,387 bytes) — the actual deploy artifacts, confirming the vendored-dependency fix doesn't break the testutils-must-not-leak-into-release constraint this workspace's own Cargo.toml comment warns about.

Tests

• test_execute_recovery_emits_both_admin_transferred_and_recovery_completed — full recovery flow (3 guardians, threshold 2), decodes both published events off env.events().all() and asserts every field of RecoveryCompletedEvent, including that approving_guardians contains exactly the two guardians who approved and not the third, silent one.
• test_execute_recovery_emits_recovery_completed_with_full_over_quorum_guardian_set — companion test, 5 guardians all approving against a threshold of 3, proving approving_guardians reflects every approval on the proposal rather than being truncated to threshold, and that executed_at correctly differs from ready_at when execution happens a few ledgers after the recovery became executable.
• test_accept_admin_emits_only_admin_transferred_not_recovery_completed — the negative case the issue's Definition of done explicitly asks for: a routine transfer must never emit recovery_completed, or the event's entire value (unambiguous meaning) is gone.

This is also the first place in this test module that decodes published events at all (find_event helper, using soroban_sdk::testutils::Events) — no prior test in this file asserted on event contents, only that calls succeeded/failed.

Definition of done

• execute_recovery emits both the existing admin_transferred event (byte-for-byte unchanged) and a new, distinctly-named recovery_completed event
• The new event's payload includes old admin, new admin, and the approving guardian set (plus threshold, ready_at, executed_at for additional monitoring context — see rationale above)
• Test proving both events fire with the correct topic/payload — two tests, covering both a bare-quorum and a full-over-quorum recovery
• Test proving accept_admin emits only admin_transferred, never recovery_completed
• Rationale for the event shape written out above (why a struct, why both events, why these specific fields)
• cargo test --workspace output pasted, from a freshly-regenerated lockfile

Adjacent behavior re-verified

• Every pre-existing test in globe-wallet and token-wrapper still passes (84/84) — nothing regressed by either the event change or the instance-TTL bump being added to 19 functions.
• Recovery's existing guarantees (guardian-only approval, quorum + timelock, admin cancellation at any point, stripped approvals on guardian removal) are all still covered by their existing tests, unmodified, and still pass.
• The release/WASM build path — the actual deploy artifact — still builds clean, confirming the dependency fix doesn't leak testutils/std-only code into it.