Skip to content

Add pre-flight payment safety checks with SEP-29 memo detection #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zeemscript merged 1 commit into from
Jul 23, 2026
Merged

Add pre-flight payment safety checks with SEP-29 memo detection #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 159 additions & 43 deletions src/controllers/stellar/paymentController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
buildPaymentTransaction,
buildPathPaymentTransaction,
buildSep7Uri,
calculateFeeSplit,
preflightPayment,
submitTransaction,
verifyTransaction,
verifyPaymentOperations,
Expand All @@ -29,6 +31,66 @@ import {
paymentsFailed,
} from "../../config/metrics.js";

/**
* Resolve the item, its creator, and the settlement destination wallet for a
* purchase. Shared by initializePayment and the pre-flight endpoint so both
* look up the same destination the same way.
*/
const resolvePaymentDestination = async ({ itemType, itemId, session }) => {
const Model = itemType === "book" ? Book : Course;
const populateField = itemType === "book" ? "author" : "createdBy";

const query = Model.findById(itemId).populate(populateField, "stellarWallet name");
const item = session ? await query.session(session) : await query;
Comment on lines +40 to +44

if (!item) {
return { error: { status: 404, message: `${itemType} not found` } };
}

const creator = itemType === "book" ? item.author : item.createdBy;
const platformCollectEnabled = process.env.PLATFORM_COLLECT_ENABLED === "true";
let destinationPublicKey;
let settlementMode = "direct";

if (!creator?.stellarWallet?.publicKey) {
if (!platformCollectEnabled) {
return {
error: {
status: 400,
message: "Creator has not connected their Stellar wallet yet",
},
};
}

const platformWalletKey =
process.env.PLATFORM_WALLET_PUBLIC_KEY || PLATFORM_WALLET_PUBLIC_KEY;
if (!platformWalletKey) {
return {
error: {
status: 500,
message: "Platform wallet is not configured for platform-collect mode",
},
};
}

destinationPublicKey = platformWalletKey;
settlementMode = "platform_collect";
} else {
destinationPublicKey = creator.stellarWallet.publicKey;
}

return { item, creator, destinationPublicKey, settlementMode };
};

/**
* Platform memo convention: purchases are tagged DNB-<ITEMTYPE>-<last 8 chars
* of the Mongo item id>, always as a text memo. This is always non-empty, so
* it already satisfies SEP-29 "some memo present" destinations; it does not
* substitute for a destination-specific memo (e.g. an exchange deposit id).
*/
const buildPurchaseMemo = (itemType, itemId) =>
`DNB-${itemType.toUpperCase()}-${itemId.toString().slice(-8)}`;

/**
* Get a quote for paying with a non-USDC asset via path payment
* POST /api/stellar/payment/quote
Expand Down Expand Up @@ -140,6 +202,76 @@ export const getQuote = async (req, res) => {
}
};

/**
* Run pre-flight payment safety checks (destination existence, USDC
* trustline, source balance/reserve, SEP-29 memo-required) before the
* frontend prompts the wallet to sign anything.
* POST /api/stellar/payment/preflight
*/
export const getPaymentPreflight = async (req, res) => {
try {
const buyerId = req.user._id;
const { itemType, itemId } = req.body;

if (!["book", "course"].includes(itemType)) {
return res.status(400).json({
success: false,
message: "Invalid item type. Must be 'book' or 'course'",
});
}

const buyer = await User.findById(buyerId);
if (!buyer?.stellarWallet?.publicKey) {
return res.status(400).json({
success: false,
message: "Please connect your Stellar wallet first",
});
}

const resolved = await resolvePaymentDestination({ itemType, itemId });
if (resolved.error) {
return res.status(resolved.error.status).json({
success: false,
message: resolved.error.message,
});
}

const { item, destinationPublicKey, settlementMode } = resolved;

if (!item.price || item.price === 0) {
return res.status(400).json({
success: false,
message: "This item is free, no payment required",
});
}

const memo = buildPurchaseMemo(itemType, itemId);
const feeSplitPreview =
settlementMode === "direct" ? calculateFeeSplit(item.price) : null;

const preflight = await preflightPayment({
sourcePublicKey: buyer.stellarWallet.publicKey,
destinationPublicKey,
amount: item.price.toString(),
memo,
operationCount: feeSplitPreview ? 2 : 1,
});

res.status(200).json({
success: true,
preflight,
});
} catch (error) {
logger.error("Payment preflight error:", error);
res.status(500).json({
success: false,
message: "Failed to run payment pre-flight checks",
error:
process.env.NODE_ENV === "development" ? error.message : undefined,
});
}
};
Comment on lines +211 to +273

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Endpoint logic looks correct, but this handler and initializePayment's new preflight block (Lines 401-410) are byte-for-byte duplicated logic.

