-
Notifications
You must be signed in to change notification settings - Fork 56
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
refactor: Refactor proof handling for ownership and borrowing #1214
refactor: Refactor proof handling for ownership and borrowing #1214
Conversation
24fbf44
to
e301252
Compare
How come? #1209 accomplishes the above without such clone |
That's because #1209 drops the recursive proofs as it calls compress on them ( This is fine for #1209, since reusing the recursive proof after compression of that recursive proof is not exercised anywhere I could spot. |
7ba040a
to
098f2a6
Compare
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.
Indeed, we're going to need this in order to compress the recursive proof and still move on with it, without losing ownership. Thanks for unlocking this!
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.
Now that #1209 was merged, this has become high priority to unlock the next steps of the proof server
On a long enough timeline, we'll want to generate compressed proofs without dropping the associated `RecursiveProof`. This is for the purpose of re-starting the proof using the prior `RecursiveProof`, which at the moment can only be obtained through cloning prior to compression. The current approach witnesses that compression only requires a reference to the current proof, and returns a `Cow`: the only situation in which this would result in a clone is if: - we create a `CompressedProof`, - we call `compress()` on it, - we further perform a modificaiton of that `CompressedProof` that requires an owned value.. In detail: - Updated `compress` function across various files to take `&self` instead of `self`, avoiding the drop of the `RecursiveProof`; - Adjusted dependencies in chain-server's `Cargo.toml` to use workspace versions, streamlining dependency management and increasing project consistency.
098f2a6
to
00b579b
Compare
On a long enough timeline, we'll want to generate compressed proofs without dropping the associated
RecursiveProof
. This is for the purpose of re-starting the proof using the priorRecursiveProof
, which at the moment can only be obtained through cloning prior to compression (which itself would be impossible prior to the companion PR used here).The current approach witnesses that compression only requires a reference to the current proof, and returns a
Cow
: the only situation in which this would result in a clone is if:CompressedProof
,compress()
on it,CompressedProof
that requires an owned value.=> this flow seems unlikely.
In detail:
compress
function across various files to take&self
instead ofself
, avoiding the drop of theRecursiveProof
;Cargo.toml
to use workspace versions, streamlining dependency management and increasing project consistency.Note
This depends on and requires the companion PR argumentcomputer/arecibo#364. CI is fully expected to fail on the
cargo-deny
job due to use of a fork (until companion PR is merged).