fix: Use pubkey instead of pubKey in encrypted key JSON

[dknopik]

Issue Addressed

• Allow pubkey and pubKey in encrypted_private_key.json #712

Proposed Changes

• use #[serde(alias = "pubKey")] to allow deserializing old keys while serialising with the new name pubkey
• Add a test to handle both cases

[claude-code-actions-sigp]

Claude finished @dknopik's task —— View job

---

Code Review Complete

Tasks

• Read and analyze changed files
• Review backward compatibility approach
• Verify test coverage
• Check for edge cases and potential issues
• Provide comprehensive feedback

---

Summary

This PR successfully standardizes the encrypted key JSON format to use pubkey (lowercase) instead of pubKey (camelCase) while maintaining backward compatibility. The implementation is clean and follows Rust best practices.

Positive Observations

1. Correct Use of Serde Alias
The change from #[serde(rename = "pubKey")] to #[serde(alias = "pubKey")] at anchor/common/operator_key/src/encrypted.rs:58 is exactly the right approach:

• Serialization now uses the field name pubkey (lowercase)
• Deserialization accepts both pubkey and pubKey for backward compatibility
• This ensures old keys can still be read while new keys follow the standardized format

2. Clear Documentation
The module-level documentation (lines 7-8) clearly explains the backward compatibility approach, which is helpful for future maintainers.

3. Adequate Test Coverage
Two tests verify the implementation:

• test_decrypt_existing() - Tests the new format with pubkey
• test_decrypt_legacy() - Tests backward compatibility with pubKey

4. Scope Verification
Verified that pubKey (camelCase) only appears in the files changed by this PR, confirming the fix is complete.

Detailed Review

Code Quality ✅

• Clean, minimal changes that achieve the stated goal
• Follows Rust idioms and serde best practices
• No unnecessary complexity added

Security ✅

• No security concerns introduced
• The change is purely cosmetic for the JSON field name
• Encryption/decryption logic remains unchanged
• Pubkey validation logic (PubkeyDoesNotMatch error) is still enforced at line 107

Performance ✅

• No performance impact
• Serde alias handling is zero-cost at runtime

Testing ✅

• Both new and legacy formats are tested
• Tests use the actual test key files, ensuring real-world compatibility
• Consider: Could add a test that verifies newly encrypted keys serialize with pubkey (not pubKey)

Documentation ✅

• Module documentation updated correctly
• Comment at line 98 should be updated to reference pubkey instead of pubKey:

/// If the pubkey was provided along the encrypted key in a "pubKey" attribute, it is verified

Should be:

/// If the pubkey was provided along the encrypted key in a "pubkey" attribute (also accepts
/// legacy "pubKey" for backward compatibility), it is verified

Minor Suggestions

1. Comment Update Needed (anchor/common/operator_key/src/encrypted.rs:98)

The doc comment on the decrypt() method still references "pubKey" specifically:

/// If the pubkey was provided along the encrypted key in a "pubKey" attribute, it is verified

This should be updated to reflect that both field names are accepted:

/// If the pubkey was provided along the encrypted key (as "pubkey" or legacy "pubKey"), it is verified

2. Optional: Add Serialization Test

Consider adding a test that explicitly verifies new keys are serialized with pubkey (lowercase):

#[test]
fn test_encrypt_uses_lowercase_pubkey() {
    let key = Rsa::generate(2048).unwrap();
    let password = "test";
    let encrypted = EncryptedKey::encrypt(&key, password).unwrap();
    let json = serde_json::to_string(&encrypted).unwrap();
    assert!(json.contains(r#""pubkey":"#));
    assert!(!json.contains(r#""pubKey":"#));
}

This would document the expected serialization behavior explicitly.

Conclusion

This is a well-implemented fix that correctly addresses issue #712. The use of #[serde(alias)] is the appropriate solution for maintaining backward compatibility while standardizing on the new format. The only recommendation is updating the doc comment to reflect the current behavior.

Recommendation: Approve with the minor doc comment improvement suggested above.

---

[diegomrsantos]

LGTM