feat(adapters): reject outdated Ledger firmware before signing - #837
Open
woahwhattheheck wants to merge 2 commits into
Open
feat(adapters): reject outdated Ledger firmware before signing#837woahwhattheheck wants to merge 2 commits into
woahwhattheheck wants to merge 2 commits into
Conversation
The adapter signed without checking the device Stellar app version, so an outdated build failed part-way through with an opaque device error. The version is now read via getAppConfiguration() before the signing request and compared against MIN_LEDGER_FIRMWARE, rejecting with LedgerFirmwareTooOldError naming both the version found and the version required. Also fixes a pre-existing typecheck error in this file: WalletAdapter was imported from ../types.js, the rich wallet interface requiring name/connect/ disconnect/onAccountChange, rather than the two-method adapter interface in ./types.js that the sibling adapters use.
This was referenced Sep 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #775.
LedgerAdaptersigned without checking the device's Stellar app version, soan outdated build failed part-way through signing with an opaque device error
rather than a message saying what to do about it.
Changes
src/adapters/ledger.ts:MIN_LEDGER_FIRMWARE— exported,"3.0.0".LedgerFirmwareTooOldError— carriesfoundVersionandrequiredVersion, and names both in the message.getAppConfiguration()before the signingrequest is sent, so an unsupported device is rejected up front.
skipFirmwareCheckconstructor flag to bypass the check.compareLedgerVersions— exported, so callers can gate their own UI onthe same comparison.
Constructor stays backward compatible
The adapter previously took a bare path string. It now accepts
string | LedgerAdapterOptions, sonew LedgerAdapter("44'/148'/0'")continues to work unchanged; the options form adds
skipFirmwareCheckandlets the transport and app bindings be injected, which is what makes the gate
testable without a physical device.
Two details
"3.10.0"is newer than"3.9.0", which a string comparison gets backwards. Missing trailingcomponents count as zero, so
"3.0","3"and"3.0.0"all compare equal.unsupported rather than waved through. Signing on a device whose
compatibility could not be established is exactly the outcome this check
exists to prevent, so an unrecognised value rejects with the same error,
quoting what the device reported.
getAddress()is deliberately not gated — it does not sign.Also fixes a pre-existing typecheck error in this file
ledger.tsimportedWalletAdapterfrom../types.js— the rich walletinterface requiring
name,connect,disconnectandonAccountChange—while the sibling adapters import the two-method adapter interface from
./types.js. That produced a standingClass 'LedgerAdapter' incorrectly implements interface 'WalletAdapter'error. Corrected to./types.js, whichmatches
walletconnect.tsand what the class actually implements.Validation
npx vitest run test/ledger.firmware.test.ts— 20/20 passing. Coversnewer/equal/older firmware, the signing request not being sent when the gate
rejects, both versions named on the error, the unparseable-version path, the
transport still being closed when the gate rejects,
skipFirmwareCheck,getAddress()staying ungated, the bare-string constructor, the defaultderivation path, and the comparator itself (numeric ordering, zero-padded
components, unparseable inputs).
npx tsc --noEmitreports 209 errors on this branch against 210 onunmodified
main— one fewer, andsrc/adapters/ledger.tsnow reportsnone. The remaining 209 are pre-existing and untouched by this change.
Two notes on the version comparison
A pre-release version is rejected.
parseVersionrequires every dot-separatedcomponent to be digits, so
"3.0.0-rc1"returnsnulland the gate treats it asunsupported. That is deliberate — the check exists to refuse a device whose
compatibility cannot be established, and an unrecognised string is exactly that
case. Flagging it explicitly because if Ledger ever ships RC builds to users it
becomes a policy question rather than a parsing one, and
skipFirmwareCheckisthe intended escape hatch meanwhile.
The comparison logic is duplicated with #838. That PR adds an equivalent
parseVersion+ compare pair for the xBull extension insrc/wallets/adapters/XBullAdapter.ts. I kept them separate so neither PRdepends on the other landing, but if both merge the SDK carries two copies of
the same logic that can drift. Happy to follow up with a shared helper in
src/utils/once the second one lands, or to fold it into whichever of the twoyou take first if you would rather not merge the duplication at all.