fix(transaction): reject empty and whitespace-only memos by default (…

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");
+    });
+  });
+});

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 () => {

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);
   }
 

src/transaction/buildTransaction.ts

@@ -195,6 +195,43 @@ function resolveAsset(
  */
 export function validateMemoPolicy(params: MemoParams): SorokitResult<void> {
   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<void> {
         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);
   }
 

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)

src/transaction/index.ts

@@ -248,6 +248,7 @@ export {
 export type {
   TransactionBuilder,
   TransactionOperation,
+  SetMemoOptions,
 } from "./transactionBuilder";
 export type {
   TransactionResult,