Feat/blake pow #766
Feat/blake pow #766
Conversation
b821811 to
5e3e142
Compare
icicle/include/icicle/hash/pow.h
Outdated
| false; /**< True to run the pow solver asynchronously, false to run synchronously. Default is false. */ | ||
| ConfigExtension* ext = nullptr; /**< Pointer to backend-specific configuration extensions. Default is nullptr. */ | ||
| }; | ||
| const uint8_t BLOCK_LEN = 64; |
There was a problem hiding this comment.
why not in the config struct?
There was a problem hiding this comment.
oh this value is unused, forgot to delete it
icicle/include/icicle/hash/pow.h
Outdated
|
|
||
| eIcicleError pow_solver( | ||
| Hash hasher, | ||
| uint8_t* challenge, |
There was a problem hiding this comment.
for consistency maybe make it std::byte* like hash APIs
icicle/include/icicle/hash/pow.h
Outdated
| uint32_t padding_size, | ||
| uint8_t bits, | ||
| const PowConfig& config, | ||
| bool* found, |
There was a problem hiding this comment.
Other ICICLE APIs in C++ use reference for outputs, not pointers so probably better to make it consistent.
There was a problem hiding this comment.
(1) I now noticed that you support them being device memory. Also the found boolean? This is a little confusing I think.
(2) Please add description to the API. It's unclear what should be the size of nonce for example etc.
wrappers/rust/icicle-hash/src/pow.rs
Outdated
| use icicle_core::hash::{Hasher, HasherHandle}; | ||
| use icicle_runtime::{config::ConfigExtension, errors::eIcicleError, memory::HostOrDeviceSlice, IcicleStreamHandle}; | ||
|
|
||
| const PADDING_SIZE: u32 = 24; |
There was a problem hiding this comment.
this should be in the config I think
wrappers/rust/icicle-hash/src/pow.rs
Outdated
| #[derive(Debug, Clone)] | ||
| pub struct PowConfig { | ||
| pub stream: IcicleStreamHandle, // `icicleStreamHandle` is represented as a raw pointer. | ||
| pub is_challenge_on_device: bool, |
There was a problem hiding this comment.
Can you make the 'on-device' fields private? We usually set them instead of the users so they don't need to care about it.
|
|
||
| let hasher = Blake3::new(0).unwrap(); | ||
|
|
||
| let err = pow_solver(&hasher, input_host, BITS, &cfg, found_host, nonce_host, mined_hash_host); |
There was a problem hiding this comment.
(1) I am not sure I understand this API. How do you define the threshold value for the hash result?
(2) can you verify that with the nonce you found, the hashing indeed succeeds? in this test I mean
|
added |
icicle/include/icicle/hash/pow.h
Outdated
| * @return eIcicleError Error code indicating success or failure of the operation. | ||
| */ | ||
| eIcicleError pow_solver( | ||
| Hash& hasher, |
There was a problem hiding this comment.
why hasher and challenge not const?
icicle/include/icicle/hash/pow.h
Outdated
| * @return eIcicleError Error code indicating success or failure of the verification process. | ||
| */ | ||
| eIcicleError pow_check( | ||
| Hash& hasher, |
icicle/include/icicle/hash/pow.h
Outdated
| * | ||
| * @return eIcicleError Error code indicating success or failure of the verification process. | ||
| */ | ||
| eIcicleError pow_check( |
There was a problem hiding this comment.
mayber pow_verify? I think clearer than pow_check
| uint8_t solution_bits, | ||
| const PowConfig& config, | ||
| bool& found, | ||
| uint64_t& nonce, |
There was a problem hiding this comment.
yeah, I changed it because they are just single values
This PR adds POW algorithm
cuda-backend-branch: feat/blake-pow