Skip to content

feat(stellar): unify refresher key resolution for open and save flows - #378

Open
khanti42 wants to merge 11 commits into
mainfrom
feat/confirmation-refresher-keys-alignment
Open

khanti42 wants to merge 11 commits into
mainfrom
feat/confirmation-refresher-keys-alignment

Conversation

@khanti42

@khanti42 khanti42 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Explanation

Refactor: Confirmation Context Refreshers

  • Core Change: Persist preference- and flow-enabled refresher keys at dialog open, then reuse that snapshot on MemoEdit Save so restarts do not re-fetch live preferences.
  • Policy Alignment: Open and Save use the same ungated key set. Fetch-status gates stay in the refreshers (shouldFetch / recoveryResult no-op Error slices such as RequiresMemo) until Save resets status and restarts the chain.
  • Utility Extraction: Extracted a shared resolveRefresherKeys utility that maps enable flags (enablePricing, enableSecurityScan, enableLocalSimulation) to refresher keys. It does not take fetch status.
  • Controller/Handler Updates:
    • Persist and reuse resolved keys (refresherKeys) from the dialog-open snapshot.
    • Prevents preference drift between open and save paths.

References

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

@khanti42
khanti42 force-pushed the feat/confirmation-refresher-keys-alignment branch from 3440631 to 5916383 Compare September 28, 2026 17:48
@khanti42
khanti42 force-pushed the feat/confirmation-refresher-keys-alignment branch from 5916383 to 0879f5a Compare September 28, 2026 17:50
@khanti42
khanti42 marked this pull request as ready for review September 29, 2026 07:19
@khanti42
khanti42 requested a review from a team as a code owner September 29, 2026 07:19
@khanti42
khanti42 deployed to default-branch September 29, 2026 07:19 — with GitHub Actions Active
Comment thread packages/stellar-wallet-snap/CHANGELOG.md Outdated
Comment thread packages/stellar-wallet-snap/src/ui/confirmation/controller.tsx Outdated
@khanti42
khanti42 force-pushed the feat/confirmation-refresher-keys-alignment branch from 42aedeb to 11f90fd Compare September 29, 2026 08:56

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The open-path gates are lost, and late snapshot persistence creates a Save race.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Unifies refresher-key resolution and reuses the dialog-open snapshot during MemoEdit saves.

Changes:

  • Adds shared refresher-key resolution.
  • Persists and reuses enabled refresher keys.
  • Expands utility and MemoEdit tests.
File Description
ui/​confirmation/​controller.tsx Resolves, schedules, and persists refresher keys.
ui/​confirmation/​utils.ts Adds the shared key resolver.
ui/​confirmation/​utils.test.ts Tests resolver combinations.
ui/​confirmation/​api.ts Adds refresher keys to confirmation context.
MemoEdit/​events.tsx Restarts only snapshot-enabled refreshers.
MemoEdit/​events.test.tsx Tests selective and disabled restarts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/stellar-wallet-snap/src/ui/confirmation/controller.tsx Outdated
Comment thread packages/stellar-wallet-snap/src/ui/confirmation/controller.tsx Outdated

const { scope } = baseContext;
if (canRestartRefresh(baseContext) && typeof scope === 'string') {
const refresherKeys = Array.isArray(baseContext.refresherKeys)

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:
we can use a superstruct to validate the context

const RestartRefreshContextStruct = type({
  transaction: string(),
  accountId: string(),
  transactionsFetchStatus: enums(Object.values(FetchStatus)),
  refresherKeys: array(enums(Object.values(ConfirmationContextRefresherKey))),
  scope: KnownCaip2ChainIdStruct
})
export type RestartRefreshContext = Infer<typeof RestartRefreshContextStruct>;

canRestartRefresh(context: unknown): asserts context is RestartRefreshContext   {
  RestartRefreshContextStruct.is(context)
}

@stanleyyconsensys stanleyyconsensys changed the title feat(stellar-wallet-snap): unify refresher key resolution for open and save flows feat(stellar): unify refresher key resolution for open and save flows Sep 30, 2026
@khanti42
khanti42 force-pushed the feat/confirmation-refresher-keys-alignment branch from 5e987e6 to 47882da Compare October 6, 2026 20:36
@sonarqubecloud

sonarqubecloud Bot commented Oct 6, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

This branch was successfully deployed

1 active (outdated) deployment
default-branch — cb2076e5 Deployed Sep 29, 2026 by khanti42 via Determine whether this PR is a release PR #1351
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants