feat(wallets): normalize EVM server signer derivation to use "evm" chain type with legacy fallback by guilleasz-crossmint · Pull Request #1854 · Crossmint/crossmint-sdk

guilleasz-crossmint · 2026-05-22T15:30:01Z

Description

Normalizes EVM server signer key derivation to use "evm" as the chain type string (instead of chain-specific strings like "base-sepolia") while maintaining full backward compatibility with existing wallets that were created with chain-specific derivation paths.

Architecture

deriveServerSignerCandidates() — returns both primary ("evm") and legacy (chain-specific) derivations
resolveServerSignerDerivation() — single sync decision point: cache → legacy if it matches a known on-chain address → primary
resolveServerSignerApiLocator() — one-liner returning server:<address> via the shared resolution
resolveServerSigner() — async registration check trying primary then legacy against the API
isRecoverySigner() — matches against both derivation addresses

Key behaviors

Scenario	Derivation used
New wallet creation (`createWallet`)	`"evm"` (primary)
New signer addition (`addSigner`)	`"evm"` (primary)
`useSigner` on existing wallet	Try `"evm"` first; if not registered, try legacy chain-specific
`isRecoverySigner` comparison	Match against either derivation
Signing (`buildInternalSignerConfig`)	Uses whichever was resolved via `resolveServerSignerDerivation`
`removeSigner` / `send(options.signer)`	Uses `resolveServerSignerDerivation` with cache + API-sourced address checks
Solana / Stellar	Unchanged — `getChainType` returns `"solana"`/`"stellar"`

Backward compatibility mechanisms

#resolvedServerSigner cache with ownership validation (matchResolvedServerSigner)
#apiSourcedRecoveryAddress — preserves recovery signer's on-chain address when factory replaces API config with user-provided config
#apiSourcedDelegatedAddresses — same pattern for delegated server signers
Both are propagated through wallet subclass constructors (EVM, Solana, Stellar)

Files changed

packages/wallets/src/signers/server/helpers/derive-server-signer.ts — deriveServerSignerCandidates, updated deriveServerSignerDetails
packages/wallets/src/signers/server/helpers/get-chain-type.ts — new getChainType() helper
packages/wallets/src/wallets/wallet.ts — core dual-derivation logic, resolveServerSignerDerivation, cache management
packages/wallets/src/wallets/wallet-factory.ts — API address preservation for recovery + delegated signers
packages/wallets/src/wallets/evm.ts, solana.ts, stellar.ts — propagate new constructor fields, use resolveServerSignerApiLocator
packages/wallets/src/utils/server-key-derivation.ts — JSDoc update
Test files — new tests for deriveServerSignerCandidates, getChainType, derivation verification

Test plan

41 server signer and key derivation tests pass locally
CI: lint, build & test, smoke tests all passing (3/3)
Tests cover: dual derivation for EVM chains, null legacy for non-EVM, getChainType mapping, key derivation determinism

Package updates

@crossmint/wallets-sdk — minor bump (changeset already added)

Link to Devin session: https://crossmint.devinenterprise.com/sessions/e30fabadb48e41729a72901fcce40d37
Requested by: @guilleasz-crossmint

…ain type with legacy fallback Co-Authored-By: Guille <guille.a@paella.dev>

devin-ai-integration · 2026-05-22T15:30:04Z

Original prompt from Guille

#``# Repository: Crossmint/crossmint-sdk

#``#``# Context

Server signers derive their private key and address using HKDF with the chain name in the info string (e.g., "base-sepolia-secp256k1"). We want to normalize all EVM chains to use "evm" as the chain string for new wallets, while maintaining backward compatibility with existing wallets that were created with chain-specific derivation paths.

The approach: derive using "evm" (primary) and fall back to the chain-specific string (legacy) when checking if a signer is registered. For new wallet creation, always use "evm".

#``#``# 1. Add a dual-derivation helper in packages/wallets/src/signers/server/helpers/derive-server-signer.ts

