namespace: PrefixedData/PrefixedData8 are easy to misuse
- Dominant language
- Go
- Stars
- 122
- Forks
- 53
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 5
Description
The `PrefixedData` and `PrefixedData8` are both byte slices:
```go
// PrefixedData simply represents a slice of bytes which consists of
// a namespace.ID and raw data.
// The user has to guarantee that the bytes are valid namespace prefixed data.
// Go's type system does not allow enforcing the structure we want:
// [namespaceID, rawData ...], especially as this type does not expect any
// particular size for the namespace.
type PrefixedData []byte
// PrefixedData8 like PrefixedData is just a slice of bytes.
// It assumes that the slice it represents is at least 8 bytes.
// This assumption is not enforced by the type system though.
type PrefixedData8 []byte
```
The definitions are problematic, because (1) casting byte slices may violate the type invariants (as pointed out in the comments) (2) they're mutable (3) PrefixedData has no way to indicate the namespace identifier length. For an example where mutability is a problem, see
https://github.com/celestiaorg/nmt/blob/62742433f65a37daf889ee563b0ee117c83e9664/nmt.go#L253-L257
where it is not clear to the caller that the parameter `namespacedData` is retained by the `Push` method.
The missing namespace length is problematic for `validateAndExtractNamespace`,
https://github.com/celestiaorg/nmt/blob/62742433f65a37daf889ee563b0ee117c83e9664/nmt.go#L336-L340
because it can only validate that the namespace+data total length is at least as long as the expected namespace length.
Perhaps the definitions of `PrefixedData` and `PrefixedData8` are caused by wanting to support zero-copy and zero-allocation use, where the user generates `PrefixedData` values by slicing existing byte slices. If so, the behaviour of `Push` should document that argument values are retained.
Otherwise, I recommend changing the definition to hide the internal structure, and add validating constructors. There are several possibilites:
```go
type PrefixedData struct {
// namespace and data can be sliced from the same
// underlying []byte allocation.
namespace []byte
data []byte
}
func NewPrefixedData(namespace, data []byte) PrefixedData {
bytes := make([]byte, len(namespace)+len(data))
d := PrefixedData{
namespace: bytes[:len(namespace)],
data: bytes[len(namespace)],
}
copy(d.namespace, namespace)
copy(d.data, data)
return d
}
```
or equivalently
```go
type PrefixedData struct {
namespaceAndData []byte
namespaceLen int
}
```
@odeke-em
Contributor guide
Assessment
This issue has not been assessed yet.