use b36 keys by default for keys and IPNS#7582
Conversation
core/commands/keystore.go
Outdated
| default: | ||
| enc, err := mbase.EncoderByName(formatLabel) | ||
| if err != nil { | ||
| panic("invalid IPNS key encoding") |
There was a problem hiding this comment.
Separating the validation function from the formatting function feels like a footgun. As we may accidentally call formatID without calling verifyIDFormatLabel and ending up with a panic.
How about creating a tiny struct to combine these two functions, one for creating the encoder and the other for running it.
|
@petar also need to: switch to b36 by default (and add add support for We should also add a sharness test showing the flag works for each of these commands. The IPNS over PubSub https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0183-namesys-pubsub.sh sharness tests may need some modifications anyway since they use the peer's id instead of We should make sure to rebase the ed25519 by default PR on this one to see if there's anything else we missed. |
538e1ae to
0991651
Compare
… of string representation to avoid encoding mismatches
add sharness tests for --ipns-base in name publish and subs commands
aschmahmann
left a comment
There was a problem hiding this comment.
A couple small nits, but looks like we're just about good to go here 😄. Thanks for cleaning this up it looks great.
| ipfsi 1 name resolve /ipns/$PEERID_0_BASE36 > name1 && | ||
| ipfsi 2 name resolve /ipns/$PEERID_0_BASE36 > name2 && |
There was a problem hiding this comment.
Unless I've missed them somewhere else we should add some sanity checks that IPNS over PubSub works (i.e. publishes + resolves) with b36, b58, and when publishing with one and resolving with the other.
It's probably fine, but given that IPNS over PubSub is experimental it's easier for bugs to creep in and get missed
There was a problem hiding this comment.
It turns out that to do this, we need to either: (a) add --ipns-base to 'ipfs add' or (b) expand the supported bases for 'ipfs cid'.
There was a problem hiding this comment.
Why? We could just do ipfs name resolve b58key and ipfs name resolve b36key and that should just work.
Looking at this file I don't think we're really testing IPNS over PubSub very well anyway since the DHT is likely very fast in this two node test resulting in the tests passing even if PubSub is sort of broken. We don't have to fix this issue in this PR though.
No description provided.