Cache derived public key and refine Identity exceptions #292
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements caching for derived public keys in the Identity class and refines exception handling by replacing generic RuntimeExceptions with more specific IllegalStateException and IllegalArgumentException types.
- Adds lazy computation and caching for public keys to avoid redundant derivation operations
- Replaces generic RuntimeException with IllegalStateException for system state issues and IllegalArgumentException for invalid inputs
- Introduces comprehensive unit tests covering public key caching behavior and failure scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Identity.java | Implements public key caching field, lazy computation logic, and refined exception handling |
| IdentityTest.java | Adds unit tests for public key caching validation and exception handling scenarios |
|
|
||
| @Test | ||
| public void testSignWithInvalidKeyFails() { | ||
| String invalidPriv = "0000000000000000000000000000000000000000000000000000000000000000"; |
There was a problem hiding this comment.
The same invalid private key string is duplicated between tests. Consider extracting this as a constant to improve maintainability and ensure consistency.
| String invalidPriv = "0000000000000000000000000000000000000000000000000000000000000000"; | |
| String invalidPriv = INVALID_PRIVATE_KEY; | |
| Identity identity = Identity.create(invalidPriv); | |
| Assertions.assertThrows(IllegalStateException.class, identity::getPublicKey); | |
| } | |
| @Test | |
| public void testSignWithInvalidKeyFails() { | |
| String invalidPriv = INVALID_PRIVATE_KEY; |
Copilot uses AI. Check for mistakes.
| cachedPublicKey = new PublicKey(Schnorr.genPubKey(this.getPrivateKey().getRawData())); | ||
| } catch (Exception ex) { | ||
| log.error("Failed to derive public key", ex); | ||
| throw new IllegalStateException("Failed to derive public key", ex); |
There was a problem hiding this comment.
The caching implementation is not thread-safe. Multiple threads could simultaneously check the null condition and perform redundant public key computations. Consider using synchronization or a thread-safe lazy initialization pattern.
Copilot uses AI. Check for mistakes.
Summary
Identityand compute it lazilyIdentitywithIllegalStateException/IllegalArgumentExceptionTesting
mvn -q verify(fails: Could not find a valid Docker environment)Network Access
https://chatgpt.com/codex/tasks/task_b_68991e3003dc833194788df45d96ab50