[Fix] Address feedback from program upgradability audits.

[d0cd]

This PR addresses feedback from auditors that reviewed program upgradability.
Auditors used the following commit: 962d5fd as the reference.

[randomsleep]

This requires us to call this function on every error, which is error-prone. I think an alternative way is to use "defer" function to automatically call the revert_stacks function when exiting the current scope. Like this:

    defer! {
        // This code runs when the scope ends
        process.revert_stacks();
    }

If the scope succeeds, we call process.commit_stacks() like the current way and the above revert_stacks will do nothing.

[d0cd]

This is pretty neat! I've never used this macro.
Is there a crate you recommend? defer, scopeguard or defer_lite?
I'll check with the rest of the team on whether they're open to this.

[randomsleep]

The macro is basically defining a Struct in the current scope and execute the specified function when it is dropped (called RAII). You can implement a simple local version like this:

// Define a struct that holds a closure to be executed on drop
pub struct Defer<F: FnOnce()> {
    f: Option<F>,
}

impl<F: FnOnce()> Defer<F> {
    pub fn new(f: F) -> Self {
        Defer { f: Some(f) }
    }
}

impl<F: FnOnce()> Drop for Defer<F> {
    fn drop(&mut self) {
        if let Some(f) = self.f.take() {
            f();
        }
    }
}

// Macro to create a defer block with a unique guard variable
#[macro_export]
macro_rules! defer {
    ($($body:tt)*) => {
        let _defer_guard = $crate::Defer::new(|| { $($body)* });
    };
}

And use it:

fn main() {
    defer! {
        println!("execute defer");
    }

    println!("in main");
}
// Output:
// in main
// execute defer

[d0cd]

First of all big fan of this, I'll be borrowing this in a lot of my other code :)
Do you see a way defer can be done conditionally? Basically, if there's an error then revert_stacks otherwise commit_stacks?

[randomsleep]

For this case, if you already executed commit_stacks, then the revert_stacks (or called revert_uncommitted_stack) will do nothing right?

[d0cd]

Open question on whether it's cleaner to stage stacks in Process or manage them out of band in atomic_{finalize, speculate}

[d0cd]

Note to reviewers. Removing this read lock held over the duration of verification improved snarkOS node performance.

Furthermore, this is safe since editions are always sequentially incremented. If an edition is updated, the cache key will simply be invalid and the full verification will run again.

[d0cd]

Closed in favor of #2807