Both compute feeSplitPreview and call preflightPayment with the identical shape. If the operationCount logic or preflight params ever need a fix (e.g. adding sendAsset support), it's easy to update one call site and forget the other, silently reintroducing the exact race this PR is trying to close.

Consider extracting a shared helper, e.g.:

♻️ Suggested extraction 
const runPreflight = async ({ buyerWallet, destinationPublicKey, item, memo, settlementMode }) => {
  const feeSplitPreview =
    settlementMode === "direct" ? calculateFeeSplit(item.price) : null;

  return preflightPayment({
    sourcePublicKey: buyerWallet,
    destinationPublicKey,
    amount: item.price.toString(),
    memo,
    operationCount: feeSplitPreview ? 2 : 1,
  });
};

Then call runPreflight(...) from both getPaymentPreflight and initializePayment.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/stellar/paymentController.js` around lines 211 - 273, Extract
the duplicated fee-split and preflightPayment invocation from
getPaymentPreflight and initializePayment into a shared runPreflight helper.
Have the helper accept the buyer wallet, destinationPublicKey, item, memo, and
settlementMode, preserve the existing amount and operationCount behavior, and
update both handlers to use it.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

No Jest coverage found for the new getPaymentPreflight endpoint.

The only tests supplied (test/preflightPayment.test.js) exercise stellarService.preflightPayment/isMemoRequired directly, not the controller/route. Given this endpoint drives frontend payment gating, an endpoint-level test (mocking resolvePaymentDestination/preflightPayment, asserting the 400s for invalid itemType, missing wallet, free item, and the 200 success shape) would close the gap.

As per path instructions, "New or changed endpoints need Jest coverage (the runner uses --experimental-vm-modules)."

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/stellar/paymentController.js` around lines 211 - 273, The new
getPaymentPreflight endpoint lacks controller-level Jest coverage. Add endpoint
tests that mock resolvePaymentDestination and preflightPayment, covering invalid
itemType, missing Stellar wallet, free items, and a successful response
asserting the 200 status and preflight payload shape; ensure the tests use the
project’s --experimental-vm-modules-compatible setup.

Source: Path instructions


