feat(wallet): encrypt private keys at rest using eth-keystore#45
Open
Aboudjem wants to merge 9 commits intoPolymarket:mainfrom
Open
feat(wallet): encrypt private keys at rest using eth-keystore#45Aboudjem wants to merge 9 commits intoPolymarket:mainfrom
Aboudjem wants to merge 9 commits intoPolymarket:mainfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g keystore Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a user has an existing encrypted keystore and fails password authentication (e.g., 3 wrong attempts), the setup flow previously fell through to setup_wallet() which silently overwrote the keystore. Now we bail with a clear error message instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If save_key_encrypted succeeds but save_wallet_settings fails, the plaintext key would remain permanently alongside the encrypted copy. Now the keystore is deleted on failure so the user can retry cleanly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ting After auto-migrating a plaintext key to encrypted keystore, resolve_key_string was calling load_key_encrypted which performs a redundant scrypt decryption. Since we already have the plaintext key in memory from the config, return it directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…g them resolve_key previously used `if let Ok(Some(config)) = load_config()` which silently ignored I/O and parse errors. Now returns Result so callers can surface config file problems to the user. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| anyhow::bail!( | ||
| "Failed to unlock existing keystore: {e}\n\ | ||
| Run `polymarket wallet export` with the correct password, \ | ||
| or `polymarket wallet delete` to start fresh." |
There was a problem hiding this comment.
Error message references non-existent wallet delete command
Medium Severity
The error message tells users to run polymarket wallet delete to start fresh, but no delete subcommand exists. The actual command is polymarket wallet reset. Users who encounter a keystore unlock failure during setup will be given instructions that don't work.
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.
Summary
POLYMARKET_PASSWORDenv var for CI/automationwallet exportsubcommand to decrypt and display key when needed--private-keyflag → env var → auto-migrate → keystore promptChanges
src/password.rssrc/config.rssrc/auth.rssrc/commands/wallet.rscreate/importuse encryption, newexportsubcommandsrc/commands/setup.rsCargo.tomlalloy/signer-keystore,rpassword,rand,secrecyTest plan
cargo clippy -D warnings— cleancargo fmt --check— cleanFixes #18
Note
Medium Risk
Changes how wallet private keys are stored and loaded (introducing encrypted keystore files, password prompts, and migration), which can break auth flows or lock users out if edge cases aren’t handled correctly.
Overview
Encrypts wallet private keys at rest and adds password-based key management. Wallet creation/import now writes an encrypted
keystore.json(and stores only non-sensitive settings inconfig.json), with new password prompting (includingPOLYMARKET_PASSWORDsupport) and retries.Updates key resolution and CLI workflows. Auth/provider setup now resolves keys via
--private-key/env var, auto-migrates legacy plaintext config keys to the encrypted keystore, or prompts to decrypt the keystore;wallet exportis added to decrypt/print the key when needed, andsetup/walletcommands are adjusted to detect existing keystores and avoid accidental overwrite.Dependency updates add Alloy keystore support plus
secrecy,rpassword, andrand, and tests are extended to cover keystore encrypt/decrypt behavior and CLI help output.Written by Cursor Bugbot for commit 7cc2e4c. This will update automatically on new commits. Configure here.