Skip to content

feat: stellar memo confirm UI - #323

Open
khanti42 wants to merge 27 commits into
mainfrom
feat/stellar-memo-confirm-ui
Open

khanti42 wants to merge 27 commits into
mainfrom
feat/stellar-memo-confirm-ui

Conversation

@khanti42

@khanti42 khanti42 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Explanation

Today, when a Stellar send destination requires a memo (SEP-29 RequiresMemo), validation fails before the user can fix it in the send confirmation UI. There is no way to open a draft confirmation, edit a memo, and continue to a successful rebuild/sign.

This PR adds the confirm-UI recovery path on top of memo attach (#289) and recoverable refresh (#291):

Note: includes #291 + confirm UI; #289 already on main.

Screen.Recording.2026-09-17.at.10.36.22.mov
  • On RequiresMemo, rebuild a draft send with skipMemoRequirementCheck: true and open the confirmation dialog with a banner / Add–Update memo flow (MemoEdit).
  • Store the memo on confirmation context (context.memo), not on client confirmSend wire params (memo / memoType).
  • Dialog resolves as { confirmed, memo? }. Live refresh treats RequiresMemo as recoverable so saving a memo can restart validation/security scanning without nulling securityScanRequest.
  • After confirm, re-validate with the memo attached (memo check enforced again) before signing. Confirming without a memo still fails as invalid.

Memo resolution uses resolveStellarMemo from the attach path: all-digit uint64 → memo id, otherwise text (≤ 28 UTF-8 bytes). Draft UI validation follows the same rules via getMemoDraftValidationError in api/memo.

Depends on / must land after: #289 (memo attach / resolveStellarMemo) and #291 (recoverable confirmation refresh). This PR merges both and adds the UI + context.memo wiring.

References

Requirements

Related to WPN-2033 / WPN-2034 (SEP-29 memo recovery)

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

…and attach resolved Stellar memos on send build path
@khanti42 khanti42 changed the title Feat/stellar memo confirm UI feat: stellar memo confirm UI Sep 17, 2026
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
53.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

* When true, SEP-29 memo-required checks are skipped during local simulation.
* Used to build a draft envelope for a recoverable RequiresMemo confirmation.
*/
skipMemoRequirementCheck?: boolean;

@khanti42 khanti42 Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see comment above

): Promise<void> {
const { id } = options;
await resolveInterface(id, false);
await resolveInterface(id, { confirmed: false });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't keep a boolean and read memo from context after resolve because after resolveInterface, the dialog is done; pulling memo from context again is awkward with the snaps interface lifecycle. Returning memo in the resolve payload is more straightforward handoff to confirmSend.

Why not put memo on confirmSend RPC params?
Memo is UI-owned for SEP-29 recovery (add/edit in the dialog). We deliberately don’t add client wire memo / memoType.

So: object result = confirm + optional memo in one shot; cancel uses the same shape for consistency.

* @param params.destination - The destination address.
* @param params.destinationAccount - The destination account.
* @param params.memo - Optional Stellar memo value to attach to the envelope.
* @param params.skipMemoRequirementCheck - When true, skips SEP-29 memo-required checks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before this PR, a send to a SEP-29 memo-required destination with no memo failed validation and opened a dismiss-only error confirmation — user couldn’t fix the memo in-flow.

We still need a built envelope to open the normal send confirm (fees, scan, etc.). So after RequiresMemoException, confirmSend rebuilds once with skipMemoRequirementCheck: true to skip only the memo-required check and open a recoverable confirm with Add/Update memo.

That flag is not used for signing. After the user adds a memo (context.memo), refresh / post-confirm rebuild run without the flag so SEP-29 is enforced again before sign.

@stanleyyconsensys stanleyyconsensys left a comment

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.

left batch 1 comments

* @param value - Raw draft from the confirmation UI (may include whitespace).
* @returns Locale error key, or `null` when empty/whitespace or valid.
*/
export function getMemoDraftValidationError(

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.

not sure if we should put it here, it is similar to resolveStellarMemo

* @returns The transaction and whether memo recovery UI should be shown.
* @throws {UserRejectedRequestError} After the pre-submit error dialog is dismissed.
*/
async #buildSendTransactionForConfirm(params: {

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.

i think it is a bit repeat

all we wanna is to skip memo require at first time

and then return to caller , we need memo

why not just skip it and build it

and then use assertMemoWhenDestinationRequires to validate ?

Comment on lines +178 to +183
if (requiresMemoRecovery && !confirmedMemo) {
return {
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
}

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.

you dont need it, refreshTransactionAfterConfirmation will do the job

Suggested change
if (requiresMemoRecovery && !confirmedMemo) {
return {
valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
}

Comment on lines +175 to +176
const trimmedMemo = dialogResult.memo?.trim();
const confirmedMemo = trimmedMemo === '' ? undefined : trimmedMemo;

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.

nit:

|| allow "" to undefined
?? not allow

Suggested change
const trimmedMemo = dialogResult.memo?.trim();
const confirmedMemo = trimmedMemo === '' ? undefined : trimmedMemo;
const trimmedMemo = dialogResult.memo?.trim() || undefined

Comment on lines +492 to +494
// Keep the original request so Add/Update memo still works on this
// dead-end validation dialog (no localSimulation / refresh).
request,

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.

display error is not necessary to own the request, becoz we skip the memo error , this displayDialogWithErrorMessage only handle non-memo error

assetId: KnownCaip19AssetIdOrSlip44Id;
destination: string;
memo?: string;
skipMemoRequirementCheck?: boolean;

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.

i would prefer

skipExceptions: TransactionValidationException[]

it is more flexible

@khanti42
khanti42 force-pushed the feat/stellar-memo-confirm-ui branch from 0d0005c to d7fc796 Compare September 17, 2026 23:18
@stanleyyconsensys
stanleyyconsensys marked this pull request as ready for review September 18, 2026 02:01
@stanleyyconsensys
stanleyyconsensys requested a review from a team as a code owner September 18, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants