Skip to content

federated addr. memo type inference#2744

Open
leofelix077 wants to merge 15 commits into
masterfrom
fix/harden-federated-memo-validation
Open

federated addr. memo type inference#2744
leofelix077 wants to merge 15 commits into
masterfrom
fix/harden-federated-memo-validation

Conversation

@leofelix077
Copy link
Copy Markdown
Collaborator

@leofelix077 leofelix077 commented Apr 30, 2026

closes #2717

This PR adds safer support for federation-provided memos in Send
It now recognizes the main SEP-0002 memo formats (text, id, hash) instead of treating everything the same way.
It improves validation/error handling when federation servers return invalid data (including invalid destination addresses).
It also fixes send-flow behavior so memo data is kept consistent when users change recipient input.
+ it adds test coverage for the new federation memo scenarios, including prefill, invalid responses, and recipient switching.

federated-switch-address-clears-memo.webm
federated-hash-memo.webm
federated-id-memo.webm
federated-text-memo.webm
federated-invalid-account-id-memo.webm

@leofelix077 leofelix077 self-assigned this Apr 30, 2026
Copilot AI review requested due to automatic review settings April 30, 2026 15:51
@leofelix077 leofelix077 added bug Something isn't working wip not for merging yet don't review yet wip / tests in progress / missing videos or screenshots / pending self code-review labels Apr 30, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for SEP-0002 federation memo handling in the Send flow by persisting the federation-provided memo + memo_type, validating them, and using the correct Stellar memo type when building classic transactions.

Changes:

  • Added validateFederationMemo / buildMemoFromFederation helper (SEP-0002 constraints) with unit tests.
  • Threaded memoType through Redux + Send flow and used it when building the classic transaction XDR for simulation.
  • Added Playwright stubs + e2e coverage for federation responses that include memo fields, plus a new i18n string for invalid federation account_id.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
extension/src/popup/locales/pt/translation.json Adds new federation-validation error string (pt locale).
extension/src/popup/locales/en/translation.json Adds new federation-validation error string (en locale).
extension/src/popup/helpers/federationMemo.ts New SEP-0002 memo validation + memo builder helper.
extension/src/popup/helpers/tests/federationMemo.test.ts Unit tests covering memo validation + memo construction.
extension/src/popup/ducks/transactionSubmission.ts Persists memoType in transaction submission state and adds reducer/action.
extension/src/popup/components/send/SendTo/index.tsx Saves federation memo + memo type into Redux during destination resolution.
extension/src/popup/components/send/SendTo/hooks/useSendToData.tsx Parses/validates federation response (account_id, memo, memo_type).
extension/src/popup/components/send/SendAmount/hooks/useSimulateTxData.tsx Uses memoType when building the memo for classic tx simulation.
extension/e2e-tests/sendPayment.test.ts Adds e2e tests for federation memos and invalid federation account_id.
extension/e2e-tests/helpers/stubs.ts Adds stubFederationWithMemo to override the default federation route.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread extension/src/popup/components/send/SendTo/index.tsx
Comment thread extension/src/popup/locales/pt/translation.json Outdated
Comment thread extension/src/popup/components/send/SendTo/hooks/useSendToData.tsx Outdated
Comment thread extension/src/popup/components/send/SendTo/hooks/useSendToData.tsx
Comment thread extension/src/popup/components/send/SendTo/hooks/useSendToData.tsx Outdated
Comment thread extension/src/popup/components/send/SendTo/index.tsx Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread extension/src/popup/locales/pt/translation.json Outdated
Comment thread extension/src/popup/components/send/SendTo/hooks/useSendToData.tsx
Comment thread extension/src/popup/components/send/SendTo/index.tsx Outdated
Comment thread extension/src/popup/ducks/transactionSubmission.ts Outdated
Comment thread extension/src/popup/helpers/federationMemo.ts
@leofelix077 leofelix077 changed the title federated memo validation federated memo type inference May 4, 2026
@leofelix077 leofelix077 changed the title federated memo type inference federated addr. memo type inference May 4, 2026
@leofelix077 leofelix077 requested a review from Copilot May 4, 2026 17:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@leofelix077 leofelix077 requested a review from Copilot May 4, 2026 18:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread extension/src/popup/helpers/federationMemo.ts Outdated
Comment thread extension/src/popup/components/send/SendTo/index.tsx Outdated
Comment thread extension/src/popup/components/send/SendTo/index.tsx Outdated
Comment thread extension/src/popup/components/send/SendTo/hooks/useSendToData.tsx
Comment thread extension/src/popup/helpers/__tests__/federationMemo.test.ts

