Use Base58Check where humans are involved

[t-nelson]

Problem

We use base58 without checksums where humans are in the loop. This is error prone

Proposed Solution

Enable bs58's "check" feature and use it

[garious]

@mvines, we should bump the priority on this one. Only checksums within the clients though. No need for those bits to go on the wire.

[mvines]

Another vote for this issue, Wolfgang recently screwed up his --trusted-validator argument and a checksum would have caught it

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[mvines]

Unstale-ing. It would be nice for somebody to champion a path here even though it could be quite disruptive. The fact that accidentally excluding the first or last character of a base58 address can produce a valid base58 address is an enormous footgun

[t-nelson]

The best option I've thought of is that we decouple address from pubkey and discern between them in FromStr with everyone's favorite discriminator, decoded length. Address then looks like

enum Address {
    Legacy(Pubkey),
    Checked {
        chaincode: u8,  // cluster/version discriminator
        pubkey: Pubkey,
        checksum: u32,
    },
}

impl Address {
   pub fn pubkey(&self) -> Pubkey {
   ...
   }
   ...
}

[t-nelson]

Shower thought: Add an optional, detached checksum to the address string encoding. Something like

{BASE58_ADDRESS}[{SEP}{BASE58_CHECKSUM}]

Pros:

• Typo/copypasta resistance
• No changes to accounts storage as the checksum is discarded after parsing
• Optional. Adoption can be incremental
• Recoverable from existing addresses. Keep your vanity addresses

Cons:

• Clunky compared to other chains
• We'll have to pick a good SEP to balance recognition with copypasta-compat
• Herding the exchange/wallet cats to adopt

[mvines]

not bad, a SEP of _ perhaps.

[t-nelson]

_ seems to behave well (doesn't break double-click select) in the fields I've tested so far

[mvines]

oh, 0 could be a SEP too. Maybe that's better since it'll just blend in with the rest of the address.

[t-nelson]

oh, 0 could be a SEP too. Maybe that's better since it'll just blend in with the rest of the address.

I was thinking we want checked vs raw to be easily discernible at a glance. Especially during the transitional phase. Maybe not though?

[mvines]

I feel that telling users to manually strip the checksum to pass to a legacy address seems like a recipe for fund loss, making it easy to visually separate the SEP (like _ does) makes this manual editing tempting.

Instead during the transition I imagine checksum-compatible wallets would present both addresses, maybe burying the non-checksum legacy address behind an extra click, so the user in both cases always copy/pastes the entire thing without edits

[t-nelson]

I feel that telling users to manually strip the checksum to pass to a legacy address seems like a recipe for fund loss, making it easy to visually separate the SEP (like _ does) makes this manual editing tempting.

Instead during the transition I imagine checksum-compatible wallets would present both addresses, maybe burying the non-checksum legacy address behind an extra click, so the user in both cases always copy/pastes the entire thing without edits

Makes sense to optimize for the utopian future and just let the rocky transition be rocky I suppose.

[t-nelson]

@CriesofCarrots @sconybeare @armaniferrante @vidorge any thoughts on detached address checksums as proposed here

[garious]

When wallets show the first and last characters, should they show the last characters of the pubkey or its checksum? First N characters and full checksum seems good to me.

[t-nelson]

When wallets show the first and last characters, should they show the last characters of the pubkey or its checksum? First N characters and full checksum seems good to me.

I'd agree once adoption is complete. Might need to show both with a mouseover or something in the interim in case the user is comparing a service that has adopted to one who hasn't

[garious]

Since this feature requires wallet ecosystem coordination, what if instead of this feature, we add cancelable payments. So the sender can cancel up until the moment the recipient accepts. If there's a typo in the address, odds someone is sitting on that private key are nearly zero, so you'd just cancel.

[t-nelson]

Pretty low odds we're going to get exchange buy in for something like that. They're very resistant to anything that doesn't fit their chain integration interfaces. The UX deviation is going to be a support burden as well. There's evidence of this in Serum's "settle trade" operation, which still brings us confused users weekly.

[kubanemil]

I think this issue can be closed