Skip to content

chore(stellar): Client request handler confirmSend now throws when signing, or submission fails after the user confirms - #550

Merged
stanleyyconsensys merged 3 commits into
mainfrom
fix/stellar/align-non-evm-errorhandle-send
Nov 4, 2026
Merged

stanleyyconsensys merged 3 commits into
mainfrom
fix/stellar/align-non-evm-errorhandle-send

Conversation

@stanleyyconsensys

@stanleyyconsensys stanleyyconsensys commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Explanation

This PR to address the comment from extension PR

Client request handler confirmSend now throws when signing, or submission fails after the user confirms

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

}

// Unexpected errors are swallowed into `{ valid: false }`, so track them for debugging.
await trackError(error);

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 dont need track error, as we dont swallow anymore

@stanleyyconsensys stanleyyconsensys changed the title fix(stellar): align non-evm error handle in confirmSend after user confirm txn fix(stellar): Client request handler confirmSend now throws when signing, or submission fails after the user confirms Nov 3, 2026
* @param result - Dialog result from `snap_resolveInterface`.
* @returns Normalized confirmation + optional memo from the UI.
*/
function parseConfirmSendDialogResult(

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.

move to private method

@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@stanleyyconsensys stanleyyconsensys changed the title fix(stellar): Client request handler confirmSend now throws when signing, or submission fails after the user confirms chore(stellar): Client request handler confirmSend now throws when signing, or submission fails after the user confirms Nov 3, 2026
@stanleyyconsensys
stanleyyconsensys requested a balanced review from Copilot November 3, 2026 09:59

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.

🟡 Changes recommended

The changed failure contract still needs corrected documentation, a breaking-change changelog entry, and regression tests for signing and submission failures.

3 open issues
What changed in this PR

Updates Stellar’s confirmSend handler so failures after user confirmation reject the request instead of returning { valid: false, errors }, aligning error handling with the client send flow.

Changes:

  • Propagates errors while preserving user-rejection handling.
  • Updates validation-failure tests to expect thrown exceptions.
  • Revises response documentation and adds a changelog entry.
File Description
packages/​stellar-wallet-snap/​src/​handlers/​clientRequest/​confirmSend.ts Rethrows failures and simplifies dialog-result parsing.
packages/​stellar-wallet-snap/​src/​handlers/​clientRequest/​confirmSend.test.ts Expects rejection for fee, memo, and balance validation failures.
packages/​stellar-wallet-snap/​docs/​use-cases/​client-request/​confirmSend.md Documents the revised failure contract.
packages/​stellar-wallet-snap/​CHANGELOG.md Records the client-facing error-handling change.

Review effort: Balanced


### Changed

- Client request handler `confirmSend` now throws when signing, or submission fails after the user confirms, instead of returning a JSON error response such as `{ valid: false, errors }` ([#550](https://github.com/MetaMask/internal-snaps/pull/550))

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.

it is not a breaking change, becoz client already has this handled

valid: false,
errors: [{ code: MultiChainSendErrorCodes.Invalid }],
};
throw error;

@khanti42 khanti42 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.

LGTM

@stanleyyconsensys
stanleyyconsensys added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit cf0bdc1 Nov 4, 2026
57 checks passed
@stanleyyconsensys
stanleyyconsensys deleted the fix/stellar/align-non-evm-errorhandle-send branch November 4, 2026 02:56
LabinatorSolutions pushed a commit to LabinatorSolutions/metamask-extension that referenced this pull request Nov 7, 2026
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Bumps the Solana, Stellar, and Tron wallet snaps to the versions
released in internal-snaps `22.0.0`:

- `@metamask/solana-wallet-snap`: `^7.2.0` → `^7.3.0`
- `@metamask/stellar-wallet-snap`: `^2.0.0` → `^2.1.0`
- `@metamask/tron-wallet-snap`: `^4.3.1` → `^4.4.0`

All three are backwards-compatible minor updates; no application code
changes are required.

### @metamask/solana-wallet-snap 7.3.0

- Remove the temporary fallback that injected `https://metamask.io` as
the request origin when `keyring_submitRequest` was called without one
([MetaMask#533](MetaMask/internal-snaps#533)).

### @metamask/stellar-wallet-snap 2.1.0

- `confirmSend` now throws when signing or submission fails after the
user confirms, instead of returning a JSON error response
([MetaMask#550](MetaMask/internal-snaps#550)).
- Bump `@stellar/stellar-sdk` from `^17.2.1` to `^17.4.0`
([MetaMask#545](MetaMask/internal-snaps#545)).
- Retry transient Token API, Price API, Security Alerts API, RPC, and
Horizon failures with bounded timeouts and circuit breaking
([MetaMask#534](MetaMask/internal-snaps#534),
[MetaMask#551](MetaMask/internal-snaps#551)).
- Fix `onAmountInput` to accept the optional destination as `toAddress`
instead of `to`
([MetaMask#558](MetaMask/internal-snaps#558)).

### @metamask/tron-wallet-snap 4.4.0

- Support `VoteWitnessContract` and `AccountPermissionUpdateContract`
transactions by upgrading TronWeb to 6.5.1
([MetaMask#561](MetaMask/internal-snaps#561)).
- Align transaction security warning copy and scan-loading behavior with
user preferences
([MetaMask#557](MetaMask/internal-snaps#557)).

## **Changelog**

CHANGELOG entry: null

## **Related issues**

n/a

## **Manual testing steps**

n/a

## **Screenshots/Recordings**

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve included tests if applicable
- [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
anxolin pushed a commit to anxolin/internal-snaps that referenced this pull request Nov 7, 2026
## Explanation

This release branch prepares version `22.0.0`. It contains:

- `@metamask/solana-wallet-snap` `7.2.0` → `7.3.0` (minor: remove
obsolete `https://metamask.io` origin fallback)
- `@metamask/stellar-wallet-snap` `2.0.0` → `2.1.0` (minor: fetch policy
for RPC, Horizon and HTTP clients, `@stellar/stellar-sdk` upgrade,
`onAmountInput` `toAddress` fix, `confirmSend` now throws after
confirmation failures)
- `@metamask/tron-wallet-snap` `4.3.1` → `4.4.0` (minor:
`VoteWitnessContract` and `AccountPermissionUpdateContract` support via
TronWeb 6.5.1, security warning copy and scan-loading alignment)
- Root monorepo version bumped to `22.0.0`

## References

- `@metamask/solana-wallet-snap`:
[MetaMask#533](MetaMask#533)
- `@metamask/stellar-wallet-snap`:
[MetaMask#550](MetaMask#550),
[MetaMask#545](MetaMask#545),
[MetaMask#534](MetaMask#534),
[MetaMask#551](MetaMask#551),
[MetaMask#558](MetaMask#558)
- `@metamask/tron-wallet-snap`:
[MetaMask#561](MetaMask#561),
[MetaMask#557](MetaMask#557)

## Checklist

- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/internal-snaps/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/internal-snaps/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them
weitingsun pushed a commit to Poovarasan-Sandhanam/metamask-mobile that referenced this pull request Nov 10, 2026
…taMask#39134)

<!--
Please submit this PR as a draft initially.

Do not mark it as "Ready for review" until this PR meets the canonical
Definition of Ready For Review in `docs/readme/ready-for-review.md`.
-->

## **Description**

Bumps the Bitcoin, Solana, Stellar, and Tron wallet snaps to their
latest published versions:

- `@metamask/bitcoin-wallet-snap`: `^3.2.0` → `^3.3.0`
- `@metamask/solana-wallet-snap`: `^7.2.0` → `^7.3.0`
- `@metamask/stellar-wallet-snap`: `^2.0.0` → `^2.1.0`
- `@metamask/tron-wallet-snap`: `^4.3.0` → `^4.4.0`

All are backwards-compatible minor updates; no application code changes
are required.

### @metamask/bitcoin-wallet-snap 3.3.0

- Retry transient Bitcoin indexer, price API, and signed transaction
broadcast failures with the shared fetch policy, and treat duplicate
broadcasts as successful
([MetaMask#530](MetaMask/internal-snaps#530)).
- Fix formatting of small Bitcoin balances to avoid scientific notation
([MetaMask#543](MetaMask/internal-snaps#543)).

### @metamask/solana-wallet-snap 7.3.0

- Remove the temporary fallback that injected `https://metamask.io` as
the request origin when `keyring_submitRequest` was called without one
([MetaMask#533](MetaMask/internal-snaps#533)).

### @metamask/stellar-wallet-snap 2.1.0

- `confirmSend` now throws when signing or submission fails after the
user confirms, instead of returning a JSON error response
([MetaMask#550](MetaMask/internal-snaps#550)).
- Bump `@stellar/stellar-sdk` from `^17.2.1` to `^17.4.0`
([MetaMask#545](MetaMask/internal-snaps#545)).
- Retry transient Token API, Price API, Security Alerts API, RPC, and
Horizon failures with bounded timeouts and circuit breaking
([MetaMask#534](MetaMask/internal-snaps#534),
[MetaMask#551](MetaMask/internal-snaps#551)).
- Fix `onAmountInput` to accept the optional destination as `toAddress`
instead of `to`
([MetaMask#558](MetaMask/internal-snaps#558)).

### @metamask/tron-wallet-snap 4.4.0

- Support `VoteWitnessContract` and `AccountPermissionUpdateContract`
transactions by upgrading TronWeb to 6.5.1
([MetaMask#561](MetaMask/internal-snaps#561)).
- Align transaction security warning copy and scan-loading behavior with
user preferences
([MetaMask#557](MetaMask/internal-snaps#557)).

## **Changelog**

CHANGELOG entry: Updated the Solana, Stellar, and Tron wallet snaps to
their latest versions.

## **Related issues**

Fixes: N/A

## **Manual testing steps**

```gherkin
Feature: Bitcoin and non-EVM wallet flows

  Scenario: user views account assets
    Given the user has Bitcoin, Solana, Stellar, and Tron accounts with assets
    When user opens the wallet and switches between those networks
    Then the assets and balances load successfully

  Scenario: user sends funds on each network
    Given the user has funds on Bitcoin, Solana, Stellar, and Tron
    When user sends funds on each network
    Then the fee estimate and confirmation are displayed correctly
    And the transaction is broadcast and completes successfully
```

## **Screenshots/Recordings**

### **Before**

N/A

### **After**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

#### Performance checks (if applicable)

- [x] I've tested on Android
  - Ideally on a mid-range device; emulator is acceptable
- [x] I've tested with a power user scenario
- Use these [power-user
SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93)
to import wallets with many accounts and tokens
- [x] I've instrumented key operations with Sentry traces for production
performance metrics
- See [`trace()`](/app/util/trace.ts) for usage and
[`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274)
for an example

For performance guidelines and tooling, see the [Performance
Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers).

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
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.

3 participants