Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 4, 2025

The async/await refactor changed deleteKey to throw .invalidPrivateKey and .invalidPrivateKeyEncoding errors instead of silently ignoring them as the original implementation did.

Changes

  • Reverted error handling in LocalKeystore.deleteKey to ignore .invalidPrivateKey and .invalidPrivateKeyEncoding errors
  • Maintains original behavior where wallet deletion succeeds even when these errors occur
// Before: Throws errors, breaks deletion flow
case .invalidPrivateKey, .invalidPrivateKeyEncoding:
    throw error

// After: Silently ignores, matches original behavior
case .unknownWalletInWalletCore, .unknownWalletIdInWalletCore, .unknownWalletInWalletCoreList, .invalidPrivateKey, .invalidPrivateKeyEncoding:
    break

This preserves the original contract where WalletService.delete(_:) completes successfully even in edge cases where private key operations fail during cleanup.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

The original code silently ignored .invalidPrivateKey and
.invalidPrivateKeyEncoding errors when deleting wallets. The async/await
refactor changed this to throw these errors, which could cause wallet
deletion to fail in cases where it previously succeeded. This reverts
the error handling to maintain backward compatibility.

Co-authored-by: gemcoder21 <[email protected]>
Copilot AI changed the title [WIP] WIP address feedback on keystore and wallet operations Restore silent error handling in deleteKey for backward compatibility Nov 4, 2025
Copilot AI requested a review from gemcoder21 November 4, 2025 23:22
@gemcoder21
Copy link
Contributor

@copilot I think we need to return an error here via resume

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.

2 participants