ProfileIDs exist in two forms in our codebase. First, as a raw byte string called profile.ID, which is inherited from libp2p. Secondly, as a base58 encoded string, useful for serialization and display.
Due to two separate but related issues, it's easy to make mistakes in how profileIDs are used.
1) Profile.ID is a type alias for peer.ID, which is an alias for string. This makes it possible to do this:
const myProfileID = "QmarGp4Dr1QJ67zj2YEq3JP1gWgPoD1kfAcoJw2Ka9Pazo"
...
func myCode() {
// f takes a profile.ID (a raw byte string)
f(myProfileID)
}
This call to f causes an implicit conversion to a raw byte array, whereas properly calling f should require a call to IDB58Decode. This mostly happens in test code, but is still an easy mistake to make.
2) Profile.ID defines a .String() method, which does not merely display the profile.ID, but encodes it into a base58 string. Since this method matches the Stringer interface, this is automatically called by fmt.Printf("%s"). This makes it difficult and confusing to inspect profile.IDs, or compare them to base58 encoded ids.
For (1), it would be better if profile.ID was defined as a []byte, since that's really how it is being used. However this isn't possible, as peer.ID is defined in libp2p, outside of our control. But, we can add a Validate method to profile.ID, that at least ensures that methods receiving a profile.ID are being called correctly.
For (2), a better approach is to have an Encode() method that makes it explict that a base58 string is being created. The existing String() method should be changed so that it more accurately represents the fact that the profile.ID is in its raw byte form.
Prior work: https://github.com/qri-io/qri/pull/1121
ProfileIDs exist in two forms in our codebase. First, as a raw byte string called
profile.ID, which is inherited from libp2p. Secondly, as a base58 encoded string, useful for serialization and display.Due to two separate but related issues, it's easy to make mistakes in how profileIDs are used.
1) Profile.ID is a type alias for peer.ID, which is an alias for string. This makes it possible to do this:
This call to
fcauses an implicit conversion to a raw byte array, whereas properly callingfshould require a call toIDB58Decode. This mostly happens in test code, but is still an easy mistake to make.2) Profile.ID defines a .String() method, which does not merely display the profile.ID, but encodes it into a base58 string. Since this method matches the
Stringerinterface, this is automatically called by fmt.Printf("%s"). This makes it difficult and confusing to inspect profile.IDs, or compare them to base58 encoded ids.For (1), it would be better if profile.ID was defined as a
[]byte, since that's really how it is being used. However this isn't possible, as peer.ID is defined in libp2p, outside of our control. But, we can add aValidatemethod to profile.ID, that at least ensures that methods receiving a profile.ID are being called correctly.For (2), a better approach is to have an
Encode()method that makes it explict that a base58 string is being created. The existingString()method should be changed so that it more accurately represents the fact that the profile.ID is in its raw byte form.