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. |
petar
force-pushed
the
petar/keys
branch
2 times, most recently
from
538e1ae
to
0991651
Compare
… of string representation to avoid encoding mismatches
add sharness tests for --ipns-base in name publish and subs commands
| 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.