Remove Author and Signature fields from Record #15
Conversation
pb/record.proto
Outdated
| // Time the record was received, set by receiver | ||
| optional string timeReceived = 5; | ||
| optional string timeReceived = 3; |
There was a problem hiding this comment.
please don't change the field id, it is backwards incompatible.
There was a problem hiding this comment.
(although I'm pretty sure that's technically a harmless change.)
But I agree, don't change this.
There was a problem hiding this comment.
Nice catch.. please take a look at the diff of record.pb.go. The protoc compiler I'm using has added in a field called Descriptor. I'm not sure what that is so just want to make sure it's ok
pb/record.proto
Outdated
| optional string author = 3; | ||
|
|
||
| // A PKI signature for the key+value+author | ||
| optional bytes signature = 4; |
There was a problem hiding this comment.
We should probably leave a comment reserving these fields.
|
i think @whyrusleeping should chime in here too, that's a pretty drastic change in behaviour and i'd like to know if we'll break anything by suddenly removing signatures and their validation. |
|
@vyzo I've talked with @whyrusleeping offline but yes, I'd like him to give a final signoff. This shouldn't break anything as we only validate signatures iff they're present but still... After talking about this with Juan again (and again), we're pretty sure this signature came about as an early attempt at IPRS. Unfortunately, when we added support for multiple IPNS keys, we kept on signing the DHT records with the wrong key so trying to use this for IPRS at this point wouldn't work all that well... Personally, I'd almost like to just introduce IPRS as separate path. For the "backwards compat period", ipfs nodes would publish/retrieve both the IPRS and old records. After that compatibility period, they'd stop retrieving the old records. With request pipelining, this shouldn't be that expensive and allows us to start over. Anyways, the way we do signatures on these records and IPNS records is kind of broken and is only secure by happy coincidence (ipfs/notes#249). |
|
Yeah, I'm comfortable moving forward with this. The signatures on the dht records themselves are pretty useless all things considered, and as @Stebalien it might have been an early (failed) attempt at IPRS |
|
LGTM. Thanks @dirkmc! |
|
Glad to be of help |
|
@dirkmc I'd still like to use something like the |
No description provided.