input-output-hk / implementation-decisions

A repository for all concrete decisions made about how to implement the formal specs
Other
5 stars 5 forks source link

Address binary serialisation #12

Closed NicolasDP closed 5 years ago

NicolasDP commented 5 years ago

resolves #2

Propose short address binary serialisation as well as a new human readable encoding for it using bech32

rendered markdown

commandodev commented 5 years ago

I think we've given this enough time and eyeballs now. @NicolasDP if you could adress @nc6's review I'll go ahead and merge.

NicolasDP commented 5 years ago

Thanks for the feedback and fixes.

NicolasDP commented 5 years ago

@boothead , before merging we need to agree on the CIP number: 0001 seems the obvious choice.

commandodev commented 5 years ago

@boothead , before merging we need to agree on the CIP number: 0001 seems the obvious choice.

Good point! Yes, let's go for 0001!

NicolasDP commented 5 years ago

@kantp thanks for your feedback on this. Just like you said in your previous comment : we can implement the spec's addresses with the address types we currently have. The pointer address is not necessary now. Right now we need to focus on delivering the necessary resources before Q2. To do so we need to make choices and axes the non necessary elements. As you notice, there are room for new types (up to 121 new address types) so we can always extend this CIP with another one (and I think the pointer address will need a proper CIP since there are lot of questions that will arise from this).

commandodev commented 5 years ago

I'm going to go ahead and merge this. Thanks everyone for their input. If we've missed anything, please feel free to put comments here and we can look at re-opening if necessary or superseding with another RFC.

SebastienGllmt commented 5 years ago

Two points that I thought might be worth bringing up:

1) In the original BECH32 spec, there was discussion about whether or not the checksum should come at the start of the data section or at the end. The rationale for putting it at the front is because most users only check that the first few characters in an address match (especially in the hardware wallet case where scrolling an address to the end on a tiny screen is tedious and slow). They ended up having to put it at the end for some compatibility reason but we're not bound to this design decision

2) BitcoinCash uses : as the separator instead of 1. The rationale is that this makes the addresses not only easier to read, it also makes all your addresses automatically into URI Schemes (ex: bitcoincash:q9adhakpwzztepkpwp5z0dq62m6u5v5xtyj7j3h2ws4mr9g01). The special character ':' is still part of the QR code alphanumeric mode so you still get equivalent compression. The main downside to this is that right now websites are only allowed to register URI Schemes that start with web+ (only desktop applications can omit the web+ prefix). This is why the Cardano URI Scheme spec we wrote for Yoroi uses web+. That being said, it may be worth making this change to cardano: for our addresses also (reference: https://www.bitcoincash.org/spec/cashaddr.html )

SebastienGllmt commented 5 years ago

I also noticed the Rust codebase uses many different HRPs for bech32 serialization of internal data structures. Maybe these should be defined in some registry? (ex: https://github.com/input-output-hk/chain-libs/blob/d50858350f37e864aeda6b7ce573f773a2dbecd2/chain-crypto/src/algorithms/ed25519_derive.rs#L45)

ilap commented 5 years ago

I think, they should consider ':' separator for addresses, but for the keys we should stick with the 1, which will give some advantages e.g. easily distinguish between keys and addresses. But, IMO, the Bech32 should be standardised first (versioned either) for general use as chain-libs uses some other variant of the original, due to the fact that the original spec has some limitations, the 90 max data size as an example.

ilap commented 5 years ago

However, the checksum is not really relevant as it was designed for length 71 and is effective till 89. That's why there is a size restriction (90 i.e. 450 bits data which is 56 length octets) in the original spec.

SebastienGllmt commented 5 years ago

I agree. The fact that the Rust codebase uses bech32 for literally everything (not just addresses) is strange because the checksum wasn't designed to be used for arbitrary lengths. As long as we don't care about the checksum it's not an issue though