Skip to content

fix: fix value bit corruption in right_shift#348

Draft
stringhandler wants to merge 2 commits intoBlockstreamResearch:masterfrom
stringhandler:fix/value_padding
Draft

fix: fix value bit corruption in right_shift#348
stringhandler wants to merge 2 commits intoBlockstreamResearch:masterfrom
stringhandler:fix/value_padding

Conversation

@stringhandler
Copy link

Yet another option to solve #337

The tests are actually generated by Claude. The comments are a bit wordy, so happy to try clean it up if it bothers anyone.

The interesting thing to note is that the bug is not as a result of from_padded_bits, as @apoelstra (and myself) suspected. Upon adding the two regression tests from #339, prune_regression_337_1 still fails, indicating another bug in from_padded_bits.

However, prune_regression_337_2 passes on this PR without any changes to from_padded_bits. The tests added in this PR also reproduce the error without calling from_padded_bits.

I have also tried to keep the optimization of not cloning the array if the bit is already equal to new_bit and added it to the true case as well.

For those interested here are the before and after benchmarks. Nothing jumps out at me as a huge improvement or degradation.

BEFORE (Commit f84b09d)

test node::redeem::benches::decode_fixed_program          ... bench:      46,840.83 ns/iter (+/- 596.58)
test value::benches::bench_value_create_512_256           ... bench:      60,676.67 ns/iter (+/- 285.67)
test value::benches::bench_value_create_512_256_compact   ... bench:       3,226.49 ns/iter (+/- 73.52)
test value::benches::bench_value_create_512_256_padded    ... bench:       3,277.02 ns/iter (+/- 108.64)
test value::benches::bench_value_create_64k               ... bench:     669,047.50 ns/iter (+/- 4,790.50)
test value::benches::bench_value_create_64k_compact       ... bench:      34,772.54 ns/iter (+/- 3,966.75)
test value::benches::bench_value_create_64k_padded        ... bench:      33,474.00 ns/iter (+/- 3,886.75)
test value::benches::bench_value_create_deep_some         ... bench:     402,400.62 ns/iter (+/- 3,995.75)
test value::benches::bench_value_create_deep_some_compact ... bench:     145,885.00 ns/iter (+/- 1,446.33)
test value::benches::bench_value_create_deep_some_padded  ... bench:       1,224.16 ns/iter (+/- 4.50)
test value::benches::bench_value_create_u2048             ... bench:     469,603.75 ns/iter (+/- 6,750.75)
test value::benches::bench_value_create_u2048_compact     ... bench:       2,117.00 ns/iter (+/- 7.90)
test value::benches::bench_value_create_u2048_padded      ... bench:       2,063.22 ns/iter (+/- 11.30)
test value::benches::bench_value_create_u64               ... bench:          22.49 ns/iter (+/- 0.32)
test value::benches::bench_value_create_u64_compact       ... bench:         165.02 ns/iter (+/- 1.33)
test value::benches::bench_value_create_u64_padded        ... bench:         114.52 ns/iter (+/- 0.88)
test value::benches::bench_value_display_512_256          ... bench:      69,926.25 ns/iter (+/- 804.88)
test value::benches::bench_value_display_64k              ... bench:     707,050.00 ns/iter (+/- 10,249.00)
test value::benches::bench_value_display_deep_some        ... bench:      44,677.50 ns/iter (+/- 727.31)
test value::benches::bench_value_display_u2024            ... bench:      44,629.52 ns/iter (+/- 341.24)
test value::benches::bench_value_display_u64              ... bench:         351.55 ns/iter (+/- 2.40)

AFTER (c70e3ed)

