Integrate coin-type configuration #338
Conversation
interchain.go
Outdated
| // NOTE: this is hardcoded to the cosmos coin type. | ||
| // In the future, we may need to get the coin type from the chain config. | ||
| const coinType = types.CoinType | ||
| coinTypeU64, err := strconv.ParseUint(config.CoinType, 10, 32) |
There was a problem hiding this comment.
The variable is named coinTypeU64 but you are telling ParseUInt to use 32bits, and then on L451 there's a typecast to uint32. It looks like the coin type is a uint32 so I'd say this parse call is fine but the type cast could be eliminated.
Also the name could be shortened to just coinType for brevity
There was a problem hiding this comment.
ParseUInt outputs a typed uint64 integer whether you specify 32bits or 64bits.
That hd.CreateHdPath function takes a uint32. This is the reason for the type cast.
Regardless, for readability, I like what you're saying here and can do the type cast directly in the hd.CreateHdPath function (L456) with the renaming suggestion.
There was a problem hiding this comment.
Well TIL! Disregard my comment then 😛
| typ := reflect.TypeOf(c) | ||
| f, _ := typ.FieldByName("CoinType") | ||
| coinType := f.Tag.Get("default") |
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, I thought so too! After testing, if the value is empty, it takes on "0" which is Bitcoin's coin-type.
If there is a better way to go about this, I'm happy to learn.
There was a problem hiding this comment.
Seems very counterintuitive. I'll go ahead and approve, if there is a better way to do this we can always circle back up on it. Good job!
* integrate coin-type configuration * account for cointype 0 * fix types * add comment * handle feedback
The PR adds the ability to specify Coin Types when creating or restoring keys for chains.
The coinType is invoked in the chainConfig but defaults to "118" (Cosmos SDK default) if the key:value pair is left blank.