Remove WitnessNode #265
Remove WitnessNode #265
Conversation
uncomputable
force-pushed
the
2025-02-remove-witness-node
branch
from
dfd1516 to
d80f52b
Compare
|
Rebased on |
|
In ce012c5: I'm really confused by the claim that you can remove an error variant by just putting incorrect data into the program. Can you elaborate why this is reasonable? |
|
Oh, I see, this is only in the |
My approach is to insert reasonable default data into a program during finalization. The data does not change the CMR of the program. The user will see how the program fails on the Bit Machine and be able to make adjustments based on that. The underlying idea is that Simplicity development will involve frequent running of the Bit Machine. We should strive to make these execution errors as helpful as possible and we should make it as easy as possible to create a program that can be run (reducing error paths during finalization). |
It's not "reasonable default data". It's unremarkable, possibly-valid data that was put into a program without the user's knowledge and will result in incredibly confusing errors. The user will not "see how the program fails on the Bit Machine". If they are lucky the program will fail at all, and then they will need to sift through their witness data and find the magic zero bits that were fabricated by this library. I agree that we should strive to make debugging and errors as simple as possible. I do not believe that it will ever be possible for debugging to be so simple that it's OK for the development toolchain to be silently inserting bad data into real programs. It's fine if this is test-only code or code that we intend to rip out. (As noted in its doccomment, this finalizer already does no type-checking even though this is now easily possible with our API, and it also can't handle disconnect, and it doesn't attempt any pruning, etc etc). But if you are saying that this is something you seriously want to do, then NACK. |
|
I'm surprised by the backlash since the idea of reducing error paths during finalization is a continuation from the zero values from #259. In a test program, made-up default values might make a program run on the Bit Machine, but if the same happens in a production program then your program is insecure. The library will never accidentally make up a valid signature, etc. But yeah, it would be good to make sure the program takes the spending paths that the user intended. In any case, this PR only touches |
|
The zero values in #259 are only used in places where we expect that they will never be visible to the user (because they will be converted to hidden nodes and deleted). There is a giant block comment explaining this. And furthermore, I had hoped that we would be able to get rid of this once we had hidden nodes again. |
|
Can you add a block comment to the |
We can remove an error path by using the respective zero value as default witness value.
The difference between ConstructNode and WitnessNode was always confusing. ConstructNode came earlier and was used mostly for decoding programs. Its witness nodes were always empty because there are no witness values during decoding. WitnessNode was added later for creating satisfied Simplicity policies. Its witness nodes were potentially filled (Option<Value>) because a witness value might exist (satisfied fragment) or not exist (unsatisfied fragment). The code base for ConstructNode and for WitnessNode is almost identical. Small changes make both identical and allow us to remove one of them. In this commit, we set <Construct as Marker>::Witness to Option<Value> and by copy over the finalization code from WitnessNode. We replace all code occurrences of `Witness*` with `Construct*`. Finally, we delete src/node/witness.rs entirely.
uncomputable
force-pushed
the
2025-02-remove-witness-node
branch
from
d80f52b to
61b7ddc
Compare
|
I added a disclaimer to I will use |
Remove
WitnessNodeby merging its code with the nearly identicalConstructNode.