Remove unused variable assignment #31497

The variable is conditionally assigned toward the end of the loop and not used after. It's then set back to its default value at the beginning of the loop.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31497.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Looks like the same is true for should_shift below. Paging @murchandamus just to make sure those vars are scoped as-intended and that this isn't pointing out a bug.

I don't think the same is true for should_shift. It's last assigned on L505 but that's inside a while loop that depends on the condition of should_shift.

Ah, of course you're right. I didn't notice that loop condition.

Sorry for the noise.

Sorry for the noise.

No problem. It's pretty plain to see that this variable isn't used and can be removed. Not sure if it's worth bugging Murch to look at.

Sorry for the noise.

No problem. It's pretty plain to see that this variable isn't used and can be removed. Not sure if it's worth bugging Murch to look at.

Yes, agreed. I pinged because I thought there was more than 1 instance, which might have pointed out a broken refactor at some point. But yeah, it's pretty clearly just an unnecessary assignment.

utACK b9766c9977e58a9ebc358d9879576376e76a72b1

crACK b9766c9977e58a9ebc358d9879576376e76a72b1

This dead store was a leftover from the introduction of CoinGrinder in 6cc9a46cd0f4ed80d4523bbef1508142e0c80d27 / #27877.

Checked that there are no shenanigans with the variable being captured into a lambda further up which is then called further down.

code review ACK b9766c9977e58a9ebc358d9879576376e76a72b1

I surmise that I was resetting should_cut in the block that performs the "cut" operation, because performing the cut removes the need for a cut. I agree that the assignment to false is unnecessary, as the variables are recreated at the start of the loop, and starting with fresh variables each loop is easier to parse than instantiating outside of the loop and ensuring whether the flags are cleaned up correctly after each operation.

ACK b9766c9977e58a9ebc358d9879576376e76a72b1

ACK b9766c9977e58a9ebc358d9879576376e76a72b1

Thanks for the reviews.