go-libp2p-record
go-libp2p-record copied to clipboard
Provide a nice record type
Motivation:
- Nicer types (e.g., typed author). Working with types generated by the protobuf package isn't exactly fun.
- Impossible to do stupid things like trust the author field when there's no valid signature.
- Avoids exposing other unrelated fields rid of any other unrelated fields (e.g., TimeReceived which only exists for local record keeping).
- Ensure we only store the fields we care about (prevent the DHT from being used as a general purpose KV).
Note: this is now about exposing a type to use elsewhere. I'd like to rename ValidationRecord
to simply Record
and use it outside of this library instead of passing around the protobuf.
However, this is kind of a low priority.
It looks like the same protobuf message is used on the wire, and in storage. But the extra field timeReceived
is only used in storage. Would a change to the binary layout be unworkable due to backwards compatibility?
Ideally I think we would want this:
message Record {
// The key that references this record
bytes key = 1;
// The actual value this record is storing
bytes value = 2;
// These field numbers were used previously.
reserved 3 to 5;
}
import "google/protobuf/timestamp.proto";
message StoredRecord {
Record record = 1;
google.protobuf.Timestamp received = 2;
}
But I think this causes a binary incompatibility with existing data stores. An alternate that should be binary compatible would be to fork Record:
message Record {
// The key that references this record
bytes key = 1;
// The actual value this record is storing
bytes value = 2;
// These field numbers were used previously.
reserved 3 to 5;
}
message StoredRecord {
bytes key = 1;
bytes value = 2;
reserved 3, 4;
string timeReceived = 5;
}
Really, we can go either way. The former is more complex because we'd need to get the JS team to agree and then we'd need to create a datastore migration but I won't object if you want to put in the work.
I'm not sure about the utility of the latter. It's not really removing the hack, just kind of hiding it under the covers. But I could be convinced.
@vasco-santos which of these options would suit the JS implementation? I notice https://github.com/libp2p/js-libp2p-record/blob/master/src/record.proto.js which appears to duplicate the protobuf definition in this repo. Would it be better that all implementations share a mutual proto definition for the record sent over the wire? The same would go for the datastore, if that's intended to be compatible between implementations (it doesn't appear so?).
Even if the first isn't worthwhile, the second option (splitting the record) will still act as some type-enforcement that the 2 records should not be interchangeable.
I'm not of the most appropriate place to discuss this.
I'm not of the most appropriate place to discuss this.
If we're not really touching the spec, here is probably fine. Note: if you need to get someone's attention, IRC is probably the place to go.