Repository navigation
refactor: key wallet clippy warnings #123
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
Changes from 4 commits
ef52ce3
6f3a964
05ac46b
1a4fb90
6870686
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ exclude = ["tests", "contrib"] | |
| default = ["std"] | ||
| std = ["alloc", "internals/std"] | ||
| alloc = ["internals/alloc"] | ||
| schemars = ["actual-schemars", "dyn-clone"] | ||
| schemars = ["actual-schemars"] | ||
| serde-std = ["serde/std"] | ||
| x11 = ["rs-x11-hash"] | ||
|
|
||
|
|
@@ -31,10 +31,7 @@ core2 = { version = "0.4.0", default-features = false, optional = true } | |
| # Only enable this if you explicitly do not want to use "std", otherwise enable "serde-std". | ||
| serde = { version = "1.0.219", default-features = false, optional = true } | ||
| # Do NOT use this as a feature! Use the `schemars` feature instead. Can only be used with "std" enabled. | ||
| actual-schemars = { package = "schemars", version = "<=0.8.3", optional = true } | ||
| # Do NOT enable this dependency, this is just to pin dyn-clone (transitive dep from schemars) | ||
| # because 1.0.8 does not build with Rust 1.41.1 (because of useage of `Arc::as_ptr`). | ||
| dyn-clone = { version = "<=1.0.7", default-features = false, optional = true } | ||
| actual-schemars = { package = "schemars", version = "1.0", optional = true } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainCrate alias changed to Code under Run to verify any remaining 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Cargo alias:"
rg -nP '^\s*actual-schemars\s*=' -n --glob 'hashes/Cargo.toml' -N
echo -e "\nRust imports using schemars:: :"
rg -nP --type=rust -C2 '\buse\s+schemars::' | sed -n '1,120p'
echo -e "\nRe-export present?"
rg -nP --type=rust -C2 'pub\s+use\s+actual_schemars\s+as\s+schemars;'Length of output: 788 Align 🤖 Prompt for AI Agents |
||
| secp256k1 = { default-features = false, features = ["hashes"], version= "0.30.0" } | ||
|
|
||
| rs-x11-hash = { version = "0.1.8", optional = true } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -583,19 +583,19 @@ macro_rules! hash_newtype_known_attrs { | |
|
|
||
| #[cfg(feature = "schemars")] | ||
| pub mod json_hex_string { | ||
| use schemars::JsonSchema; | ||
| use schemars::r#gen::SchemaGenerator; | ||
| use schemars::schema::{Schema, SchemaObject}; | ||
| use schemars::SchemaGenerator; | ||
| use schemars::{Schema, json_schema}; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imports should target the aliased crate name ( Given Cargo uses - use schemars::SchemaGenerator;
- use schemars::{Schema, json_schema};
+ use actual_schemars::SchemaGenerator;
+ use actual_schemars::{Schema, json_schema};Alternatively, add in hashes/src/lib.rs (or the crate root) so existing #[cfg(feature = "schemars")]
pub use actual_schemars as schemars;🤖 Prompt for AI Agents |
||
| macro_rules! define_custom_hex { | ||
| ($name:ident, $len:expr) => { | ||
| pub fn $name(generator: &mut SchemaGenerator) -> Schema { | ||
| let mut schema: SchemaObject = <String>::json_schema(generator).into(); | ||
| schema.string = Some(Box::new(schemars::schema::StringValidation { | ||
| max_length: Some($len * 2), | ||
| min_length: Some($len * 2), | ||
| pattern: Some("[0-9a-fA-F]+".to_owned()), | ||
| })); | ||
| schema.into() | ||
| pub fn $name(_generator: &mut SchemaGenerator) -> Schema { | ||
| // In schemars 1.0, we can use the json_schema! macro to create schemas | ||
| json_schema!({ | ||
| "type": "string", | ||
| "minLength": $len * 2, | ||
| "maxLength": $len * 2, | ||
| "pattern": "^[0-9a-fA-F]+$" | ||
| }) | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3789,16 +3789,30 @@ void wallet_manager_free_addresses(char **addresses, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Encrypt a private key with BIP38 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Safety | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This function is unsafe because it dereferences raw pointers: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `private_key` must be a valid, null-terminated C string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `passphrase` must be a valid, null-terminated C string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `error` must be a valid pointer to an FFIError or null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| char *bip38_encrypt_private_key(const char *private_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const char *passphrase, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FFINetwork network, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FFINetwork _network, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FFIError *error) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+3792
to
3805
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BIP38 encrypt docs: document allocation/free and current unimplemented behavior The header doesn’t say how to free the returned string and that this currently always returns NULL with WALLET_ERROR (feature disabled or not implemented). Please document both; also note Apply this header doc tweak (mirror in Rust doc comments so cbindgen regenerates it): -/*
- Encrypt a private key with BIP38
-
- # Safety
-
- This function is unsafe because it dereferences raw pointers:
- - `private_key` must be a valid, null-terminated C string
- - `passphrase` must be a valid, null-terminated C string
- - `error` must be a valid pointer to an FFIError or null
- */
+/*
+ Encrypt a private key with BIP38
+
+ Returns: newly allocated C string on success; NULL on error.
+ - Free the returned string with `string_free`.
+ - Currently unimplemented: returns NULL and sets error->code = WALLET_ERROR (or when the `bip38` feature is disabled).
+ - Note: `_network` is reserved/ignored at present.
+
+ # Safety
+ This function is unsafe because it dereferences raw pointers:
+ - `private_key` must be a valid, null-terminated C string
+ - `passphrase` must be a valid, null-terminated C string
+ - `error` must be a valid pointer to an FFIError or null
+ */📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Decrypt a BIP38 encrypted private key | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Safety | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This function is unsafe because it dereferences raw pointers: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `encrypted_key` must be a valid, null-terminated C string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `passphrase` must be a valid, null-terminated C string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `error` must be a valid pointer to an FFIError or null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
3808
to
3816
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FFI contract: document allocation/ownership for BIP38 decrypt State who frees the returned string and typical error path. -/*
- Decrypt a BIP38 encrypted private key
-
- # Safety
+/*
+ Decrypt a BIP38 encrypted private key
+
+ Returns: newly allocated C string (WIF) on success; NULL on error.
+ - Free the returned string with `string_free`.
+
+ # Safety
This function is unsafe because it dereferences raw pointers:
- `encrypted_key` must be a valid, null-terminated C string
- `passphrase` must be a valid, null-terminated C string
- `error` must be a valid pointer to an FFIError or null
*/📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| char *bip38_decrypt_private_key(const char *encrypted_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.
🛠️ Refactor suggestion
Feature should gate on std to reflect the comment and avoid accidental no_std builds.
Tie the
schemarsfeature to this crate’sstdfeature.📝 Committable suggestion
🤖 Prompt for AI Agents