/**
* Initialize a payment - creates pending transaction and returns XDR to sign
* POST /api/stellar/payment/initialize
Expand Down Expand Up @@ -180,53 +312,17 @@ export const initializePayment = async (req, res) => {
});
}

// Get item details
const Model = itemType === "book" ? Book : Course;
const populateField = itemType === "book" ? "author" : "createdBy";

const item = await Model.findById(itemId)
.populate(populateField, "stellarWallet name")
.session(session);

if (!item) {
// Get item details and resolve the settlement destination
const resolved = await resolvePaymentDestination({ itemType, itemId, session });
if (resolved.error) {
await session.abortTransaction();
return res.status(404).json({
return res.status(resolved.error.status).json({
success: false,
message: `${itemType} not found`,
message: resolved.error.message,
});
}

const creator = itemType === "book" ? item.author : item.createdBy;
const platformCollectEnabled =
process.env.PLATFORM_COLLECT_ENABLED === "true";
let destinationPublicKey;
let settlementMode = "direct";

// Check creator has wallet or platform-collect mode is enabled
if (!creator?.stellarWallet?.publicKey) {
if (!platformCollectEnabled) {
await session.abortTransaction();
return res.status(400).json({
success: false,
message: "Creator has not connected their Stellar wallet yet",
});
}

const platformWalletKey =
process.env.PLATFORM_WALLET_PUBLIC_KEY || PLATFORM_WALLET_PUBLIC_KEY;
if (!platformWalletKey) {
await session.abortTransaction();
return res.status(500).json({
success: false,
message: "Platform wallet is not configured for platform-collect mode",
});
}

destinationPublicKey = platformWalletKey;
settlementMode = "platform_collect";
} else {
destinationPublicKey = creator.stellarWallet.publicKey;
}
const { item, creator, destinationPublicKey, settlementMode } = resolved;

// Check if item is free
if (!item.price || item.price === 0) {
Expand Down Expand Up @@ -271,7 +367,7 @@ export const initializePayment = async (req, res) => {
}

// Generate unique memo for this transaction
const memo = `DNB-${itemType.toUpperCase()}-${itemId.toString().slice(-8)}`;
const memo = buildPurchaseMemo(itemType, itemId);

// Build the payment transaction (single op full amount for platform collect, split for direct if fee configured)
const isPathPayment = sendAssetInput && sendMax;
Comment on lines 372 to 373
Expand Down Expand Up @@ -302,6 +398,26 @@ export const initializePayment = async (req, res) => {
applyPlatformFee: settlementMode === "direct",
});
} else {
const feeSplitPreview =
settlementMode === "direct" ? calculateFeeSplit(item.price) : null;

const preflight = await preflightPayment({
sourcePublicKey: buyer.stellarWallet.publicKey,
destinationPublicKey,
amount: item.price.toString(),
memo,
operationCount: feeSplitPreview ? 2 : 1,
});

if (!preflight.ok) {
await session.abortTransaction();
return res.status(400).json({
success: false,
message: "Payment failed pre-flight safety checks",
reasons: preflight.reasons,
});
}

Comment on lines +401 to +420

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Preflight gate correctly blocks and aborts on failure — but it only runs for the non-path-payment branch.

isPathPayment (Line 373) transactions skip preflightPayment entirely and go straight to buildPathPaymentTransaction. That means for path payments, none of the destination checks (DESTINATION_ACCOUNT_MISSING, DESTINATION_NO_TRUSTLINE, DESTINATION_MEMO_REQUIRED) — which are asset-agnostic and would apply regardless of the source asset — are validated before the wallet is asked to sign. Issue #59 explicitly calls for pre-flight validation "before unsigned XDR creation and wallet signing," without carving out an exception for path payments.

I understand preflightPayment's source-side checks (USDC balance/reserve) can't apply as-is to path payments since the source asset may not be USDC — but the destination-side checks could still run. Worth either:

  1. Splitting preflightPayment so destination-only checks can be invoked independently of source-asset assumptions, or
  2. Explicitly documenting that path payments intentionally rely solely on the submit-time result-code backstop for now.

Right now the fallback (submit-time result-code handling) does still catch this at signing/submission time, which limits the blast radius — but the wallet will still be asked to sign a doomed-to-fail transaction, which is exactly what this PR set out to prevent.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/controllers/stellar/paymentController.js` around lines 401 - 420, Ensure
the isPathPayment branch also performs destination-side preflight validation
before buildPathPaymentTransaction and wallet signing. Refactor preflightPayment
or extract a destination-only validation helper so checks such as missing
accounts, trustlines, and required memos apply without assuming a USDC source
asset; preserve the existing source-asset checks for non-path payments and abort
the session with the existing failure response when validation fails.

paymentTx = await buildPaymentTransaction({
sourcePublicKey: buyer.stellarWallet.publicKey,
destinationPublicKey,
Expand Down
2 changes: 2 additions & 0 deletions src/routes/stellar/paymentRoutes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
initializePayment,
submitPayment,
getQuote,
getPaymentPreflight,
getTransactionHistory,
getTransaction,
cancelTransaction,
Expand All @@ -17,6 +18,7 @@ router.use(protect);

// Payment flow
router.post("/quote", getQuote);
router.post("/preflight", getPaymentPreflight);
router.post("/initialize", initializePayment);
router.post("/submit", submitPayment);

Expand Down
Loading
Loading