test node::redeem::benches::decode_fixed_program          ... bench:      47,575.83 ns/iter (+/- 1,888.92)
test value::benches::bench_value_create_512_256           ... bench:      62,040.00 ns/iter (+/- 684.18)
test value::benches::bench_value_create_512_256_compact   ... bench:       3,091.83 ns/iter (+/- 19.26)
test value::benches::bench_value_create_512_256_padded    ... bench:       3,034.38 ns/iter (+/- 17.38)
test value::benches::bench_value_create_64k               ... bench:     671,748.75 ns/iter (+/- 4,494.75)
test value::benches::bench_value_create_64k_compact       ... bench:      32,111.70 ns/iter (+/- 5,517.22)
test value::benches::bench_value_create_64k_padded        ... bench:      32,177.62 ns/iter (+/- 7,199.71)
test value::benches::bench_value_create_deep_some         ... bench:     401,901.88 ns/iter (+/- 4,945.75)
test value::benches::bench_value_create_deep_some_compact ... bench:     145,748.00 ns/iter (+/- 833.20)
test value::benches::bench_value_create_deep_some_padded  ... bench:       1,205.44 ns/iter (+/- 5.89)
test value::benches::bench_value_create_u2048             ... bench:     482,537.50 ns/iter (+/- 8,726.38)
test value::benches::bench_value_create_u2048_compact     ... bench:       2,169.61 ns/iter (+/- 7.86)
test value::benches::bench_value_create_u2048_padded      ... bench:       2,075.53 ns/iter (+/- 16.34)
test value::benches::bench_value_create_u64               ... bench:          32.03 ns/iter (+/- 0.91)
test value::benches::bench_value_create_u64_compact       ... bench:         162.57 ns/iter (+/- 1.75)
test value::benches::bench_value_create_u64_padded        ... bench:         113.74 ns/iter (+/- 0.67)
test value::benches::bench_value_display_512_256          ... bench:      74,847.14 ns/iter (+/- 1,099.57)
test value::benches::bench_value_display_64k              ... bench:     709,160.00 ns/iter (+/- 12,038.00)
test value::benches::bench_value_display_deep_some        ... bench:      43,401.18 ns/iter (+/- 559.59)
test value::benches::bench_value_display_u2024            ... bench:      44,913.00 ns/iter (+/- 477.15)
test value::benches::bench_value_display_u64              ... bench:         357.24 ns/iter (+/- 7.69)

New benchmarks

test node::redeem::benches::decode_fixed_program          ... bench:      47,575.83 ns/iter (+/- 1,888.92)
test value::benches::bench_value_create_512_256           ... bench:      62,040.00 ns/iter (+/- 684.18)
test value::benches::bench_value_create_512_256_compact   ... bench:       3,091.83 ns/iter (+/- 19.26)
test value::benches::bench_value_create_512_256_padded    ... bench:       3,034.38 ns/iter (+/- 17.38)
test value::benches::bench_value_create_64k               ... bench:     671,748.75 ns/iter (+/- 4,494.75)
test value::benches::bench_value_create_64k_compact       ... bench:      32,111.70 ns/iter (+/- 5,517.22)
test value::benches::bench_value_create_64k_padded        ... bench:      32,177.62 ns/iter (+/- 7,199.71)
test value::benches::bench_value_create_deep_some         ... bench:     401,901.88 ns/iter (+/- 4,945.75)
test value::benches::bench_value_create_deep_some_compact ... bench:     145,748.00 ns/iter (+/- 833.20)
test value::benches::bench_value_create_deep_some_padded  ... bench:       1,205.44 ns/iter (+/- 5.89)
test value::benches::bench_value_create_u2048             ... bench:     482,537.50 ns/iter (+/- 8,726.38)
test value::benches::bench_value_create_u2048_compact     ... bench:       2,169.61 ns/iter (+/- 7.86)
test value::benches::bench_value_create_u2048_padded      ... bench:       2,075.53 ns/iter (+/- 16.34)
test value::benches::bench_value_create_u64               ... bench:          32.03 ns/iter (+/- 0.91)
test value::benches::bench_value_create_u64_compact       ... bench:         162.57 ns/iter (+/- 1.75)
test value::benches::bench_value_create_u64_padded        ... bench:         113.74 ns/iter (+/- 0.67)
test value::benches::bench_value_display_512_256          ... bench:      74,847.14 ns/iter (+/- 1,099.57)
test value::benches::bench_value_display_64k              ... bench:     709,160.00 ns/iter (+/- 12,038.00)
test value::benches::bench_value_display_deep_some        ... bench:      43,401.18 ns/iter (+/- 559.59)
test value::benches::bench_value_display_u2024            ... bench:      44,913.00 ns/iter (+/- 477.15)
test value::benches::bench_value_display_u64              ... bench:         357.24 ns/iter (+/- 7.69)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant