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

Datastore Key conversion mixes namespaces prefix #869

Closed
dennis-tra opened this issue Aug 22, 2023 · 1 comment
Closed

Datastore Key conversion mixes namespaces prefix #869

dennis-tra opened this issue Aug 22, 2023 · 1 comment
Assignees
Labels
v2 All issues related to the v2 rewrite

Comments

@dennis-tra
Copy link
Contributor

When we receive a PUT_VALUE RPC from a remote peer, the key will contain a namespace prefix (e.g., pk or ipns) followed by a binary key. In the case of IPNS the key follows the spec:

Key format: /ipns/BINARY_ID

This is in line with how the IPNS key gets generated here:

func (n Name) RoutingKey() []byte {
	var buffer bytes.Buffer
	buffer.WriteString(NamespacePrefix)
	buffer.Write(n.src) // Note: we append raw multihash bytes (no multibase)
	return buffer.Bytes()
}

NamespacePrefix is set to /ipns/. In the handlePutValue method the key parameter will be set to the above /ipns/BINARY_ID format. Later in that handler we're generating the datastore key via convertToDsKey(rec.GetKey()):

func convertToDsKey(s []byte) ds.Key {
	return ds.NewKey(base32.RawStdEncoding.EncodeToString(s))
}

This means we're taking the bytes of the string /ipns/BINARY_ID and encode them as base32. This means we're losing the capability of proper namespacing. The /ipns prefix will still be the same in base32 for all keys but still this seems like a coincidence. In the ProviderManager, we're properly encoding each component separately here.

Changing this key format will be incompatible with any persistent stores out there. We'd need to support both types of keys (everything base32 encoded and only BINARY_ID encoded) for some time.

With go-libp2p-kad-dht v2 we could take the liberty to introduce a breaking change here.

@dennis-tra dennis-tra added the v2 All issues related to the v2 rewrite label Sep 19, 2023
@dennis-tra dennis-tra self-assigned this Sep 19, 2023
@iand
Copy link

iand commented Oct 2, 2023

Migrated to probe-lab/zikade#25

@iand iand closed this as completed Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 All issues related to the v2 rewrite
Projects
None yet
Development

No branches or pull requests

2 participants