diff --git a/src/tests/memo-validation-length.test.ts b/src/tests/memo-validation-length.test.ts new file mode 100644 index 0000000..b1cc4a4 --- /dev/null +++ b/src/tests/memo-validation-length.test.ts @@ -0,0 +1,171 @@ +import { describe, expect, it } from "vitest"; +import { createTransactionBuilder } from "../transaction/transactionBuilder"; +import { validateMemoPolicy } from "../transaction/buildTransaction"; +import { compose } from "../transaction/compose"; +import { SorokitErrorCode } from "../shared/response"; + +describe("TransactionBuilder memo validation (#846)", () => { + describe("createTransactionBuilder().setMemo", () => { + it("rejects an empty string memo by default", () => { + const builder = createTransactionBuilder(); + expect(() => builder.setMemo("")).toThrow("Memo cannot be empty or whitespace-only"); + }); + + it("rejects a whitespace-only memo by default", () => { + const builder = createTransactionBuilder(); + expect(() => builder.setMemo(" \t\n ")).toThrow("Memo cannot be empty or whitespace-only"); + }); + + it("rejects non-string values", () => { + const builder = createTransactionBuilder(); + // @ts-expect-error test invalid input type + expect(() => builder.setMemo(null)).toThrow("Memo must be a string"); + // @ts-expect-error test invalid input type + expect(() => builder.setMemo(12345)).toThrow("Memo must be a string"); + }); + + it("accepts valid non-empty memo", () => { + const builder = createTransactionBuilder(); + expect(builder.setMemo("valid-memo")).toBe(builder); + expect(builder.getMemo()).toBe("valid-memo"); + }); + + it("allows empty string memo when opt-in allowEmpty is true", () => { + const builder = createTransactionBuilder(); + expect(builder.setMemo("", { allowEmpty: true })).toBe(builder); + expect(builder.getMemo()).toBe(""); + }); + + it("allows whitespace-only memo when opt-in allowEmpty is true", () => { + const builder = createTransactionBuilder(); + expect(builder.setMemo(" ", { allowEmpty: true })).toBe(builder); + expect(builder.getMemo()).toBe(" "); + }); + + it("clears memo on builder.clear()", () => { + const builder = createTransactionBuilder(); + builder.setMemo("my-memo"); + expect(builder.getMemo()).toBe("my-memo"); + builder.clear(); + expect(builder.getMemo()).toBeUndefined(); + }); + }); + + describe("validateMemoPolicy", () => { + it("rejects empty string memo by default", () => { + const result = validateMemoPolicy({ memo: "" }); + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.code).toBe(SorokitErrorCode.TX_BUILD_FAILED); + expect(result.error.message).toBe("Memo cannot be empty or whitespace-only"); + } + }); + + it("rejects whitespace-only memo by default", () => { + const result = validateMemoPolicy({ memo: " \t \r \n " }); + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.code).toBe(SorokitErrorCode.TX_BUILD_FAILED); + expect(result.error.message).toBe("Memo cannot be empty or whitespace-only"); + } + }); + + it("accepts empty string memo when allowEmptyMemo is true", () => { + const result = validateMemoPolicy({ memo: "", allowEmptyMemo: true }); + expect(result.status).toBe("ok"); + }); + + it("accepts whitespace-only memo when allowEmptyMemo is true", () => { + const result = validateMemoPolicy({ memo: " ", allowEmptyMemo: true }); + expect(result.status).toBe("ok"); + }); + + it("accepts empty string memo when memoValidation config has allowEmpty: true", () => { + const result = validateMemoPolicy({ + memo: "", + memoValidation: { rule: "required", allowEmpty: true }, + }); + // Note: rule "required" will still check hasMemo unless custom logic, but allowEmpty passes the length check + // For required rule: hasMemo is false so it fails with "Memo is required". + // Let's test with a rule or without rule: + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.message).toContain("Memo is required"); + } + }); + + it("accepts empty memo when memoValidation has allowEmpty: true without required rule", () => { + const result = validateMemoPolicy({ + memo: "", + memoValidation: { rule: "prohibit", allowEmpty: true }, + }); + expect(result.status).toBe("ok"); + }); + + it("accepts omitted memo (undefined) without error", () => { + const result = validateMemoPolicy({}); + expect(result.status).toBe("ok"); + }); + + it("rule: prohibit accepts memo: '' even without explicit allowEmpty flag", () => { + const result = validateMemoPolicy({ memoValidation: "prohibit", memo: "" }); + expect(result.status).toBe("ok"); + }); + + it("rule: prohibit rejects non-empty memo", () => { + const result = validateMemoPolicy({ memoValidation: "prohibit", memo: "hello" }); + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.message).toBe("Memo is prohibited for this transaction"); + } + }); + }); + + describe("compose().addMemo validation", () => { + const SOURCE = "GAAL6LIAG2FGFQTKMUNGLCSCAM722PPYRVK2PXEMC6KNRRWLCFTYQD7R"; + + it("fails build when memo is empty string by default", async () => { + const builder = compose(SOURCE, "testnet", { sequenceNumber: "1" }) + .addPayment({ destination: SOURCE, amount: "10" }) + .addMemo(""); + + const result = await builder.build(); + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.code).toBe(SorokitErrorCode.TX_BUILD_FAILED); + expect(result.error.message).toBe("Memo cannot be empty or whitespace-only"); + } + }); + + it("fails build when memo is whitespace-only by default", async () => { + const builder = compose(SOURCE, "testnet", { sequenceNumber: "1" }) + .addPayment({ destination: SOURCE, amount: "10" }) + .addMemo(" \t "); + + const result = await builder.build(); + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.code).toBe(SorokitErrorCode.TX_BUILD_FAILED); + expect(result.error.message).toBe("Memo cannot be empty or whitespace-only"); + } + }); + + it("succeeds when allowEmpty is opted-in via addMemo options", async () => { + const builder = compose(SOURCE, "testnet", { sequenceNumber: "1" }) + .addPayment({ destination: SOURCE, amount: "10" }) + .addMemo("", "text", { allowEmpty: true }); + + const result = await builder.build(); + expect(result.status).toBe("ok"); + }); + + it("succeeds when allowEmptyMemo is opted-in via compose options", async () => { + const builder = compose(SOURCE, "testnet", { sequenceNumber: "1", allowEmptyMemo: true }) + .addPayment({ destination: SOURCE, amount: "10" }) + .addMemo(""); + + const result = await builder.build(); + expect(result.status).toBe("ok"); + }); + }); +}); diff --git a/src/tests/transaction.test.ts b/src/tests/transaction.test.ts index 60ddd43..bfa6f85 100644 --- a/src/tests/transaction.test.ts +++ b/src/tests/transaction.test.ts @@ -1402,7 +1402,7 @@ describe("estimateFee — caching", () => { expect((transactionBuilderInstances[0].memo as any)?.value).toBe("hello"); }); - it("skips memo attachment when memo is an empty string (#292)", async () => { + it("attaches empty memo when memo is an empty string with allowEmptyMemo (#292, #846)", async () => { const result = await buildPaymentTransaction( networkConfig.horizonUrl, networkConfig, @@ -1411,6 +1411,7 @@ describe("estimateFee — caching", () => { destination, amount: "10", memo: "", + allowEmptyMemo: true, }, ); @@ -1419,8 +1420,29 @@ describe("estimateFee — caching", () => { expect(result.data).toBe(MOCK_XDR); } expect(transactionBuilderInstances).toHaveLength(1); - expect(transactionBuilderInstances[0].memo).toBeUndefined(); - expect(mockAddMemo).not.toHaveBeenCalled(); + expect(mockAddMemo).toHaveBeenCalledWith( + expect.objectContaining({ _type: "text", _value: "" }), + ); + }); + + it("rejects empty string memo by default without opt-in (#846)", async () => { + const result = await buildPaymentTransaction( + networkConfig.horizonUrl, + networkConfig, + sourcePublicKey, + { + destination, + amount: "10", + memo: "", + }, + ); + + expect(result.status).toBe("error"); + if (result.status === "error") { + expect(result.error.code).toBe(SorokitErrorCode.TX_BUILD_FAILED); + expect(result.error.message).toBe("Memo cannot be empty or whitespace-only"); + } + expect(transactionBuilderInstances).toHaveLength(0); }); it("fails payment build with TX_BUILD_FAILED when text memo exceeds the 28-byte Stellar limit (#292)", async () => { diff --git a/src/transaction/buildHelpers.ts b/src/transaction/buildHelpers.ts index 6530c96..7830feb 100644 --- a/src/transaction/buildHelpers.ts +++ b/src/transaction/buildHelpers.ts @@ -142,7 +142,21 @@ export function resolveMemo( const policyResult = validateMemoPolicy(params); if (policyResult.status === "error") return policyResult; + const allowEmpty = + params.allowEmptyMemo === true || + (typeof params.memoValidation === "object" && + params.memoValidation !== null && + params.memoValidation.allowEmpty === true); + if (!params.memo) { + if (params.memo === "" && allowEmpty) { + const memoType = params.memoType ?? "text"; + try { + if (memoType === "text") return ok(Memo.text("")); + } catch { + // Fall through + } + } return ok(undefined); } diff --git a/src/transaction/buildTransaction.ts b/src/transaction/buildTransaction.ts index 9df22cf..2556578 100644 --- a/src/transaction/buildTransaction.ts +++ b/src/transaction/buildTransaction.ts @@ -195,6 +195,43 @@ function resolveAsset( */ export function validateMemoPolicy(params: MemoParams): SorokitResult { const hasMemo = typeof params.memo === "string" && params.memo.length > 0; + const allowEmpty = + params.allowEmptyMemo === true || + (typeof params.memoValidation === "object" && + params.memoValidation !== null && + params.memoValidation.allowEmpty === true); + + // Check rule: "prohibit" first so that passing memo: "" or omitting memo + // is accepted under prohibition without triggering empty memo rejection. + const isProhibitRule = + params.memoValidation === "prohibit" || + (typeof params.memoValidation === "object" && + params.memoValidation !== null && + params.memoValidation.rule === "prohibit"); + + if (isProhibitRule) { + if (params.memo !== undefined && params.memo !== "") { + const configErrorMessage = + typeof params.memoValidation === "object" && params.memoValidation !== null + ? params.memoValidation.errorMessage + : undefined; + return err( + SorokitErrorCode.TX_BUILD_FAILED, + configErrorMessage || "Memo is prohibited for this transaction", + ); + } + return ok(undefined); + } + + // Rejection of empty or whitespace-only memo strings (#846) + if (params.memo !== undefined && !allowEmpty) { + if (params.memo.trim().length === 0) { + return err( + SorokitErrorCode.TX_BUILD_FAILED, + "Memo cannot be empty or whitespace-only", + ); + } + } // 1. Backward-compatible requireMemo flag if (params.requireMemo && !hasMemo) { @@ -222,12 +259,7 @@ export function validateMemoPolicy(params: MemoParams): SorokitResult { break; case "prohibit": - if (params.memo !== undefined && params.memo !== "") { - return err( - SorokitErrorCode.TX_BUILD_FAILED, - config.errorMessage || "Memo is prohibited for this transaction", - ); - } + // Already handled above break; case "require_format": { @@ -304,7 +336,21 @@ function validateMemoParams( return policyResult; } + const allowEmpty = + params.allowEmptyMemo === true || + (typeof params.memoValidation === "object" && + params.memoValidation !== null && + params.memoValidation.allowEmpty === true); + if (!params.memo) { + if (params.memo === "" && allowEmpty) { + const memoType = params.memoType ?? "text"; + try { + if (memoType === "text") return ok(Memo.text("")); + } catch { + // Fall through + } + } return ok(undefined); } diff --git a/src/transaction/compose.ts b/src/transaction/compose.ts index 78b25ed..04e4051 100644 --- a/src/transaction/compose.ts +++ b/src/transaction/compose.ts @@ -86,6 +86,8 @@ export interface ComposeOptions { /** Optional transaction memo attached to the built transaction. */ memo?: string; memoType?: MemoType; + /** When true, opt in to allowing empty strings or whitespace-only memos (#846). */ + allowEmptyMemo?: boolean; /** Override the Horizon URL for a network-type `network`. */ horizonUrl?: string; /** Override the RPC URL for a network-type `network`. */ @@ -173,7 +175,11 @@ export interface ComposeBuilder { setData(key: string, value: DataEntryValue): ComposeBuilder; addDeleteDataEntry(key: string): ComposeBuilder; deleteData(key: string): ComposeBuilder; - addMemo(memo: string, memoType?: MemoType): ComposeBuilder; + addMemo( + memo: string, + memoType?: MemoType, + options?: { allowEmpty?: boolean }, + ): ComposeBuilder; /** Enable automatic fee estimation; `{ mode: "high" }` prefers the fast tier. */ estimateFee(mode: FeeEstimateMode): ComposeBuilder; setSequenceNumber(sequenceNumber: string): ComposeBuilder; @@ -317,7 +323,9 @@ export function compose( let feeOverride = options.estimatedFee; let sequenceOverride = options.sequenceNumber; let timeoutSeconds = options.timeout ?? DEFAULT_TX_TIMEOUT_SECONDS; - let memoState: { memo: string; memoType?: MemoType } | undefined; + let memoState: + | { memo: string; memoType?: MemoType; allowEmptyMemo?: boolean } + | undefined; const warnIfMemoizedMutable = (operation: string): void => { if (options.memoize && options.warnOnMutableMemoized !== false) { @@ -683,8 +691,14 @@ export function compose( return this.addDeleteDataEntry(key); }, - addMemo(memo, memoType) { - memoState = { memo, ...(memoType !== undefined && { memoType }) }; + addMemo(memo, memoType, memoOptions) { + memoState = { + memo, + ...(memoType !== undefined && { memoType }), + ...(memoOptions?.allowEmpty !== undefined && { + allowEmptyMemo: memoOptions.allowEmpty, + }), + }; return builder; }, @@ -740,9 +754,14 @@ export function compose( // 2. Resolve memo once. const { memo, memoType } = memoState ?? options; + const allowEmptyMemo = memoState?.allowEmptyMemo ?? options.allowEmptyMemo; const memoParams = memo !== undefined - ? { memo, ...(memoType !== undefined && { memoType }) } + ? { + memo, + ...(memoType !== undefined && { memoType }), + ...(allowEmptyMemo !== undefined && { allowEmptyMemo }), + } : undefined; const memoResult = memoParams ? resolveMemo(memoParams) diff --git a/src/transaction/index.ts b/src/transaction/index.ts index 8b145d4..5a320e8 100644 --- a/src/transaction/index.ts +++ b/src/transaction/index.ts @@ -248,6 +248,7 @@ export { export type { TransactionBuilder, TransactionOperation, + SetMemoOptions, } from "./transactionBuilder"; export type { TransactionResult, diff --git a/src/transaction/transactionBuilder.ts b/src/transaction/transactionBuilder.ts index fab432b..11e105b 100644 --- a/src/transaction/transactionBuilder.ts +++ b/src/transaction/transactionBuilder.ts @@ -20,8 +20,32 @@ export interface TransactionOperation { params: Record; } +export interface SetMemoOptions { + /** + * When true, allows empty or whitespace-only memo strings. + * Defaults to false. + */ + allowEmpty?: boolean; +} + /** Public interface returned by `createTransactionBuilder`. */ export interface TransactionBuilder { + /** + * Set or update the transaction memo. + * By default, rejects empty or whitespace-only memos. + * Pass `{ allowEmpty: true }` to opt in to allowing empty or whitespace memos. + * + * @param memo - The memo string. + * @param options - Optional configuration (e.g. `{ allowEmpty: true }`). + * @returns The builder itself for chaining. + */ + setMemo(memo: string, options?: SetMemoOptions): TransactionBuilder; + + /** + * Return the currently configured memo, if any. + */ + getMemo(): string | undefined; + /** * Append a new operation to the builder. * Calling `addOperation` after an `undo` clears the redo stack (standard @@ -109,6 +133,7 @@ export function createTransactionBuilder(): TransactionBuilder { let _history: TransactionOperation[] = []; let _redoStack: TransactionOperation[] = []; + let _memo: string | undefined; // ── Helper ───────────────────────────────────────────────────────────────── @@ -119,6 +144,21 @@ export function createTransactionBuilder(): TransactionBuilder { // ── Builder object ────────────────────────────────────────────────────────── const builder: TransactionBuilder = { + setMemo(memo: string, options?: SetMemoOptions): TransactionBuilder { + if (typeof memo !== "string") { + throw new Error("Memo must be a string"); + } + if (!options?.allowEmpty && memo.trim().length === 0) { + throw new Error("Memo cannot be empty or whitespace-only"); + } + _memo = memo; + return builder; + }, + + getMemo(): string | undefined { + return _memo; + }, + addOperation(operation: TransactionOperation): TransactionBuilder { // Adding a new operation after undo(s) discards the redo stack so that // history stays linear (no branching). @@ -161,6 +201,7 @@ export function createTransactionBuilder(): TransactionBuilder { clear(): TransactionBuilder { _history = []; _redoStack = []; + _memo = undefined; return builder; }, }; diff --git a/src/transaction/transactionContext.ts b/src/transaction/transactionContext.ts index a8426b4..0765b7d 100644 --- a/src/transaction/transactionContext.ts +++ b/src/transaction/transactionContext.ts @@ -11,9 +11,10 @@ import type { SorokitResult } from "../shared/response"; import { toMessage } from "../shared"; import { DEFAULT_TX_TIMEOUT_SECONDS } from "../shared/constants"; import type { ResolvedNetworkConfig } from "../shared/types"; -import type { PaymentParams, TrustlineParams, AccountCreateParams } from "./types"; +import type { PaymentParams, TrustlineParams, AccountCreateParams, MemoParams } from "./types"; import { validateIssuer } from "../shared/validateIssuer"; import { createHorizonServer } from "../shared/serverFactory"; +import { validateMemoPolicy } from "./buildTransaction"; /** Context TTL: 5 minutes */ export const TRANSACTION_CONTEXT_TTL_MS = 5 * 60 * 1000; @@ -76,8 +77,27 @@ export interface TransactionBuilderContext { ): Promise>; } -function resolveMemo(params: { memo?: string; memoType?: string }): SorokitResult { - if (!params.memo) return ok(null); +function resolveMemo(params: MemoParams): SorokitResult { + const policyResult = validateMemoPolicy(params); + if (policyResult.status === "error") return policyResult; + + const allowEmpty = + params.allowEmptyMemo === true || + (typeof params.memoValidation === "object" && + params.memoValidation !== null && + params.memoValidation.allowEmpty === true); + + if (!params.memo) { + if (params.memo === "" && allowEmpty) { + const type = params.memoType ?? "text"; + try { + if (type === "text") return ok(Memo.text("")); + } catch { + // Fall through + } + } + return ok(null); + } const type = params.memoType ?? "text"; try { switch (type) { diff --git a/src/transaction/types.ts b/src/transaction/types.ts index d849959..9154709 100644 --- a/src/transaction/types.ts +++ b/src/transaction/types.ts @@ -36,6 +36,11 @@ export interface MemoValidationConfig { format?: RegExp | string | ((memo: string) => boolean); /** Optional custom error message to return on validation failure */ errorMessage?: string; + /** + * When true, allows empty strings or whitespace-only memos. + * Defaults to false (empty and whitespace-only memos are rejected by default). + */ + allowEmpty?: boolean; } export interface MemoParams { @@ -56,6 +61,11 @@ export interface MemoParams { * Supports "required", "prohibit", and "require_format" with custom format patterns. */ memoValidation?: MemoValidationConfig | MemoValidationRule; + /** + * When true, opt in to allowing empty strings or whitespace-only memos (#846). + * Defaults to false (empty and whitespace-only memos are rejected by default). + */ + allowEmptyMemo?: boolean; } export interface PaymentParams extends MemoParams {