Skip to content

PaymentRecord's sequence counter is global, not per-sender, making the (from, seq) storage key non-contiguous despite its shape implying otherwise #52

Description

@abayomicornelius

Overview

PaymentRecord entries are stored under a composite key of (from, seq), and the module doc comment frames seq as if it's meaningfully associated with the sender:

// stellar_send/src/lib.rs:14-22
//! Storage layout
//! ──────────────
//! Instance storage (short-lived, cheap):
//!   KEY_CONFIG  → ContractConfig
//!   KEY_SEQ     → u64  (global payment sequence counter)
//!
//! Persistent storage (survives ledger closings):
//!   (from, seq) → PaymentRecord

But KEY_SEQ is explicitly documented as a global counter, and the code confirms it: next_seq (stellar_send/src/lib.rs:379-389) increments a single shared u64 regardless of who's calling, and it's shared not just across every sender but across every payment-producing function in the contractsend_payment (:218) and every individual leg inside send_batch_payment's loop (stellar_send/src/batch.rs:64) all draw from the exact same counter. payment_request/subscription correctly use their own separate, purpose-specific counters (KEY_REQ_SEQ, KEY_SUB_SEQ) — only PaymentRecord's seq is shared this broadly.

The practical consequence: for a given sender X, their own PaymentRecords occupy scattered, non-contiguous seq values interleaved with every other sender's activity. If Alice sends a payment (seq = 1), Bob sends one (seq = 2), then Alice sends another (seq = 3), Alice's two records live at keys (alice, 1) and (alice, 3) — not (alice, 1) and (alice, 2) as the (from, seq) key shape would naturally suggest to anyone reading the storage layout for the first time. There is no get_next_seq_for(from), no per-sender payment count exposed anywhere, and get_payment_record(from, seq) (stellar_send/src/lib.rs:353-365) requires the caller to already know the exact global seq for a specific historical payment.

This means the (from, seq) addressing scheme cannot actually be used for its evidently intended purpose — enumerating "all of Alice's payments" — without already knowing, from an external source (off-chain event indexing, since emit_payment_sent at stellar_send/src/events.rs:11-23 does carry enough data), which of the enormous range of global seq values belong to Alice. A naive integrator reading only the module doc comment and the (from, seq) key shape (without independently discovering this global-counter detail) would reasonably expect to be able to iterate seq = 1, 2, 3, ... for a given from and enumerate that sender's full history — and would get almost entirely PaymentRecordNotFound misses instead.

Requirements

  • Either make seq genuinely per-sender (a separate counter keyed by from, incremented independently for each sender, exactly like payment_request/subscription already do for their own id spaces) so the (from, seq) key shape actually delivers contiguous, enumerable per-sender history as it visually implies, or explicitly re-document the storage layout to make clear seq is global and (from, seq) is only ever meant to be used as an opaque, individually-known lookup key (never iterated), matching reality.
  • If moving to a per-sender counter, decide how to handle send_batch_payment's multiple legs within one call — each leg still needs its own unique seq per sender, which a per-sender counter handles naturally.
  • Whichever direction is chosen, make sure get_payment_record's doc comment and the module-level storage layout comment agree with the actual behavior.

Acceptance Criteria

  • The relationship between seq and from is either fixed (per-sender counter) or the documentation is corrected to explicitly warn that seq is global and (from, seq) should be treated as an opaque key obtained from events, not an enumerable range.
  • If fixed: a test creates several payments from the same sender interleaved with a different sender's payments, and confirms the first sender's records are retrievable via a contiguous seq range starting from a per-sender count exposed via a new query (e.g. get_sequence, already listed as unshipped backlog in stellar_send/src/stubs.rs:12).
  • If documented-not-fixed: the module doc comment (stellar_send/src/lib.rs:14-22) and get_payment_record's doc comment (:343-352) both explicitly state the global-counter, non-enumerable nature of seq.

Additional Notes

Precise references: stellar_send/src/lib.rs:14-22 (module doc's storage-layout comment, which doesn't flag seq as global despite the field itself being documented that way two lines up), :44-45 (KEY_SEQ: Symbol, stellar_send/src/lib.rs:379-389's next_seq, explicitly commented "global payment sequence counter"), stellar_send/src/batch.rs:64 (confirming batch legs share the exact same global counter as send_payment), stellar_send/src/payment_request.rs:19 and stellar_send/src/subscription.rs:68-69 (the separate, purpose-specific counters KEY_REQ_SEQ/KEY_SUB_SEQ these other two features correctly use instead — making PaymentRecord's shared global counter look like an inconsistency relative to the rest of this same file's own conventions, not an isolated design choice).

Relationship to existing issue #21 ("escrow has no way to enumerate escrows by depositor or beneficiary"): related in spirit (both are about the difficulty of enumerating a party's own records without off-chain indexing) but a meaningfully different underlying cause — escrow's gap is the complete absence of any secondary index. Here, the (from, seq) key shape already exists and strongly implies per-sender enumerability that the actual global-counter semantics silently don't deliver — a subtler, more surprising defect since the API looks like it should already support the very thing it doesn't.

Test/reproduction plan: in stellar_send/src/test.rs, extend test_get_payment_record (stellar_send/src/test.rs:312-337): create a payment from sender_a, then one from sender_b, then another from sender_a. Show that sender_a's two records live at seq = 1 and seq = 3 (not 1 and 2), and that get_payment_record(sender_a, 2) returns PaymentRecordNotFound even though sender_a genuinely has a second payment — demonstrating concretely that naive sequential iteration per sender doesn't work today. After whichever fix direction is chosen, either assert sender_a's records are now contiguous at 1, 2 under a per-sender counter, or assert the corrected documentation accurately describes the global-counter behavior demonstrated by this same test.

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaignbugSomething isn't workingcontractsSmart contract logicdocumentationImprovements or additions to documentationvery hardVery difficult / senior-level bounty issue

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions