Context
docs/design/architecture.md's "Future: wiring the contracts together" section, plus record_spend's existing doc comment on contracts/globe-wallet/src/lib.rs: "Reentrancy invariant: keep the interval from reading DailySpent through writing its replacement free of external contract calls."
Problem
architecture.md correctly identifies that globe-wallet and token-wrapper currently don't call each other, and that this is a real security gap (a payment through token-wrapper::transfer_from directly bypasses record_spend's daily-limit enforcement entirely). It proposes two concrete approaches for wiring them: either globe-wallet calls token-wrapper::transfer_from internally after record_spend succeeds, or a new send-style entry point on globe-wallet chains record_spend → transfer_from atomically.
Neither approach, as sketched, addresses what happens once that wiring calls into an arbitrary token_id — which by definition it must, since token-wrapper::transfer_from already takes token_id: Address as a caller-supplied parameter, and nothing constrains it to only the well-known, trusted Stellar Asset Contract implementation. record_spend's own reentrancy invariant is scoped to its own function body — it says nothing about what happens when a different function later in the same call chain (transfer_from) invokes a non-standard, potentially adversarial token contract's transfer, which — unlike the trusted SAC — could itself call back into globe-wallet's public API (e.g. another record_spend, or unrelated guardian/asset functions) before the outer call chain unwinds.
This is forward-looking rather than exploitable today (the wiring doesn't exist yet), but it's exactly the kind of constraint that needs to be part of the wiring's initial design, not retrofitted after a naive sequential implementation ships. record_spend → transfer_from as two separate contract invocations (even within one transaction) is not equivalent to record_spend's existing single-function atomicity guarantee.
Impact
If the wiring described in architecture.md is implemented as a simple sequential call without this in mind, and token_id is ever allowed to be anything other than a small, explicitly-trusted allowlist of token contracts, a malicious token contract's transfer implementation could re-enter globe-wallet mid-flight — for example calling record_spend again for the same or a different asset before the outer spend/transfer completes, potentially double-counting, under-counting, or otherwise desynchronizing the daily-spend bookkeeping from what actually settles on-chain.
Suggested fix
When implementing the wiring: (a) restrict token_id (at the globe-wallet or wiring layer) to a small, admin-curated allowlist of known-safe token contracts rather than accepting arbitrary caller-supplied addresses, and/or (b) apply a checks-effects-interactions ordering across the whole wired call chain — not just within record_spend — so that all of globe-wallet's own state changes (spend recording included) are finalized before the external token-contract call happens, and any reentrant call back into globe-wallet mid-flight sees fully-consistent state rather than a half-committed one. Document which of these (or both) was chosen and why, per CONTRIBUTING.md's design-decision expectations.
Definition of done
Context
docs/design/architecture.md's "Future: wiring the contracts together" section, plusrecord_spend's existing doc comment oncontracts/globe-wallet/src/lib.rs: "Reentrancy invariant: keep the interval from reading DailySpent through writing its replacement free of external contract calls."Problem
architecture.mdcorrectly identifies thatglobe-walletandtoken-wrappercurrently don't call each other, and that this is a real security gap (a payment throughtoken-wrapper::transfer_fromdirectly bypassesrecord_spend's daily-limit enforcement entirely). It proposes two concrete approaches for wiring them: either globe-wallet callstoken-wrapper::transfer_frominternally afterrecord_spendsucceeds, or a newsend-style entry point on globe-wallet chainsrecord_spend→transfer_fromatomically.Neither approach, as sketched, addresses what happens once that wiring calls into an arbitrary
token_id— which by definition it must, sincetoken-wrapper::transfer_fromalready takestoken_id: Addressas a caller-supplied parameter, and nothing constrains it to only the well-known, trusted Stellar Asset Contract implementation.record_spend's own reentrancy invariant is scoped to its own function body — it says nothing about what happens when a different function later in the same call chain (transfer_from) invokes a non-standard, potentially adversarial token contract'stransfer, which — unlike the trusted SAC — could itself call back intoglobe-wallet's public API (e.g. anotherrecord_spend, or unrelated guardian/asset functions) before the outer call chain unwinds.This is forward-looking rather than exploitable today (the wiring doesn't exist yet), but it's exactly the kind of constraint that needs to be part of the wiring's initial design, not retrofitted after a naive sequential implementation ships.
record_spend→transfer_fromas two separate contract invocations (even within one transaction) is not equivalent torecord_spend's existing single-function atomicity guarantee.Impact
If the wiring described in
architecture.mdis implemented as a simple sequential call without this in mind, andtoken_idis ever allowed to be anything other than a small, explicitly-trusted allowlist of token contracts, a malicious token contract'stransferimplementation could re-enterglobe-walletmid-flight — for example callingrecord_spendagain for the same or a different asset before the outer spend/transfer completes, potentially double-counting, under-counting, or otherwise desynchronizing the daily-spend bookkeeping from what actually settles on-chain.Suggested fix
When implementing the wiring: (a) restrict
token_id(at the globe-wallet or wiring layer) to a small, admin-curated allowlist of known-safe token contracts rather than accepting arbitrary caller-supplied addresses, and/or (b) apply a checks-effects-interactions ordering across the whole wired call chain — not just withinrecord_spend— so that all of globe-wallet's own state changes (spend recording included) are finalized before the external token-contract call happens, and any reentrant call back into globe-wallet mid-flight sees fully-consistent state rather than a half-committed one. Document which of these (or both) was chosen and why, per CONTRIBUTING.md's design-decision expectations.Definition of done
token_idcould do if allowed to reach the wiring, and why the chosen mitigation closes ittransfer, proving the wiring is not exploitablerecord_spend's existing single-function reentrancy invariant/testscargo test --workspaceoutput pasted