Skip to content

federated addr. memo type inference#2744

Open
leofelix077 wants to merge 13 commits intomasterfrom
fix/harden-federated-memo-validation
Open

federated addr. memo type inference#2744
leofelix077 wants to merge 13 commits intomasterfrom
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

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

3 participants