Make Solana device-signer registration provider-aware by tomas-martins-crossmint · Pull Request #104 · Crossmint/crossmint-swift-sdk

tomas-martins-crossmint · 2026-06-11T21:04:00Z

Solana wallet creation included the device signer in the create-wallet request, but device-signer support on Solana depends on the wallet's provider and is only validated server-side. On providers that reject it the backend returns a 400 and the whole wallet creation fails.

Signer registration on Solana and Stellar also returns a pending transaction instead of the per-chain entries EVM returns under chains, which approveIfNeeded didn't handle. A device signer registered through that path would map its key locally but never get approved server-side.

Changes

Wallet: createWalletApiModel no longer attaches a device signer on Solana; the first recover() registers it instead, and a DEVICE_SIGNER_NOT_SUPPORTED rejection wipes the local key, keeps signing on the recovery signer, and stops retrying for that wallet instance
Wallet: added WalletError.deviceSignerNotSupported, mapped from the backend's stable DEVICE_SIGNER_NOT_SUPPORTED code; new public enum case, so exhaustive switches over WalletError need a new case
Wallet: useSigner(.device) throws deviceSignerNotSupported once the provider rejected device signers, and explicit addSigner(.device) surfaces the typed error instead of walletGeneric
Wallet: SignerRegistrationService approves transaction-shaped registrations by fetching the transaction and signing its pending approvals; AddDelegatedSignerResponse gains the transaction field
Http: added NetworkError.serviceErrorCode, exposing the body's stable code field the same way serviceErrorMessage exposes the message
Added WalletDeviceSignerRecoveryTests, DefaultWalletServiceTests, SignerRegistrationTransactionApprovalTests, and a wallet-creation test covering the typed-error contract, the recover fallback, the transaction-shaped approval, and the Solana deferral

…ion approval paths

greptile-apps · 2026-06-11T21:08:57Z

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Sources/Wallet/Model/Wallet/DeviceSignerService.swift:63-79
**Transaction approval is unreachable for Solana device signers**

`approveIfNeeded` is only called when `registration.chains?[chainName]` is non-nil and `awaiting-approval`. For Solana/Stellar, `addSigner` returns `{transaction: {...}}` with a nil `chains`, so this block is never entered — the approval transaction is never submitted, but `storage.mapAddressToKey` still runs on line 82, leaving local state claiming the device signer is registered while the backend never approved it.

This is the exact problem the PR description says the `approveIfNeeded` refactor fixes ("a device signer registered through that path would map its key locally but never get approved server-side"), but `DeviceSignerService.register()` still gates the call on the EVM-style `chains` response. `SignerRegistrationService.register(locator:signer:)` avoids this by calling `approveIfNeeded` unconditionally; the same fix should be applied here.

The test `fetchesAndApprovesPendingApprovalsFromTheRegistrationTransaction` exercises `approveIfNeeded` directly rather than through this call site, so the gap is not caught by the new test suite.

_{Reviews (1): Last reviewed commit: "Replace the static device-signer error h..." | Re-trigger Greptile}

…rService

greptile-apps · 2026-06-11T21:41:32Z

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
Sources/Wallet/Data/CreateWallet/AddDelegatedSignerResponse.swift:15
**Optional `id` silently skips approval and locally persists an unregistered key**

`RegistrationTransaction.id` is `String?`, so if the backend ever returns `{"transaction": {}}` (no `id` field), the `if let transactionId = registration.transaction?.id` guard in `approveIfNeeded` silently no-ops. Execution then falls through to `storage.mapAddressToKey(address:publicKeyBase64:)`, storing the key as registered locally even though the server-side transaction was never approved. The device signer appears valid on-device but will be rejected when the user next tries to sign.

Since a `RegistrationTransaction` without an `id` is presumably a malformed response, making the field non-optional (or adding an explicit guard-with-throw before the map step) would surface the failure instead of producing a silent phantom registration.

### Issue 2 of 2
Sources/Wallet/Model/Wallet/Wallet+SignerManagement.swift:28-50
**`addSigner(.device)` doesn't update `_deviceSignerUnsupported`, leaving the wallet in needs-recovery state**

When a user explicitly calls `addSigner(.device)` on a Solana wallet whose provider rejects device signers, `deviceSignerNotSupported` is correctly surfaced (good), but `_deviceSignerUnsupported` is never set to `true`. If `_needsRecovery` was `true` before the call (the common case for a new Solana wallet), it remains `true` after the error. The next implicit call to `recover()` via `preAuthIfNeeded()` on a transaction will re-attempt registration, receive the same rejection, and only then reach `fallBackToRecoverySigner`.

This means a user who handles the `addSigner(.device)` error and proceeds to send a transaction will see an extra spurious registration attempt before the wallet settles into the unsupported state. Setting `_deviceSignerUnsupported = true` (and `_needsRecovery = false`) in the `addSigner(.device)` error path when the error is `deviceSignerNotSupported` would align the explicit-add path with the recovery path.

_{Reviews (2): Last reviewed commit: "Approve transaction-shaped device-signer..." | Re-trigger Greptile}

afeight · 2026-06-12T18:34:52Z

