-
Notifications
You must be signed in to change notification settings - Fork 256
Revert "Backport Proof of Space perf improvements from abundance" #3722
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
Conversation
🛡️ Immunefi PR ReviewsWe noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below: Once submitted, we'll take care of assigning a reviewer and follow up here. |
|
I talked to Jeremy about this, we're thinking of doing a revert to get the stack overflow fix out. @vedhavyas what do you think? |
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.
Comment @cursor review or bugbot run to trigger another review on this PR
| }); | ||
|
|
||
| let (sector, plotted_sector, table_generator) = plotting_result_receiver.await.unwrap(); | ||
| let (sector, plotted_sector, mut table_generator) = plotting_result_receiver.await.unwrap(); |
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.
Bug: Mutable borrow break after trait change
The variable table_generator was declared as mutable on line 153 (let (sector, plotted_sector, mut table_generator) = plotting_result_receiver.await.unwrap();), but the trait TableGenerator::generate() now requires a mutable reference after the revert (changed from &self to &mut self in the trait definition). However, the code on line 189 calls table_generator.generate_parallel(seed) which also requires &mut self. Since table_generator is captured by the closure on line 189 and used multiple times, this will cause compilation errors due to multiple mutable borrows. The generate_parallel method signature was changed from taking &self to &mut self, which breaks the closure's ability to call it multiple times.
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.
Hallucination, there are no compilation errors.
|
Yeah, we can revert and pick it up later |
Reverts #3712 because it contained some unexpected incompatible changes.