PushBytes::read_scriptint is a bit unwieldy

[tcharding]

While upgrading mininscript to track master branch I found that the PushBytes::read_scriptint function is unwieldy.

Should we add this back in:

/// Decodes an integer in script(minimal CScriptNum) format.
///
/// See [`PushBytes::read_scriptint`] for documentation.
pub fn read_scriptint(v: &[u8]) -> Result<i64, Error> {
    let push = <&PushBytes>::try_from(v).map_err(|_| Error::NumericOverflow)?;
    push.read_scriptint()
}

[tcharding]

ref: rust-bitcoin/rust-miniscript#755

[tcharding]

And the PRs that touched read_scriptint:

• script::read_uint should read i32 #1547
• Move read_scriptint to PushBytes & create read_int function #2535

[apoelstra]

It looks like there are two places where we call this method in miniscript. One is to convert a slice from script::Instruction::PushBytes which for some reason holds a &[u8] instead of a PushBytes. Shouldn't it hold a PushBytes instead?

The other is in witness_to_scriptsig in src/util.rs which is, frankly, a bizarre method and I think it's fine if the code there is a bit unwieldy.

[tcharding]

It looks like there are two places where we call this method in miniscript. One is to convert a slice from script::Instruction::PushBytes which for some reason holds a &[u8] instead of a PushBytes. Shouldn't it hold a PushBytes instead?

This is a bitcoin::script::Instruction::PushBytes(PushBytes) so this call site is ok.

The other is in witness_to_scriptsig in src/util.rs which is, frankly, a bizarre method and I think it's fine if the code there is a bit unwieldy.

This is the call site that caused me to raise the issue, so you've cleared it up nicely. Thanks