Skip to content

feat(connector): [payload] add support to split payments #13121

Draft
AkshayaFoiger wants to merge 1 commit into
mainfrom
payload_split_payment
Draft

feat(connector): [payload] add support to split payments #13121
AkshayaFoiger wants to merge 1 commit into
mainfrom
payload_split_payment

Conversation

@AkshayaFoiger

@AkshayaFoiger AkshayaFoiger commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Blocked on payload

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@AkshayaFoiger AkshayaFoiger requested review from a team as code owners July 1, 2026 06:33
@semanticdiff-com

semanticdiff-com Bot commented Jul 1, 2026

Copy link
Copy Markdown

@AkshayaFoiger AkshayaFoiger changed the title fix(router): fix notify connectors feat(connector): [payload] add support to split payments Jul 1, 2026
@AkshayaFoiger AkshayaFoiger marked this pull request as draft July 1, 2026 06:39
@XyneSpaces

Copy link
Copy Markdown

test

1 similar comment
@XyneSpaces

Copy link
Copy Markdown

test

@XyneSpaces

Copy link
Copy Markdown

⚠️ Missing Secret<T> wrapper for sensitive receiver_id

In PayloadLedgerItem, the receiver_id field holds a processing identifier that could be sensitive. Consider wrapping it in Secret<String>:

pub struct PayloadLedgerItem {
    pub amount: MinorUnit,
    pub receiver_id: String,  // <-- Should this be Secret<String>?
}

Fix: If receiver_id is a credential/token that shouldn't appear in logs, change to pub receiver_id: Secret<String>.

@XyneSpaces

Copy link
Copy Markdown

🚨 Potential correctness issue in response type mapping

The ConnectorChargeResponseData::PayloadSplitPayment variant stores PayloadSplitPaymentRequest (the request type) in the response data. This is semantically incorrect - responses should use a dedicated response type, not the request type.

// Line ~460 in common_types/src/payments.rs
pub enum ConnectorChargeResponseData {
    // ... other variants
    #[smithy(value_type = "PayloadSplitPaymentRequest")]  // <-- Request type in response enum
    PayloadSplitPayment(PayloadSplitPaymentRequest),
}

Fix: Define a PayloadChargeResponseData struct that captures the actual response fields from Payload's API (transaction IDs, processing status, ledger confirmation, etc.), or rename if this is intentional pass-through behavior and add a clarifying comment.

@XyneSpaces

Copy link
Copy Markdown

📝 Review Summary

Found 1 issue requiring attention:

[should-fix] Split payments validation allows positive amounts

In crates/router/src/core/payments/helpers.rs around line 9120+, the validation for PayloadSplitPayment checks:

if split_item.amount.get_amount_as_i64() > 0 {
    return Err(errors::ApiErrorResponse::InvalidDataValue {
        field_name: "split_payments.payload_split_payment.ledger.amount",
    });
}

Issue: The validation only rejects positive amounts, but the comment says "negative = debit from the payment". However, a zero amount would pass validation, which doesn't make sense for a split payment ledger entry.

Suggestion: Consider validating amount != 0 or clarify if zero amounts are semantically valid for this use case.


This is an automated review. Please verify the findings before making changes.

@XyneSpaces

Copy link
Copy Markdown

[should-fix] In payments.rs (common_types), the new PayloadSplitPaymentRequest struct and related types are well-documented with docstrings. However, there's a potential validation gap.

The PayloadLedgerItem has an amount field as MinorUnit with a comment "Signed amount in minor units (negative = debit from the payment)".

In helpers.rs, validation checks that amount > 0 returns an error, but negative amounts are allowed. Please confirm this is intentional - typically split payments should have specific validation for:

  1. Sum of all ledger amounts equals the total payment amount
  2. No single receiver gets more than the total payment
  3. At least one negative entry (the debit) exists

Without these validations, malformed requests could cause reconciliation issues.

@XyneSpaces

Copy link
Copy Markdown

⚠️ Ledger amount validation logic

In the validation for PayloadSplitPaymentRequest, the PR adds a check that ledger amounts must be ≤ 0. Please verify this is the correct direction for the constraint:

// From diff: validation in helpers.rs
if item.amount > 0 {  // This check implies amounts must be <= 0
    return Err(...);
}

Question: If this represents money being transferred out (debit), negative or zero makes sense. But if this represents received funds (credit), the constraint may be inverted. Confirm the sign convention matches Payload's API specification.

Also ensure receiver_id non-empty check handles the case where the connector might accept null/empty for certain split types (e.g., marketplace fees vs. direct splits).

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.

2 participants