[discussion] allow PublicKeys to be input and output as base58check

[mikemaccana]

Motivation

StackOverflow is filled with people that truncated one character of a wallet address and send tokens to that address. Sadly with how base58 works, a truncated address can still be valid. Boo.

What if we allowed inputs for publickey values to be either base58 or base58check? And likewise, made a way to get base58 check outputs from publickey?

We'd need support from a lot of other software (eg, wallets would need to output checked addresses) but starting with web3 is a good start.

Example use case

An app outputs base58checked using web3.js

Someone pasted that into another app, and truncates it.

The app tells the user the address is invalid, instead of sending tokens to another address.

[mcintyre94]

Interesting, TIL about base58check! I've noticed the same issue about truncating an address and it still being valid in isAddress.

I'd be curious to know why it wasn't used for Solana, given it's used for Bitcoin. Is there some tradeoff that means we wouldn't want to use it for some reason?

In general it should be pretty easy to write a codec for base58CheckedAddress, and an isBase58CheckedAddress etc. It'd be easy to use that to convert from base58check -> base58 address (and vice versa)

I think allowing them as input throughout the library gets trickier though. Base58EncodedAddress is just an opaque string type, and in theory this would be the same - so you couldn't distinguish them at runtime without introducing some sort of wrapper that we've avoided so far for addresses.

A good starting point here would probably be for someone to implement a web3js codec + other functions we have for addresses for Base58Checked, which could be an independent package for now. That'd be composable with everything else, and you could convert between the two formats easily (eg base58checked encoder, existing address decoder to go from base58check to base58).

Then we can figure out if/how to inline it and use it across the library without introducing too much complexity. We'd also need to update the Kinobi renderer + generated clients if we made that sort of change to the library, to similarly accept them as input.

[buffalojoec]

A good starting point here would probably be for someone to implement a web3js codec + other functions we have for addresses for Base58Checked, which could be an independent package for now.

New package makes sense to me, so we don't have to worry about breaking any APIs for the existing codecs packages. It shouldn't be imported/reexported from any of them, either.

Then we can figure out if/how to inline it and use it across the library without introducing too much complexity.

I'm envisioning something like a flag (allowTruncated: boolean) in different helpers like isAddress. Would there be a drawback to such a flag if it was set to true by default?

[mcintyre94]

I'm envisioning something like a flag (allowTruncated: boolean) in different helpers like isAddress. Would there be a drawback to such a flag if it was set to true by default?

I don't think that really matches the intention here. If you have a plain current base58 address because someone copied it from their wallet, you don't want to allow truncation, you just don't have a checksum. People would set it to false without reading the docs, thinking they can protect their users from truncating their existing address, and then no address they have would be considered valid. They're really different data types - presumably a Base58Check is basically going to look like a Bitcoin address. You also don't want 0xbitcointhingy to pass isAddress, because then you might pass it to something Solana that's going to fail.

I think one way would be to introduce an opaque type called something like Base58EncodedAddressFromChecked, which is under the hood just a base58 address we see today. Like with readonly uint8 arrays, we could strip all the string modifying functions from that type and make it immutable-ish. Then we have a library function to create one of those from a Base58CheckEncodedAddress, which is just its own thing with its own codec.

It's typescript so you can of course just cast, but if you don't do that then you can accept a base58check address from your user, validate it, convert it to a Base58EncodedAddressFromChecked, and have your entire app be confident that it hasn't been accidentally modified anywhere along the way.

You could do all of that without touching any existing APIs, and just cast it to Address when you want to pass it to something in our library. But we could also then make all the APIs that take an address also take a Base58EncodedAddressFromChecked because none of them need string modify functions and under the hood in JS runtime land they're both just plain strings anyway.

[steveluscher]

Related: solana-labs/solana#6970

[steveluscher]

I've only thought about this for two seconds, but my initial thoughts are:

• This seems to really only matter at the outer edge, where humans can make mistakes. That is to say I don't think any of our APIs should stop taking Address, but we could consider at most to offer codecs that let you commute between Address and AddressWithErrorCorrection, as has been suggested.
• I think the opaque type should be called AddressWithErrorCorrection
• We'll need a version prefix for Solana addresses. How do we even go about standardizing a version prefix? https://en.bitcoin.it/wiki/Base58Check_encoding#Version_bytes
• I can't remember, can codecs be async? They would need to be because Base58Check requires that you crypto.subtle.digest('sha256', ...) a bunch of times.