Remove secio usage and cleanup exports #519
Conversation
| @@ -54,7 +48,7 @@ proc newStandardSwitch*(privKey = none(PrivateKey), | |||
| of SecureProtocol.Noise: | |||
| secureManagerInstances &= newNoise(rng, seckey).Secure | |||
| of SecureProtocol.Secio: | |||
| secureManagerInstances &= newSecio(rng, seckey).Secure | |||
| quit("Secio is deprecated!") # use of secio is unsafe | |||
There was a problem hiding this comment.
instead, might be able to mark the Secio enum value with {.deprecated.} - if not, might want to provide instructions for how to use secio anyway since it's needed to access old go implementations like ipfs etc
There was a problem hiding this comment.
AFAIK, secio has been removed everywhere, since it's been marked insecure
| multiaddress, crypto, lpstream, bufferstream | ||
| minprotobuf, varint,switch, peerid, peerinfo, | ||
| connection, multiaddress, crypto, lpstream, | ||
| bufferstream, bearssl, muxer, mplex, transport, |
There was a problem hiding this comment.
not sure internals like bearssl and varint necessarily should be exported, they're not part of the public api exposed in this module and pollute the global namespace with many ambiguous symbols - the rule is basically "anything that forms part of the public api should be exported"
There was a problem hiding this comment.
I'm fine with bearssl, but though I expose it since it's now used as the default RNG provider.
Varint seems like a good candidate to export tho, since it's used extensively across libp2p and can be useful for custom parsers and such?
There was a problem hiding this comment.
I'm fine with bearssl, but though I expose it since it's now used as the default RNG provider.
ok - if any bear type appears in public API needed by this module, then yes
Varint seems like a good candidate to export tho,
depends - ie libp2p consumes varint but presents a more sane API to to user that doesn't expose any varints, so I would tend towards no, for pollution reasons (ie if the user is using another varint library, of which there are plenty, they'll get into symbol trouble which is annoying and easily causes breakage
There was a problem hiding this comment.
depends - ie libp2p consumes varint but presents a more sane API to to user that doesn't expose any varints, so I would tend towards no, for pollution reasons (ie if the user is using another varint library, of which there are plenty, they'll get into symbol trouble which is annoying and easily causes breakage
OK, I don't disagree since it can be imported implicitly.
There was a problem hiding this comment.
ok - if any bear type appears in public API needed by this module, then yes
Yeah, we need BrHmacDrbgContext which seems to be part of bearssl and is returned by newRng().
Secio has been deprecated in nim-libp2p since [2021](#519) - farewell.
Note, as the title state, this removes secio usage and would
quitif it's attempted to be used withnewStandardSwitch- maybe quitting is a bit too drastic?