mimblewimble / grin

Minimal implementation of the Mimblewimble protocol.
https://grin.mw/
Apache License 2.0
5.04k stars 991 forks source link

cleanup keychain is_floo param #3427

Open antiochp opened 4 years ago

antiochp commented 4 years ago

We pass is_floo around in a bunch of places related to the BIP32Hasher. This is used to differentiate between mainnet and floonet with some prefix bytes on the public and private keys (xpub/xprv style).

But we don't do this for usertesting in our tests. Confusingly we seem to set is_floo=true in various wallet related tests.

I think it makes sense to have specific prefix bytes for usertesting. And rather than passing is_floo taking advantage of global::get_chain_type()


Further investigation - the reason we cannot call global::get_chain_type() from the keychain crate is that keychain does not have a dependency on the core crate. And cannot without introducing a circular dependency.

Proposal: Introduce a new NetworkBytes struct that wraps the network/pub/network_priv bytes.

Pull the hardcoded "network bytes" out of the hasher and expose them in a more flexible way via -

This way we can access these via the configured chain_type via - global::network_bytes() and pass this into the various keychain fns rather than the limited is_floo bool.

antiochp commented 4 years ago

One observation here - bitcoin uses "xpub" and "xprv" as human readable prefixes on keys (public and private keys respectively). It appears we actually use some non-printable characters in our "network bytes" -

https://github.com/mimblewimble/grin/blob/6a012d7e5b1511eba4cd0ac33cfc528084e441cd/keychain/src/extkey_bip32.rs#L108-L122

0x08 being particularly interesting as it translates to BACKSPACE.

Actually this is all dependent (obviously) on how we map the bytes to characters...

Bitcoin uses Base58 so of course xpub and xprv are only applicable in this context.

We use Hex so everything is entirely different... it being non-trivial to display xpub in hex...