+            guard let jsonObject = try JSONSerialization.jsonObject(with: errorData, options: []) as? [String: Any],
+                  let code = jsonObject["code"] as? String else {


why use this over JSONDecoder with a defined type?

afeight · 2026-06-12T18:35:15Z

    public let chains: [String: ChainRegistrationEntry]?
+    public let transaction: RegistrationTransaction?
+
+    init(chains: [String: ChainRegistrationEntry]?, transaction: RegistrationTransaction? = nil) {


if this is not public, it's not necessary to define is it?

afeight · 2026-06-12T18:36:31Z

            errorType: WalletError.self
-        )
+        ) { networkError in
+            deviceSignerNotSupportedError(code: networkError.serviceErrorCode,


is this the style? on the next change we put param 1 on the next line when there are params spread over multiple lines

afeight · 2026-06-12T18:38:31Z

+        } catch {
+            try? await storage.deletePendingKey(publicKeyBase64: publicKeyBase64)
+            Logger.smartWallet.error(LogEvents.walletRegisterDeviceSignerError, attributes: ["error": "\(error)"])
+            throw error


this function is now 60 lines long and hard to read. Can we condense with helpers?

afeight · 2026-06-12T21:00:46Z

+            let transactionModel = try await smartWalletService.fetchTransaction(
+                .init(transactionId: transactionId, chainType: chainType)
+            )
+            guard let transaction = transactionModel.toDomain(withService: smartWalletService) else {
+                throw TransactionError.transactionGeneric("Failed to decode signer registration transaction")
+            }


the service should be already doing this .toDomain conversion before it returns the value, that's the whole point--that http-specific models never leak out into further business logic

afeight · 2026-06-12T21:12:59Z

+                let signRequest = try await makeSignRequest(signer: signer, message: approval.message)
+                _ = try await smartWalletService.signTransaction(
+                    .init(transactionId: transactionId, apiRequest: signRequest, chainType: chainType)


and then similarly this-I shouldn't have to create the http object here, I should just provide the params (signer, message) and then that's packaged preflight with smartWalletService.signTransaction

…er rejections

… layer

greptile-apps · 2026-06-12T22:12:11Z

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Sources/Wallet/DefaultCrossmintWallets.swift:183
**Stellar not covered by the same deferral**

The guard `chainType != .solana` defers eager device-signer attachment for Solana, but the PR description notes that Stellar also returns a pending-transaction response instead of per-chain `chains` entries. If Stellar providers can also reject device signers at wallet creation (the same `DEVICE_SIGNER_NOT_SUPPORTED` 400 path), Stellar wallet creation would still fail. `signerRegistrationChain` already treats Solana and Stellar identically (`chainType == .solana || chainType == .stellar ? nil`), suggesting the same deferral was at least considered.

_{Reviews (3): Last reviewed commit: "Split device-signer registration into st..." | Re-trigger Greptile}

tomas-martins-crossmint added 6 commits June 11, 2026 16:37

Make Solana device-signer registration provider-aware

53e829c

Extract the sign-request construction shared by both signer registrat…

b206c8f

…ion approval paths

Remove implementation comments, keeping docs on public APIs only

4755115

Drop the Squads example from the device-signer-unsupported docs

dc01bb5

Fold the Solana eager-attach decision into createWalletApiModel

61b89c9

Replace the static device-signer error helpers with an instance method

992b98c

tomas-martins-crossmint requested a review from afeight June 11, 2026 21:05

tomas-martins-crossmint marked this pull request as ready for review June 11, 2026 21:05

Approve transaction-shaped device-signer registrations in DeviceSigne…

4e105fa

…rService

afeight reviewed Jun 12, 2026

View reviewed changes

tomas-martins-crossmint added 4 commits June 12, 2026 19:07

Require the registration transaction id and remember explicit addSign…

ca11663

…er rejections

Decode the service error body with JSONDecoder

f7eff2d

Move transaction decoding and sign-request packaging into the service…

7b58fc4

… layer

Split device-signer registration into step helpers

d7c0ca7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Solana device-signer registration provider-aware#104

Make Solana device-signer registration provider-aware#104
tomas-martins-crossmint wants to merge 11 commits into
mainfrom
tomas/fix-solana-device-signer-provider-aware

tomas-martins-crossmint commented Jun 11, 2026

Uh oh!

greptile-apps Bot commented Jun 11, 2026

Uh oh!

greptile-apps Bot commented Jun 11, 2026

Uh oh!

afeight Jun 12, 2026

Uh oh!

afeight Jun 12, 2026

Uh oh!

afeight Jun 12, 2026

Uh oh!

afeight Jun 12, 2026

Uh oh!

afeight Jun 12, 2026

Uh oh!

afeight Jun 12, 2026

Uh oh!

greptile-apps Bot commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		guard let jsonObject = try JSONSerialization.jsonObject(with: errorData, options: []) as? [String: Any],
		let code = jsonObject["code"] as? String else {

Conversation

tomas-martins-crossmint commented Jun 11, 2026

Changes

Uh oh!

greptile-apps Bot commented Jun 11, 2026

Uh oh!

greptile-apps Bot commented Jun 11, 2026

Uh oh!

afeight Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

afeight Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

afeight Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

afeight Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

afeight Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

afeight Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants