Avoid unused locals for CScript::IsWitnessProgram

[Empact]

For those cases where the version and program are not useful.

[practicalswift]

utACK b9f0169bcce3ca7c9d0646ddda5bf264062be87b

Nice simplification

[jimpo]

utACK b9f0169

[jimpo]

Might as well return this directly.

[Empact]

I like that style too, but held back for the sake of simplifying review. Here's my choice if I'm to simplify the method:

static constexpr bool IsOP_N(const unsigned char op)
{
    return (op == OP_0 || (op >= OP_1 && op <= OP_16));
}

// A witness program is any valid CScript that consists of a 1-byte push opcode
// followed by a data push between 2 and 40 bytes.
bool CScript::IsWitnessProgram() const
{
    return ((this->size() >= 4 && this->size() <= 42) &&
            IsOP_N((*this)[0]) &&
            (size_t)((*this)[1] + 2) == this->size());
}

Incidentally, this matches the style of IsPayToWitnessScriptHash and IsPayToScriptHash, above. Thoughts?

[promag]

@Empact looks good, I'd ACK that.

[promag]

utACK b9f0169.

[promag]

@Empact looks good, I'd ACK that.

[Empact]

Rebased, squashed, and updated to take advantage of IsSmallInteger from #13194

[sdaftuar]

I am generally a concept NACK on changes like this; IMO the review burden for refactoring consensus-critical code typically outweighs the benefits.

[laanwj]

I am generally a concept NACK on changes like this; IMO the review burden for refactoring consensus-critical code typically outweighs the benefits.

Agree, seems to risky, closing.