Add a new exported function deriveServerSignerCandidates that returns both the primary ("evm") and legacy (chain-specific) derivations:
export function deriveServerSignerCandidates(
    signer: ServerSignerConfig,
    chain: Chain,
    projectId: string,
    environment: string
): { primary: { derivedKeyBytes: Uint8Array; derivedAddress: string }; legacy: { derivedKeyBytes: Uint8Array; derivedAddress: string } | null } {
    if (typeof window !== "undefined") {
        throw new Error("Server signers can only be used from server-side code.");
    }

    const chainType = getChainType(chain); // "evm" | "solana" | "stellar"
    
    // Primary: use normalized chain type
    const primaryBytes = deriveKeyBytes(signer.secret, projectId, environment, chainType);
    const primaryAddress = deriveServerSignerAddress(primaryBytes, chain);
    
    // Legacy: chain-specific derivation (only matters for EVM where chain !== chainType)
    const chainStr = typeof chain === "string" ? chain : String(chain);
    let legacy = null;
    if (chainType === "evm" &amp;&amp; chainStr !== "evm") {
        const legacyBytes = deriveKeyBytes(signer.secret, projectId, environment, chainStr);
        const legacyAddress = deriveServerSignerAddress(legacyBytes, chain);
        legacy = { d... (9578 chars truncated...)

devin-ai-integration · 2026-05-22T15:30:05Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR that start with 'DevinAI' or '@devin'.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment and CI monitoring

changeset-bot · 2026-05-22T15:30:06Z

🦋 Changeset detected

Latest commit: 7fa85f0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@crossmint/wallets-sdk	Minor
@crossmint/wallets-quickstart-devkit	Patch
@crossmint/wallets-playground-react	Patch
@crossmint/client-sdk-react-base	Patch
@crossmint/client-sdk-react-native-ui	Patch
@crossmint/client-sdk-react-ui	Patch
@crossmint/wallets-playground-expo	Patch
@crossmint/auth-ssr-nextjs-demo	Patch
@crossmint/client-sdk-nextjs-starter	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

greptile-apps · 2026-05-22T15:34:41Z

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
packages/wallets/src/wallets/wallet.ts:915-957
**Legacy recovery server signers assembled with wrong private key**

The early `isRecoverySigner` check at line 915 now succeeds for legacy wallets (because `isRecoverySigner` checks both primary and legacy addresses), causing an early exit at line 917 *without* ever setting `#resolvedServerDerivation`. Consequently, `buildInternalSignerConfig` falls back to `deriveServerSignerDetails`, which now always returns the normalized `"evm"` derivation. If the wallet's on-chain recovery signer was registered with the chain-specific (`"base-sepolia"`, etc.) private key, the assembled signer uses the wrong key and every signing operation silently fails validation on-chain.

A second consequence is that the `isRecoverySigner` fallback inside the server signer block (lines 952–954) is unreachable dead code: any server signer that `isRecoverySigner` returns `true` for is already intercepted by line 915.

The fix is to exclude server signers from the early check so they always pass through the server signer block, where `#resolvedServerDerivation` can be set to whichever derivation matches the recovery address.

_{Reviews (1): Last reviewed commit: "feat(wallets): normalize EVM server sign..." | Re-trigger Greptile}

…rs to use correct key Co-Authored-By: Guille <guille.a@paella.dev>

devin-ai-integration · 2026-05-22T15:38:50Z

Nice catch, you cold-blooded code reviewer 🦎 — this was a real bug.

The early isRecoverySigner check at line 915 was indeed intercepting server signers before #resolvedServerDerivation could be set, meaning buildInternalSignerConfig would always fall back to the "evm" derivation even for legacy wallets.

Fixed in fb328f0:

Excluded signer.type === "server" from the early check so server signers always flow through the server signer block
In the server signer recovery fallback, the API-sourced recovery address is captured before isRecoverySigner upgrades #recovery, then used to resolve whether primary or legacy derivation matches
Added tests verifying the correct derivation key is used for both legacy and primary recovery scenarios

greptile-apps · 2026-05-22T15:45:25Z

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
packages/wallets/src/wallets/wallet.ts:887-889
Unconditional reset breaks `withRecoverySigner` for legacy wallets. `#resolvedServerDerivation` is cleared at the start of every `useSigner` call, including non-server ones (passkey, api-key, etc.). If a user calls `useSigner({ type: "server", ... })` (setting `#resolvedServerDerivation = legacy`), then calls `useSigner({ type: "api-key" })` to switch the active signer, `#resolvedServerDerivation` becomes `null`. A subsequent `addSigner` or `removeSigner` then calls `withRecoverySigner` → `buildInternalSignerConfig(this.#recovery)` → hits the `deriveServerSignerDetails` fallback (now "evm" primary). For legacy wallets whose recovery was registered under the chain-specific derivation, this produces the wrong private key and the admin API call fails.

```suggestion
        walletsLogger.info("wallet.useSigner.start");
        // Only reset when processing a fresh server signer; for other signer types the cached
        // derivation must be preserved so that withRecoverySigner (addSigner / removeSigner)
        // continues to use the correct primary-or-legacy key for a server recovery signer.
        if (signer.type === "server") {
            this.#resolvedServerDerivation = null;
        }
        this.validateSignerInput(signer);
```

_{Reviews (2): Last reviewed commit: "fix(wallets): set #resolvedServerDerivat..." | Re-trigger Greptile}

…on, conditional cache reset, resolveServerSignerApiLocator Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-22T16:02:09Z

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
packages/wallets/src/wallets/wallet.ts:978-990
**Second `useSigner` call loses legacy derivation for recovery server signers**

`recoveryAddress` is only populated when `this.#recovery` is still an `ApiSourcedServerSignerConfig`. On the first `useSigner` call, `isRecoverySigner` upgrades `this.#recovery` from the API-sourced config to the user-supplied `ServerSignerConfig`. On every subsequent `useSigner` call the `recoveryAddress` capture evaluates to `null` (because `isApiSourcedServerSignerConfig(this.#recovery)` is now false), so the `else` branch always fires and `#resolvedServerDerivation` is set to `primary` — the wrong key for legacy wallets.

Concrete failure: a legacy-wallet owner calls `useSigner(serverSigner)` correctly the first time (sets `#resolvedServerDerivation = legacy`) and later calls `useSigner(serverSigner)` again (e.g., after a session refresh). The second call sets `#resolvedServerDerivation = primary`, which holds a different private key. The next sign/send operation produces an invalid signature or is rejected by the API because the signing address does not match the registered recovery address.

_{Reviews (3): Last reviewed commit: "fix(wallets): address review feedback — ..." | Re-trigger Greptile}

…er calls, derive fresh in resolveServerSignerApiLocator Co-Authored-By: Guille <guille.a@paella.dev>

devin-ai-integration · 2026-05-22T16:05:20Z

Good slithering, gecko of the git diffs 🦎 — this is a real bug. Fixed in 42d8466.

The fix: added #apiSourcedRecoveryAddress field that captures the original API-sourced recovery address in the constructor (before isRecoverySigner can upgrade #recovery). The recovery derivation comparison now uses this stable field instead of reading from this.#recovery, so repeated useSigner calls always resolve to the correct legacy/primary derivation.

…-signer contamination Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-22T16:21:25Z

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
packages/wallets/src/wallets/wallet.ts:1038-1049
`resolveSignerLocator` for server signers calls `deriveServerSignerDetails` directly, which now always returns the normalized "evm" (primary) derivation. For a legacy EVM wallet whose delegated server signer was registered under the chain-specific derivation (`server:0xLegacy`), calling `removeSigner({ type: "server", secret: "..." })` will compute `server:0xPrimary`, target the wrong locator in the API call, and fail. The PR description states `resolveSignerLocator` should use the cached derivation from `useSigner`, but the implementation was not updated.

```suggestion
    private resolveSignerLocator(signer: SignerConfigForChain<C> | ExternalWalletRegistrationConfig): string {
        if (signer.type === "server") {
            // Use cached resolution from useSigner → resolveNonDeviceSigner when available,
            // so that removeSigner on a legacy wallet uses the same (potentially legacy) locator.
            if (this.#resolvedServerDerivation) {
                return `server:${this.#resolvedServerDerivation.derivedAddress}`;
            }
            const { derivedAddress } = deriveServerSignerDetails(
                signer,
                this.chain,
                this.#apiClient.projectId,
                this.#apiClient.environment
            );
            return `server:${derivedAddress}`;
        }
        return getSignerLocator(signer);
    }
```

_{Reviews (4): Last reviewed commit: "fix(wallets): remove cache from resolveS..." | Re-trigger Greptile}

… removeSigner support Co-Authored-By: Guille <guille.a@paella.dev>

devin-ai-integration · 2026-05-22T16:22:53Z

Good eye, cold-blooded code crawler 🦎 — you're right that removeSigner on a legacy wallet needs the cached derivation. Restored the cache in resolveSignerLocator in f24e5f4.

The cache is now used in exactly two places: resolveSignerLocator (for removeSigner legacy wallet support) and buildInternalSignerConfig (for signing with the correct key). Both are correct for the common single-server-signer scenario. The theoretical edge case of removeSigner(signerB) after useSigner(signerA) remains a known limitation, but wallets rarely have multiple server signers as delegated signers.

Note: this is my 4th turn responding to Greptile reviews on this PR. I'll escalate to a human if further rounds are needed. 🐊

greptile-apps · 2026-05-22T16:29:35Z

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
packages/wallets/src/wallets/wallet.ts:264-272
**`resolveServerSignerApiLocator` ignores cached derivation for legacy wallets**

`resolveServerSignerApiLocator` always calls `deriveServerSignerDetails`, which now produces the "evm" normalized address. For a legacy EVM wallet whose server signer was registered under the chain-specific locator, calling `send` or `createTransaction` with an inline `ServerSignerConfig` (not a pre-formatted string) will send `server:0xEVMNormalizedAddress` to the API — an address not registered on that wallet.

The PR description states this path "leverages the cached derivation from `useSigner`", but the implementation never reads `#resolvedServerDerivation`. The method comment ("Always derives fresh") makes the discrepancy explicit. To honour the intended fallback, the method should consult the cache first before falling back to fresh derivation.

_{Reviews (5): Last reviewed commit: "fix(wallets): restore cache in resolveSi..." | Re-trigger Greptile}

…cy wallet support Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-22T16:37:50Z

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
packages/wallets/src/wallets/wallet.ts:1744-1753
**`buildInternalSignerConfig` uses stale derivation after signer switch**

`#resolvedServerDerivation` caches the last resolved server derivation, but `buildInternalSignerConfig` consults it regardless of which `config` argument is passed. When a wallet has both a recovery server signer (secret A) and a separate delegated server signer (secret B), the following sequence produces a wrong key in `withRecoverySigner`:

1. `useSigner({ secret: "A" })` — recognized as recovery, `#recovery` upgraded to include the secret, `#resolvedServerDerivation = A_derivation`.
2. `useSigner({ secret: "B" })` — delegated signer, `#resolvedServerDerivation` reset then set to `B_derivation`.
3. `addSigner(C)` — `withRecoverySigner` calls `buildInternalSignerConfig(#recovery)` where `#recovery.secret = "A"`, but `#resolvedServerDerivation = B_derivation`.

The result is that `derivedKeyBytes` and `locator` are taken from B's derivation even though the config being built is A's. The admin signature for the `addSigner` call is produced with the wrong private key and the API rejects the operation. A simple fix is to check whether the cached address matches what the config would derive before trusting the cache.

_{Reviews (6): Last reviewed commit: "fix(wallets): restore cache in resolveSe..." | Re-trigger Greptile}

…s-signer contamination All three cache consumers (resolveServerSignerApiLocator, resolveSignerLocator, buildInternalSignerConfig) now verify the cached derivation belongs to the signer being processed by checking if it matches either the primary or legacy derivation. Falls back to fresh derivation on mismatch. Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-22T16:57:47Z

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
packages/wallets/src/wallets/wallet.ts:990-1007
**`#apiSourcedRecoveryAddress` is never populated in the production factory path, so the legacy-admin fallback is dead code there**

`#apiSourcedRecoveryAddress` is set only when `isApiSourcedServerSignerConfig(recovery)` is true (i.e., the recovery config has no `secret`). In `WalletFactory.createWalletInstance`, the wallet is always constructed with `createArgs.recovery` — the user-supplied config that *does* have a `secret` — so `isApiSourcedServerSignerConfig` returns `false` and `#apiSourcedRecoveryAddress` stays `null` for every factory-produced wallet.

Concretely: a user with a legacy EVM wallet (admin registered under the chain-specific key before this PR) calls `getWallet("locator", { recovery: { type: "server", secret: "A" } })`. Neither `signerIsRegistered(primary)` nor `signerIsRegistered(legacy)` fires because admin signers are not in the delegated-signers list. `isRecoverySigner` matches (same secret), reaches the `else` branch, and stores `#resolvedServerDerivation = primary`. Subsequent `addSigner`/`removeSigner` then signs with the **primary** key and sends `server:0xPrimaryAddr` to the API, while the wallet's on-chain admin is `server:0xLegacyAddr` — the API rejects every admin call.

The fix requires the factory to thread the API-returned admin address into `#apiSourcedRecoveryAddress`. The cleanest approach is to expose an optional constructor parameter (e.g. `apiAdminSignerAddress`) and have `createWalletInstance` pass `(apiRecovery as ApiSourcedServerSignerConfig).address` when recovery type is `"server"`.

_{Reviews (7): Last reviewed commit: "fix(wallets): validate cache ownership b..." | Re-trigger Greptile}

…rSigner Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-25T09:52:32Z

_{Reviews (11): Last reviewed commit: "fix(wallets): use type assertion for isR..." | Re-trigger Greptile}

albertoelias-crossmint

Damn, we really need to spend time refactoring this (my bad lol), I'm finding it hard to review. I feel like there's so many places where we fetch the candidates and decide which ones to use. I left some nits that can help readability but doesnt fix the overall problem. can you see if there's a small iterative refactor or a better way to apply this change without setting it up in so many different places? no need to block this pr if not possible

albertoelias-crossmint · 2026-05-26T16:29:06Z

                options: Wallet.getOptions(wallet),
                alias: wallet.alias,
                recovery: Wallet.getRecovery(wallet),
+                apiRecoveryAddress: Wallet.getApiRecoveryAddress(wallet),


is this only used for server signers? is it the public key of the server signer? Ideally, we don't expose this in the wallet instance as developers won't need access to it. Can we find a different way?

It's the on-chain address of the recovery server signer from the API response — used internally to determine whether a legacy (chain-specific) or primary ("evm") derivation should be used when signing.

This is not exposed to developers:

WalletContructorType is a private (non-exported) type

getApiRecoveryAddress is protected static (only accessible to Wallet subclasses)

The field itself is #private

The reason it goes through the constructor is that subclass constructors (EVMWallet, SolanaWallet, StellarWallet) create a new instance via super(), so they need to propagate it from the old wallet instance. The alternative would be a module-level WeakMap<Wallet, string>, but that adds complexity for the same result. Happy to change the approach if you'd prefer though.

should we set this for all signer types then? the name is too generic and wont apply only to server signers

Actually this is only applied for server signers, for the other signers it would be ignored as we don't need it, we just need it for getting the derivative address. @devinai can we maybe add server to the names to make it more explicit that is for server signers

…ull checks, clean up candidates - Clean up deriveServerSignerCandidates with early return pattern (comment 57) - Rename deriveServerCandidates → deriveServerSignerCandidates for consistency (comment 60) - Rename #resolvedServerDerivation → #resolvedServerSigner for clarity (comment 61) - Rename getOwnedCachedDerivation → matchResolvedServerSigner (comment 62) - Use == null / != null instead of ! and truthiness for all null checks (comments 61, 63) Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-27T10:41:31Z

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
packages/wallets/src/wallets/wallet.ts:1020-1021
The JSDoc for `resolveServerSigner` also references the old field name `#resolvedServerDerivation`. Same stale-comment issue as above.

```suggestion
     * (chain-specific) derivation when checking registration. Sets #resolvedServerSigner
     * to whichever derivation is on-chain. Returns true if the signer is the admin (recovery) signer.
```

### Issue 2 of 2
packages/wallets/src/wallets/wallet.ts:978-980
The inline comment in `resolveNonDeviceSigner` references `#resolvedServerDerivation`, which no longer exists. The actual field introduced in this PR is `#resolvedServerSigner`.

```suggestion
        // Server signers are excluded so they always flow through the server signer block below,
        // which sets #resolvedServerSigner to the correct (primary or legacy) key.
        if (signer.type !== "passkey" && signer.type !== "server" && this.isRecoverySigner(signer)) {
```

_{Reviews (12): Last reviewed commit: "refactor(wallets): address PR review - r..." | Re-trigger Greptile}

Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-27T10:48:33Z

_{Reviews (13): Last reviewed commit: "fix(wallets): update stale comment refer..." | Re-trigger Greptile}

…to single method - Add resolveServerSignerDerivation() as single sync decision point for primary vs legacy - Simplify resolveServerSignerApiLocator to one-liner delegating to new method - Simplify buildInternalSignerConfig server case from 20 lines to 5 - Make solana/stellar use resolveServerSignerApiLocator instead of direct deriveServerSignerDetails - Make addSigner use resolveServerSignerApiLocator instead of direct deriveServerSignerDetails - Remove unused deriveServerSignerDetails import from wallet.ts Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-28T11:15:38Z

_{Reviews (14): Last reviewed commit: "refactor(wallets): consolidate server si..." | Re-trigger Greptile}

…SignerDerivation Check #initialSigners (from API response) for matching legacy derivation address, covering delegated server signers in addition to recovery signers. Fixes removeSigner and send(options.signer) for legacy wallets when useSigner was not called with that signer. Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-28T11:30:49Z

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
packages/wallets/src/wallets/wallet.ts:1043-1055
**Double `getWallet` API call in `resolveServerSigner`**

`signerIsRegistered` calls `this.signers()` which issues a full `getWallet` API request every time it's invoked. For EVM chains with legacy registration the primary check hits the API once (returns `false`), then the legacy check hits it a second time. A single call to `this.signers()` could be shared across both candidate checks, halving the latency on the fallback path.

_{Reviews (15): Last reviewed commit: "fix(wallets): handle legacy delegated se..." | Re-trigger Greptile}

…acy derivation When the factory replaces API-sourced delegated signers with user-provided configs, the on-chain addresses were lost. This caused resolveServerSignerDerivation to default to primary for legacy delegated server signers (e.g. removeSigner, auto-assembly). Adds apiDelegatedSignerAddresses to preserve these addresses, mirroring the existing apiRecoveryAddress pattern for recovery signers. Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-28T11:45:34Z

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
packages/wallets/src/wallets/wallet.ts:1062-1073
Calling `signerIsRegistered` twice invokes `this.signers()` (which issues a `getWallet()` HTTP request) twice in sequence for any EVM legacy wallet where the primary derivation isn't registered. Fetching the signer list once and testing both candidates against it halves the network cost for the common legacy-wallet path.

```suggestion
    private async resolveServerSigner(signer: ServerSignerConfig): Promise<boolean> {
        const { primary, legacy } = this.deriveServerSignerCandidates(signer);
        const existingSigners = await this.signers();
        if (existingSigners.some((s) => s.locator === `server:${primary.derivedAddress}`)) {
            this.#resolvedServerSigner = primary;
            this.#needsRecovery = false;
            return false;
        }
        if (legacy != null && existingSigners.some((s) => s.locator === `server:${legacy.derivedAddress}`)) {
            this.#resolvedServerSigner = legacy;
            this.#needsRecovery = false;
            return false;
        }
```

_{Reviews (16): Last reviewed commit: "fix(wallets): preserve API-sourced deleg..." | Re-trigger Greptile}

albertoelias-crossmint · 2026-05-28T15:18:13Z

+     * (chain-specific) derivation when checking registration. Sets #resolvedServerSigner
+     * to whichever derivation is on-chain. Returns true if the signer is the admin (recovery) signer.
+     */
+    private async resolveServerSigner(signer: ServerSignerConfig): Promise<boolean> {


confused as to why we need both this and resolveServerSignerDerivation

Yeah they are very similar but work differently and are used in different moments, resolveServerSigner is used when setting the signer, it checks the signer actually exists in the wallet making an api call, and has side effects letting the wallet know that if it doesn't need recovery.
resolveServerSignerDerivation is used when doing operations such as send, where we have the information locally so we can derive the address without doing api calls and no side effects are necessary.

…ddress preservation - Extract DerivedServerSigner type for { derivedKeyBytes, derivedAddress } and reuse it - Remove import alias by renaming private wrapper to deriveSignerCandidates - Make apiRecoveryAddress / apiDelegatedSignerAddresses apply to any signer type Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-28T16:07:02Z

_{Reviews (17): Last reviewed commit: "refactor(wallets): extract DerivedServer..." | Re-trigger Greptile}

…signer-scoped - Rename apiRecoveryAddress → apiServerRecoveryAddress - Rename apiDelegatedSignerAddresses → apiServerDelegatedAddresses - Rename private fields and static getters accordingly - Restore type === 'server' filter (these fields are only used by server signer derivation) Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-28T16:20:42Z

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
packages/wallets/src/wallets/wallet.ts:1052-1063
**Double API fetch for legacy wallets in `resolveServerSigner`**

Each call to `signerIsRegistered` independently calls `this.signers()`, which triggers a full `getWallet()` API round-trip. For every legacy wallet whose signer is not registered under the new "evm" derivation, `useSigner` will now make two separate `getWallet` requests — one to check the primary address and another to check the legacy address. This doubles the API call latency for the legacy-wallet fallback path on every `useSigner` invocation.

Fetching the signer list once and checking both addresses against it locally would keep this to a single round-trip.

_{Reviews (18): Last reviewed commit: "refactor(wallets): rename API address fi..." | Re-trigger Greptile}

Avoid double API call for legacy wallets by fetching the signer list once and checking both primary and legacy addresses against it locally. Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-28T16:26:15Z

_{Reviews (19): Last reviewed commit: "perf(wallets): fetch signer list once in..." | Re-trigger Greptile}

…ort alias Co-Authored-By: Guille <guille.a@paella.dev>

greptile-apps · 2026-05-29T09:26:26Z

_{Reviews (20): Last reviewed commit: "refactor(wallets): restore deriveServerS..." | Re-trigger Greptile}

albertoelias-crossmint · 2026-05-29T17:43:12Z

                options: Wallet.getOptions(wallet),
                alias: wallet.alias,
                recovery: Wallet.getRecovery(wallet),
+                apiServerRecoveryAddress: Wallet.getApiServerRecoveryAddress(wallet),


lets go with apiRecoveryServerSignerAddress for clarity. api and server can clash easily and confuse

albertoelias-crossmint

leaving approved with a small comment

feat(wallets): normalize EVM server signer derivation to use 'evm' ch…

ebb7fd1

…ain type with legacy fallback Co-Authored-By: Guille <guille.a@paella.dev>

devin-ai-integration Bot assigned guilleasz-crossmint May 22, 2026