Implement pruning #259
Implement pruning #259
Conversation
| /// Prune the value down to the given type. | ||
| /// | ||
| /// The pruned type must be _smaller than or equal to_ the current type of the value. | ||
| /// Otherwise, this method returns `None`. |
There was a problem hiding this comment.
In 6436f6c:
Actually, there are no checks for compatibility. Probably if you give it a bad type, the method will panic.
I would suggest just saying "the output is undefined and the function is likely to panic" and changing the Option return to just return Self.
There was a problem hiding this comment.
The type checking happens as the value is split: There are sub-tasks of the form "prune value V to type T". We match over T.
If T is unit, then the result is simply unit. V is discarded.
If T is a sum type, then V is deconstructed if it is a left or right value. A new subtask is spawned. If V is neither left or right, then the entire algorithm returns None.
If T is a product type, then V is deconstructed if it is a product. Two subtasks are spawned. Otherwise, the algorithm returns None.
I think this is somewhat of a proof that the method never panics or enters undefined behavior.
| } | ||
|
|
||
| debug_assert_eq!(output.len(), 1); | ||
| output.pop() |
There was a problem hiding this comment.
In 6436f6c:
...and I would change this debug_assert_eq to an assert_eq. It's a super cheap check, we're only doing it once, and it'll catch some forms of "the passed type is not smaller than the original type".
There was a problem hiding this comment.
I think this assertion is guaranteed to be true for all inputs.
There was a problem hiding this comment.
I added unit tests for cases where the pruned type is not smaller than or equal to the value type.
src/value.rs
Outdated
| /// - `default( 1 )` = `()` | ||
| /// - `default( A + B )` = `default(A)` | ||
| /// - `default( A × B )` = `default(A) × default(B)` | ||
| pub fn default_of_type(ty: &Final) -> Self { |
There was a problem hiding this comment.
In 8784ada:
Can we call this zero_value or something like that? I think "default" misleadingly implies that the returned value is important.
There was a problem hiding this comment.
I borrowed Rust's Default::default terminology, where the default integer values are also zeroes. However, Rust describes this trait as "A trait for giving a type a useful default value." which implies that each implementation should think about which default value is the most useful. I, on the other hand, always return the value that serializes to zeroes, so I can see that the name "default" can be misleading.
Value::zero or Value::all_zeroes could be better.
src/lib.rs
Outdated
| @@ -98,19 +100,20 @@ pub enum Error { | |||
| impl fmt::Display for Error { | |||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | |||
| match self { | |||
| Error::Decode(ref e) => fmt::Display::fmt(e, f), | |||
| Error::Decode(ref e) => write!(f, "{e}"), | |||
There was a problem hiding this comment.
In 72a190f:
You are changing this to override any format specifications on f (e.g. alternate display). This is a mild reduction in functionality (e.g. it prevents pretty-printing these variants) and also isn't mentioned in the commit message. What is the purpose of this?
There was a problem hiding this comment.
I didn't know about this, so thank you for pointing this out. I thought write!(f, "{e}") is more readable and expands into fmt::Display::fmt(e, f).
When a Simplicity program is pruned, the types of its witness nodes may change. One way to think about this is that the bits that were not used during the program execution are removed. Another way is that the type of the witness node shrinks.
It is useful to populate witness nodes that will be pruned out after the first run of the Bit Machine. We don't need to ask the user to provide made-up witness data for branches that are not executed. To this end, we can compute a zero value for each Simplicity type. The zero value serializes to a string of zeroes, and it is easy to compute.
Include execution errors in the top-level Simplicity error enum. Finalizing a witness program as a pruned redeem program involves both Simplicty errors (disconnected branch is missing) and execution errors (pruning run failed). We can remove this if we remove the "disconnected branch is missing" error variant.
Optional tracking using the CaseTracker trait. The implementation for the unit type will be optimized out by the compiler.
Prune finalized programs. Tests will follow in two commits.
We construct redeem programs by finalizing witness programs. We often want the finalized program to be pruned, so we introduce the finalize_pruned method. This is method that most people should use. In cases where the transaction environment is not yet known, there is the finalize_unpruned method. We could make this method private in the future, depending on how we want to handle pruning in our API. We might need to come up with a nicer way to track which program is pruned for which environment, and so on.
Test correct pruning of sum and product types. We should add a fuzz test to prune a larger range of programs, but I leave this to a follow-up PR. I want to port the fuzzing infrastructure to cargo-fuzz first.
Remove manual pruning from the policy satisfier. The satisfier still needs to track the satisfiability of sub-policies, so we introduce a symmetric Result<T, T> where T is the compiled sub-policy and Ok / Err signify satisfiability. Refactor the code that satisfies threshold fragments by using the iterator API and by removing mutable state. Introduce SatisfierError to better track which errors a satisfier can actually throw and what to do about them. We can remove the "missing assembly" error if we allow the construction of hidden nodes. The satisfier only returns pruned programs and needs to know the transaction environment because of that. Returning unpruned program seems like a potential footgun, especially because callers will want to immediately put their programs on the blockchain.
WitnessNode no longer needs to track whether it should be pruned, because we do pruning directly on the Bit Machine.
IncompleteFinalization was never a nice error. It told the caller that something was wrong about the witness data, but not what exactly and what to do about it. A better approach is to provide useful feedback when the Bit Machine is run.
uncomputable
force-pushed
the
2025-01-pruning
branch
from
9389c67 to
c1ffa03
Compare
|
Rebased and addressed the comments. |
Implement full pruning by running programs on the Bit Machine and tracking (un)used branches. The unused branches are then removed from the program and types are adjusted. Witness data is also adjusted by removing unused bits.
Fixes #84