BitcoinKit icon indicating copy to clipboard operation
BitcoinKit copied to clipboard

Address protocol should not mandate output encodings

Open federicobond opened this issue 6 years ago • 2 comments

The Address protocol contains two properties (base54 and cashaddr) that enforce a requirement for addresses to be representable in certain encodings.

 var base58: String { get }
 var cashaddr: String { get }

These should belong to the concrete classes implementing, not the base protocol. The only thing that can be said about an Address at the generic protocol level is that it can be converted to a String, but not the specific encoding of this representation. For example new SegWit addreses cannot be represented in either of the two encodings mentioned earlier.

federicobond avatar Sep 01 '18 05:09 federicobond

I understand what you say.

For me, base58 and cashaddr are like convenience function like extension. If we implement it as protocol extension (computed property / function), it calculates checksum every time, and it may waste time. That's why I decided this to be stored property.

I think performance test is necessary for justifying what I thought.

usatie avatar Sep 17 '18 06:09 usatie

Those are two different things. We can move the properties to the concrete address types that support those representations while only requiring a generic string representation at the Address protocol level. No need to sacrifice that performance.

federicobond avatar Sep 17 '18 14:09 federicobond