Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor: better select challenges function #1731

Merged
merged 1 commit into from
Dec 1, 2023
Merged

Conversation

vmx
Copy link
Contributor

@vmx vmx commented Oct 31, 2023

The function to select the challenges changes a lot over time and now is hardly comprehensible. Hence it's refacored into a more understandable version. It uses a div_ceil function which is expecte to be included in the Rust standard library at some point in the future.

Copy link
Contributor

@DrPeterVanNostrand DrPeterVanNostrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Just FYI if you're interested, I believe challenges_count_all is a leftover from when PoRep's de#cluded a specific number of challenges per layer (and challenges_count_all meant the "total number of challenges across all layers per partition"). And a potential future refactor that I would also support is renaming challenges_count_all to something like partition_challenge_count.

@vmx
Copy link
Contributor Author

vmx commented Nov 2, 2023

Just FYI if you're interested, I believe challenges_count_all is a leftover from when PoRep's de#cluded a specific number of challenges per layer (and challenges_count_all meant the "total number of challenges across all layers per partition"). And a potential future refactor that I would also support is renaming challenges_count_all to something like partition_challenge_count.

Exactly! I tackle that in a later commit on the ni-porep work: d2505cb.

@vmx vmx force-pushed the split-layer-and-challenges branch from 1fc3399 to 329eacb Compare November 16, 2023 13:45
@vmx vmx force-pushed the split-layer-and-challenges branch from 329eacb to 9b129ce Compare December 1, 2023 14:05
Base automatically changed from split-layer-and-challenges to master December 1, 2023 15:51
@vmx vmx dismissed DrPeterVanNostrand’s stale review December 1, 2023 15:51

The base branch was changed.

The function to select the challenges changes a lot over time and now
is hardly comprehensible. Hence it's refacored into a more understandable
version. It uses a `div_ceil` function which is expecte to be included
in the Rust standard library at some point in the future.
@vmx vmx force-pushed the better-select-challenges branch from 589d58c to e50baf8 Compare December 1, 2023 15:54
@vmx vmx merged commit 777d4d3 into master Dec 1, 2023
@vmx vmx deleted the better-select-challenges branch December 1, 2023 18:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants