core/keystore: wrap duplicate-key Import error with ErrKeyExists for Solana and Stellar - #23718
Open
Dev-next-gen wants to merge 1 commit into
Open
Conversation
…Solana and Stellar The boot path in core/cmd/shell_local.go imports ImportedSolKeys and ImportedStellarKeys and skips duplicates with errors.Is(err2, keystore.ErrKeyExists). Both Import methods returned a bare error, so the check was false and a node restarting with an already present key exited. Same fix as smartcontractkit#23267 for Aptos.
product-security-plaid-production
Bot
requested review from
bolekk and
justinkaseman
September 11, 2026 03:44
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
1 participant
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.
I was reading #23267, which fixed Aptos
Importso the boot path incore/cmd/shell_local.gorecognises a duplicate key througherrors.Is(err2, keystore.ErrKeyExists)and continues instead of callings.errorOut. That PR left the other keystores out on the grounds that they are not on the boot path, where onlyEnsureKeyis called. For Solana and Stellar that is not the case:runNodeloops overs.Config.ImportedSolKeys()ands.Config.ImportedStellarKeys(), callsImport, and relies on the exact sameerrors.Is(err2, keystore.ErrKeyExists)check. BothImportmethods still return a barefmt.Errorf("key with ID %s already exists", ...), so the check is false and a node with imported Solana or Stellar keys fails on the second boot, once the key is already in the keyring.The fix is the same one-liner as the Aptos one, applied to
solana.goandstellar.go. The rendered message becomesKey already exists: key with ID <id> already exists, identical in shape to Aptos and DKG recipient. No existing test compares the Solana or Stellar string.To lock it in, I tightened the duplicate
Importassertion inTest_SolanaKeyStore_E2EandTest_StellarKeyStore_E2Efromrequire.Errortorequire.ErrorIs(t, err, keystore.ErrKeyExists). Against a local Postgres 16 prepared withmake testdb, both tests fail without the change (Target error should be in err chain: expected: "Key already exists") and pass with it. The rest of the Aptos, DKG recipient, P2P and Stellar keystore tests still pass. I only exercised the keystore level, not a full node restart with[[Solana.Keys]]secrets. The boot code uses the sameerrors.Ischeck the test now asserts.A few other keystores (Cosmos, StarkNet, Sui, TON, Tron, OCR, OCR2, VRF, CSA, Workflow) have the same bare error. They are not imported at boot, and some of their tests compare the exact string, so I left them out to keep this change minimal.
Requires
None.
Supports
None.
AI tools used