Things to consider after upgrading to a beta release of `rust-bitcoin`

[tcharding]

Some things I noticed we may be able to do ...

[tcharding]

Use BlockHeight instead of the various integer types.

May need something like:

pub fn to_block_height(height: i64) -> Result<BlockHeight, BlockHeightError> {
    let h = to_u32(height).map_err(BlockHeight)?;
    BlockHeight::from_u32(h)
}

/// Error converting a `height` field to a `BlockHeight`.
///
/// As per `to_u32` but I have no clue what numeric type Core returns for height fields.
/// `rust-bitcoin` uses a `u32` because its a sane restriction.
#[derive(Debug)]
pub enum BlockHeightError(NumericError);

impl fmt::Display for BlockHeightError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write_err!(f, "failed to convert into to a block height"; self.0)
    }
}

#[cfg(feature = "std")]
impl std::error::Error for BolckHeightError {
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
}

[Abeeujah]

height is defined as int nHeight{0} in src/chain.h which is i32 in Rust, The use of a i32 allows for a sentinel value of -1 for cases not restricted to but including:

• A null or non-existent height.
• An unconfirmed transaction (not yet in a block).
• An error state during height calculations.

The Height method here returns the height or -1 when there's no block.

Should the implementation be u32 or i32?
Should every height field, parameter be Core's i32 or maintain their current definitions?

[luisschwab]

The Height method here returns the height or -1 when there's no block.

We should keep u32 and return an error instead.