Fix Value::prune corrupts sum tag bits when prepending Left (0) tag

[topologoanatom]

Related to #337. These changes fix behaviour described in #333.

[delta1]

I think it would be great to add unit tests that repro #333 / #337 that pass with this change but fail without it

[topologoanatom]

I think it would be great to add unit tests that repro #333 / #337 that pass with this change but fail without it

I’m currently unable to reproduce bug #333 except by using the full flow described in the issue above. Adding that script to a unit test is difficult, as it requires specific functionality from SimplicityHL

[apoelstra]

Can you take the reproduction in #337, patch rust-simplicity to add some dbg! calls around the parameters passed to Value::prune, then directly construct the value and type you see in a unit test here?

There's likely also some room to manually reduce the size of the test value.

Alternately let me attempt to write a fuzz test about this..

[topologoanatom]

Can you take the reproduction in #337, patch rust-simplicity to add some dbg! calls around the parameters passed to Value::prune, then directly construct the value and type you see in a unit test here?

There's likely also some room to manually reduce the size of the test value.

Alternately let me attempt to write a fuzz test about this..

Yes, I will reply here once I have obtained that info

[topologoanatom]

Can you take the reproduction in #337, patch rust-simplicity to add some dbg! calls around the parameters passed to Value::prune, then directly construct the value and type you see in a unit test here?

There's likely also some room to manually reduce the size of the test value.

Alternately let me attempt to write a fuzz test about this..

Here are the logs taken right before pruning in the node::redeem::prune_with_tracker() function.

https://gist.github.com/topologoanatom/c4288260d7406b69ecfe658c3a11ff62

The first is from before the final conversion (line 443), and the second is from just before the return (line 455).

[apoelstra]

I would dump the padded bit-encoding of both values (using iter_padded) so you can easily reconstruct them in a unit tests. The types you will probably have to construct manually.

[apoelstra]

FYI I wrote a fuzz test that just tries pruning various values and then seeing if the pruned compact bit encodings were the same as the original bit-encodings except with bits deleted. I ran this for 1200 CPU hours and did not see any crashes.

I will try to PR my fuzztest over the weekend.

But I'm unconvinced there is a bug here.