tighten docs/tests for pubkey #177
Conversation
|
@febo sir, is it helpfull or should I close this? |
sdk/pubkey/src/lib.rs
Outdated
| @@ -4,6 +4,17 @@ pub use five8_const::decode_32_const; | |||
| pub use pinocchio; | |||
|
|
|||
| /// Convenience macro to define a static `Pubkey` value. | |||
| /// | |||
| /// This macro validates the pubkey at compile time, ensuring it's a valid | |||
There was a problem hiding this comment.
Not always in compile time – it depends on the input. The const qualifier allows it to be done in compile time when the input is static.
sdk/pubkey/README.md
Outdated
| @@ -48,8 +48,7 @@ pinocchio_pubkey::declare_id!("Ping111111111111111111111111111111111111111"); | |||
|
|
|||
| 3) Creating a `Pubkey` from a `&str`: | |||
| ```rust | |||
| let address = String::from("7qtAvP4CJuSKauWHtHZJt9wmQRgvcFeUcU3xKrFzxKf1"); | |||
| let owner = pinocchio_pubkey::from_str(&address); | |||
| const OWNER: pinocchio::pubkey::Pubkey = pinocchio_pubkey::from_str("7qtAvP4CJuSKauWHtHZJt9wmQRgvcFeUcU3xKrFzxKf1"); | |||
There was a problem hiding this comment.
I am not sure what is the benefit of changing this example.
sdk/pubkey/src/lib.rs
Outdated
| /// This macro also defines a helper function to check whether a given pubkey is | ||
| /// equal to the program ID. | ||
| /// This macro also defines helper functions to check whether a given pubkey is | ||
| /// equal to the program ID, providing a complete program identity management solution. |
There was a problem hiding this comment.
The part of "providing a complete program identity management solution." is strange, does not read right.
| /// ```rust | ||
| /// use pinocchio_pubkey::declare_id; | ||
| /// | ||
| /// declare_id!("11111111111111111111111111111111"); |
There was a problem hiding this comment.
This example needs to be more complete, e.g., show how the helper functions can be used.
There was a problem hiding this comment.
added more uses of helper function.
sdk/pubkey/src/lib.rs
Outdated
| @@ -35,8 +59,51 @@ macro_rules! declare_id { | |||
| }; | |||
| } | |||
|
|
|||
| /// Create a `Pubkey` from a `&str`. | |||
| /// Create a `Pubkey` from a `&str` at compile time. | |||
There was a problem hiding this comment.
Same comment about the "compile time".
| fn test_const_pubkey_creation() { | ||
| const SYSTEM_PROGRAM: Pubkey = from_str("11111111111111111111111111111111"); | ||
| // This test just ensures compilation works | ||
| assert_eq!(SYSTEM_PROGRAM.len(), 32); |
There was a problem hiding this comment.
added more asserts with different cases
It could eventually be helpful, but needs some fixes. Left some comments. |
| #[inline(always)] | ||
| pub const fn from_str(value: &str) -> pinocchio::pubkey::Pubkey { | ||
| decode_32_const(value) | ||
| } | ||
|
|
||
| /// Type alias for the Pubkey type for convenience. | ||
| pub type Pubkey = pinocchio::pubkey::Pubkey; |
There was a problem hiding this comment.
This is strange and unnecessary. You can just import the Pubkey type if you need it. Most likely you want to import the type for your tests.
Added Docs and testcases for pubkey crate.