rust-bitcoin / rust-bech32

Bech32 format encoding and decoding
MIT License
93 stars 49 forks source link

UncheckedHrpstring should have a `data` accessor #160

Closed apoelstra closed 8 months ago

apoelstra commented 8 months ago

This structure is a POD pair of a HRP/data with some utility functions for choosing a checksum and stripping it. To be useful you need to be able to access the underlying data.

Probably same with CheckedHrpstring.

tcharding commented 8 months ago

I'm not sure this adds value since the caller can get this by just splitting the input string at the 1 separator and calling as_bytes(). Adding this functionality potentially clutters the API (because we the have multiple ways of getting "bytes") and perhaps makes the types harder to understand? Also since these two types are only useful for advanced usage the caller would likely already be quite familiar with the structure of the input bech32 string.

apoelstra commented 8 months ago

I'm not sure this adds value since the caller can get this by just splitting the input string at the 1 separator and calling as_bytes().

..so they need to split the string twice, once when creating the HrpString and then once manually? (And then their code needs to be correct wrt the structure of HRP strings)

Adding this functionality potentially clutters the API (because we the have multiple ways of getting "bytes") and perhaps makes the types harder to understand?

What is the other way to get the underlying bytes of the input string?

apoelstra commented 8 months ago

I would also be happy if there were a single method that returned the first byte as a Fe32 and removed it from the underlying buffer. This is the only missing piece to let the user implement CheckedHrpstring/SegwitHrpstring themselves.

tcharding commented 8 months ago

removed it from the underlying buffer.

So this piece is missing still, I was not able to fit it into the current abstraction (either in my head or in code :)

apoelstra commented 8 months ago

@tcharding look at CheckedHrpstring::validate_segwit:

        let witness_version = Fe32::from_char(self.data[0].into()).unwrap();
        self.data = &self.data[1..]; // Remove the witness version byte from data.

basically I just want those two lines in its own &mut self method.

tcharding commented 8 months ago

No worries, we can do that.

tcharding commented 8 months ago

Done