-
Notifications
You must be signed in to change notification settings - Fork 10
Feature/1121 #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/1121 #59
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e882a03a71
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
spoon_toolkits/crypto/evm/signers.py
Outdated
| if _auto_decrypt_to_vault(ENV_PRIVATE_KEY): | ||
| return _get_from_vault(ENV_PRIVATE_KEY) | ||
| value = _get_from_vault(ENV_PRIVATE_KEY) | ||
| if value: | ||
| logger.info(f"Successfully decrypted {ENV_PRIVATE_KEY} and retrieved from vault") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix unexpected indent in _get_private_key_from_vault
The new block under if _auto_decrypt_to_vault(...) is indented two levels deeper than the if, which produces an IndentationError: unexpected indent when this module is imported. That means any code path importing spoon_toolkits.crypto.evm.signers will fail immediately, regardless of runtime inputs. This is a hard failure for all EVM signing usage until the indentation is fixed.
Useful? React with 👍 / 👎.
| else: | ||
| logger.debug("Using plaintext private_key parameter") | ||
| key = private_key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve decrypted private_key parameter
When private_key is encrypted, the code successfully decrypts it into key, but then key = private_key unconditionally overwrites that decrypted value with the original encrypted string. In the encrypted-parameter scenario (with SPOON_MASTER_PWD set), the LocalSigner will still receive an encrypted value, leading to invalid key errors or signing failures. The fix is to only set key = private_key for the plaintext case, not after the decryption branch.
Useful? React with 👍 / 👎.
| logger.info(f"Found encrypted {ENV_PRIVATE_KEY} in environment, will decrypt via vault...") | ||
| key = _get_private_key_from_vault() | ||
| else: | ||
| logger.debug(f"Using plaintext {ENV_PRIVATE_KEY} from environment") | ||
| key = env_key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not overwrite decrypted env key with encrypted value
In the env-key branch, the code calls _get_private_key_from_vault() for encrypted EVM_PRIVATE_KEY but then unconditionally assigns key = env_key afterward. That overwrites the decrypted vault value with the original encrypted string, so local signing still attempts to use the encrypted value and fails. This only happens when EVM_PRIVATE_KEY is encrypted and is a regression from the intended auto-decrypt flow.
Useful? React with 👍 / 👎.
No description provided.