-
Notifications
You must be signed in to change notification settings - Fork 124
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
FRI Implementation: frontend + CPU Backend + GPU Backend #795
base: main
Are you sure you want to change the base?
Conversation
hash_input.reserve(1024); // pre-allocate some space | ||
|
||
// Build the hash input | ||
build_hash_input_query_phase(hash_input); |
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.
name not clear. I had to go back and forth to understand what it does. You only use it once too so I think you'd better inline the logic here to make code easier to read
…t code from fri.cpp
…ly holds a shared_ptr to MerkleTreeBackend, so copying is ok)
icicleStreamHandle stream = nullptr; // Stream for asynchronous execution. Default is nullptr. | ||
size_t folding_factor = 2; // The factor by which the codeword is folded in each round. | ||
size_t stopping_degree = 0; // The minimal polynomial degree at which folding stops. | ||
size_t pow_bits = 16; // Number of leading zeros required for proof-of-work. Default is 0. |
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.
the comment is wrong and probably you should remove the part about the default as it is clear
icicle/include/icicle/fri/fri.h
Outdated
*/ | ||
|
||
template <typename S, typename F> | ||
eIcicleError get_fri_proof_mt( |
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.
I think fri_prove_merkle_tree is better. But anyway mt should be merkle_tree
icicle/include/icicle/fri/fri.h
Outdated
*/ | ||
|
||
template <typename S, typename F> | ||
eIcicleError verify_fri_mt( |
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.
also here merkle_tree
icicle/include/icicle/fri/fri.h
Outdated
const uint64_t output_store_min_layer, | ||
bool& valid /* OUT */); | ||
|
||
struct FRI { |
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.
why struct? you mean namespace?
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.
I think it should be 'namespace fri_merkle_tree' and then it becomes for users fri_merkle_tree::prove() and fri_merkle_tree::verify().
In any case note that we have name conventions, for example structs Should be Fri, not FRI etc.
icicle/include/icicle/fri/fri.h
Outdated
* @tparam F The field type used in the FRI protocol. | ||
*/ | ||
template <typename S, typename F> | ||
class Fri |
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.
Do you want users to use this class? if not, move to fri.cpp
…iProof when calling verify
…rify_fri_merkle_tree, fri_merkle_tree::verify
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.
Very nice! Left couple of comments
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.
Move out of versioned docs @ShanieWinitz, this only relates to v3.5 and we don't have fri in this version
- **`folding_factor: size_t`**: The factor by which the codeword is folded in each round. | ||
- **`stopping_degree: size_t`**: The minimal polynomial degree at which folding stops. | ||
- **`pow_bits: size_t`**: Number of leading zeros required for proof-of-work. If set, the optional proof-of-work phase is executed. | ||
- **`nof_queries: size_t`**: Number of queries computed for each folded layer of FRI. | ||
- **`are_inputs_on_device: bool`**: If true, the input polynomials are stored on the device (e.g., GPU); otherwise, they remain on the host (e.g., CPU). | ||
- **`is_async: bool`**: If true, it runs the hash asynchronously. |
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.
What are the defaults?
Describe the changes
This PR implements FRI frontend, CPU, GPU backend
(optional) CUDA backend branch
cuda-backend-branch: feat/fri