Merge rust-bitcoin/rust-bitcoin#6871: Remove uses of `Index` on `secp256k1::SecretKey`
What the developer wrote
Authored by Andrew Poelstra
Merge rust-bitcoin/rust-bitcoin#6871: Remove uses of `Index` on `secp256k1::SecretKey`
72d03eb1cec3daf26290fd0913140915d03ce1b2 Remove uses of Index on secp256k1::SecretKey (Mitchell Bagot)
Pull request description:
The Index impl on secp256k1::SecretKey is considered upstream to be something we want to remove. In preparation for that, we should replace relevant uses of that trait with equivalent as_secret_bytes() calls.
Remove uses of the Index trait on the secp256k1::SecretKey.
ACKs for top commit:
apoelstra:
ACK 72d03eb1cec3daf26290fd0913140915d03ce1b2; successfully ran local tests
Tree-SHA512: d9f692403791c3a075a1fd2cc98fe3a62bebf6a7dcc58c8b93fbf62207e576c16fd304d9462156910fbef2ab113ae9d0f3631b9babd4129390445cfdb568b6c7
What changed, and why it matters
This is a small code cleanup change in a Bitcoin cryptography library. It replaces an older way of reading secret key bytes with a newer, more explicit method. There is no direct evidence in the commit that this fixes an active security bug, but it removes reliance on a trait that upstream developers want to eliminate, likely because it is considered a poor API for handling secret data.
Treat as routine maintenance. Reviewers may verify that as_secret_bytes() returns the same 32-byte slice as the previous Index implementation and that no secret-key exposure is introduced. No urgent action required.
Security signals we found
Touches secret key material (SecretKey bytes)
Removes use of Index trait on SecretKey, which upstream secp256k1 considers undesirable
No bounds-checking or validation logic changed
No advisory, CVE, or bug report referenced in commit message
Evidence from the diff
The commit replaces uses of the Index trait on secp256k1::SecretKey with explicit as_secret_bytes()/to_secret_bytes() calls in WIF serialization and BIP32 key derivation/serialization. The change is API-compatible and behaviorally equivalent for current secp256k1 versions, but prepares for upstream removal of Index on SecretKey. No memory-safety or correctness fix is visible in the diff.
Changed components
crypto/src/key.rs WifKey::to_string serializationkey_expression/src/bip32.rs Xpriv hardened child derivationkey_expression/src/bip32.rs Xpriv base58 serializationInspect captured patch +3 / −3
### crypto/src/key.rs
@@ -1172,7 +1172,7 @@ impl WifKey {
let mut ret = [0; 34];
ret[0] = if self.network_kind.is_mainnet() { 128 } else { 239 };
- ret[1..33].copy_from_slice(&self.private_key.as_inner()[..]);
+ ret[1..33].copy_from_slice(&self.private_key.to_secret_bytes()[..]);
let privkey = if self.private_key.compressed() {
ret[33] = 1;
base58::Base58CkString::encode(&ret[..])
### key_expression/src/bip32.rs
@@ -784,7 +784,7 @@ impl Xpriv {
} else {
// Hardened key: use only secret data to prevent public derivation.
engine.input(&[0u8]);
- engine.input(&self.private_key[..]);
+ engine.input(self.private_key.as_secret_bytes());
}
engine.input(&u32::from(child_number).to_be_bytes());
@@ -867,7 +867,7 @@ impl Xpriv {
ret[9..13].copy_from_slice(&u32::from(self.child_number).to_be_bytes());
ret[13..45].copy_from_slice(&self.chain_code[..]);
ret[45] = 0;
- ret[46..78].copy_from_slice(&self.private_key[..]);
+ ret[46..78].copy_from_slice(self.private_key.as_secret_bytes());
ret
}
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.