const MAX_MEMO_BYTES = 28;
const MAX_UINT64 = BigInt("18446744073709551615");
const HEX_32_BYTES_RE = /^[0-9a-fA-F]{64}$/;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these var were set in federationMemo.ts. Let's reuse

@piyalbasu
Copy link
Copy Markdown
Contributor

Code review

Found 2 issues:

  1. State leak: when the user navigates back from SendAmount to SendTo and replaces a federation address (e.g. name*exchange.com resolving to memo=12345, memo_type=id) with a regular G... recipient, the else branch resets memoType to "" but never clears memo. The previous federation memo silently rides on the new transaction with an empty type. Fix: also dispatch(saveMemo("")) in the else branch.

dispatch(saveDestination(validatedDestination));
dispatch(saveDestinationAsset(""));
dispatch(saveFederationAddress(validatedFedAdress || ""));
if (validatedFedAdress && federationMemo !== undefined) {
dispatch(saveMemo(federationMemo));
dispatch(saveMemoType(federationMemoType || ""));
} else {
dispatch(saveMemoType(""));
}
goToNext();
};

  1. UX trap: EditMemo's onSubmit dispatches saveMemo but never saveMemoType. Once a federation address has set memoType to hash or id, the user cannot clear or change the memo type from the edit dialog — they're locked into typing 64 hex chars or a uint64, with no way to revert to a freeform text memo without going back and changing the recipient. If federation-supplied types are intentionally enforced (per SEP-0002), consider rendering the field read-only or hiding the editor when a federation memo type is present; otherwise dispatch saveMemoType("") on submit so the user can escape the constraint.

setMemoEditingContext(null);
}}
onSubmit={async ({ memo }: { memo: string }) => {
dispatch(saveMemo(memo));
setIsEditingMemo(false);
// Regenerate transaction XDR with new memo (now reads memo from Redux state inside fetchData)
await fetchSimulationData();
// Reopen review sheet after memo is saved and XDR is regenerated only if user came from review flow
if (memoEditingContext === MemoEditingContext.Review) {
setIsReviewingTx(true);
setMemoEditingContext(null);
}
}}
disabled={isMemoDisabled}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@piyalbasu
Copy link
Copy Markdown
Contributor

The Claude review had 2 good finds here:

For 1, it wasn't clear to me if saveMemo is being called somewhere else and we're trying to avoid overwriting it here. I think it's a bit of a footgun in that memo and memo type aren't being saved together

Similar concern on issue 2

@leofelix077 leofelix077 removed wip not for merging yet don't review yet wip / tests in progress / missing videos or screenshots / pending self code-review labels May 5, 2026
@leofelix077
Copy link
Copy Markdown
Collaborator Author

@piyalbasu yep. it's a good catch. I updated it to save memo and type together, and clearing memo when switching from federated address + added the e2e tests for it

Comment on lines +61 to +68
if (!HEX_32_BYTES_RE.test(memo)) {
throw new Error(
"Federation memo hash must be a 64-character hex string (32 bytes)",
);
}
break;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SEP-0002 specifies hash memos as base64-encoded, but this validates them as hex (/^[0-9a-fA-F]{64}$/). A 32-byte hash in base64 is 44 chars, not 64 hex chars — so any spec-compliant federation server returning memo_type: "hash" will fail validation here and the user sees "Failed to resolve federated address".

Spec quote: "for hash this should be base64-encoded"SEP-0002.

Mobile PR stellar/freighter-mobile#821 handles this with Buffer.from(memo, "base64") and asserts length === 32 before calling Memo.hash(buffer).

Two changes needed:

  1. Validate as base64 → 32 bytes here (replace HEX_32_BYTES_RE).
  2. Line 94 below — Memo.hash(memo) also needs the decoded buffer: Memo.hash(Buffer.from(memo, "base64")). Memo.hash accepts a 64-char hex string or a 32-byte Buffer, so passing the raw base64 string silently produces an incorrect hash.

The same hex-only assumption is mirrored in useValidateMemo.ts:23-30 (the EditMemo path), which would similarly reject a real hash memo if the user opens the editor on a federated address.

After fixing, the e2e test in sendPayment.test.ts that stubs hashMemo = "93a4c1c4...64chars" will need to use a base64 string instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memo type inference for federated address

4 participants