Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDFromString and Pretty are not compatible #1038

Closed
rkapka opened this issue Jan 13, 2021 · 4 comments
Closed

IDFromString and Pretty are not compatible #1038

rkapka opened this issue Jan 13, 2021 · 4 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked

Comments

@rkapka
Copy link

rkapka commented Jan 13, 2021

Assume that id is a valid peer.ID. peer.IDFromString(id.Pretty()) generates an error, even though I'm just converting the multihash back and forth.

@rkapka rkapka added the kind/bug A bug in existing code (including security flaws) label Jan 13, 2021
@rkapka
Copy link
Author

rkapka commented Jan 13, 2021

Maybe there is a bug inside IDFromString?

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Jan 13, 2021

@rkapka

The IDFromString simply takes the peerID type converted to a string and not the string representation of the peerID.
Here's a sample example that works:

import (
	"fmt"

	"github.com/libp2p/go-libp2p-core/peer"
	"github.com/libp2p/go-libp2p-peerstore/test"
)

func main() {
	pid := test.GeneratePeerIDs(1)[0]
	id, err := peer.IDFromString(string(pid))
	if err != nil {
		panic(err)
	}
	fmt.Println(id)
	
// Output
QmVvtzcZgCkMnSFf2dnrBPXrWuNFWNM9J3MpZQCvWPuVZf

I agree the API isn't intuitive here :/.

@Stebalien @raulk Any reason the API was designed this way ?

@aarshkshah1992 aarshkshah1992 self-assigned this Jan 13, 2021
@Stebalien
Copy link
Member

I'm not sure. We should probably rename IDFromString to IDFromByteString (keeping but deprecating the former).

@BigLep BigLep added the status/ready Ready to be worked label Mar 22, 2021
@marten-seemann
Copy link
Contributor

Indeed, this was a footgun, which is why we first deprecated and then removed it: libp2p/go-libp2p-core#274.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

5 participants