Skip to content

stellar_send never invokes fee_collector::collect_fee — fee accounting totals are always zero in production #10

Description

@abayomicornelius

Problem

stellar_send::send_payment (stellar_send/src/lib.rs lines 186-237) never checks from != to. Every other fee-charging entrypoint in the same crate does enforce this: send_batch_payment rejects recipient == from (batch.rs line 46-48, StellarSendError::SelfPaymentNotAllowed), create_payment_request rejects payer == requester (payment_request.rs lines 69-72), and create_subscription rejects payer == recipient (subscription.rs lines 72-74). send_payment — the single most-used entrypoint — is the one place this guard is missing.

Why it matters

This is an inconsistency, not just a missing nicety: three of four payment paths treat self-payment as an explicit error (StellarSendError::SelfPaymentNotAllowed, variant 15 in error.rs), but the primary path silently allows it. A self-payment still charges the protocol fee (transferred to fee_collector) while the "net" amount just goes right back to the sender — functionally a fee-only burn with no economic transfer, which is probably not intended given the other three paths explicitly forbid the pattern. It also means integrators who defensively rely on SelfPaymentNotAllowed being enforced everywhere (reasonable given the other three call sites) will be surprised in production.

Detection

Call send_payment(from: X, to: X, ...) in stellar_send/src/test.rs — it currently succeeds and returns a PaymentRecord, whereas the equivalent self-referential call on any of the other three entrypoints already returns Err(SelfPaymentNotAllowed).

Proposed fix

Add the same guard used elsewhere, right after loading config:

if from == to {
    return Err(StellarSendError::SelfPaymentNotAllowed);
}

Edge cases

  • Decide whether this should be a hard error or an intentionally-allowed no-op-ish "fee-only" pattern (some protocols deliberately allow "pay yourself" as a way to test connectivity/pay a fee for an unrelated reason) — given the other three entrypoints already made the "disallow" call, consistency strongly favors disallowing here too, but worth a maintainer sign-off since it's technically a behavior change for existing integrators who may depend on today's permissive behavior.
  • send_path_payment (a fifth payment path) has the same gap — it never checks from == to either, and arguably matters even more there once issue send_path_payment simulates DEX swaps 1:1 instead of calling a real router — no actual price discovery #2's real router lands, since a self-directed path swap could be used to launder slippage/fee asymmetries. Should be fixed in the same PR.

Testing strategy

Add test_send_payment_self_payment_rejected asserting Err(StellarSendError::SelfPaymentNotAllowed), mirroring the existing test_create_subscription_self_payment / equivalent pattern already used for the other three entrypoints (naming inferred from the SelfPaymentNotAllowed usages; verify exact existing test names in stellar_send/src/test.rs and follow the same convention).

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26bugSomething isn't workingcontractsSmart contract logicsecuritySecurity concernvery hardVery difficult / senior-level bounty issue

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions