[DO NOT MERGE] poc: allow pure V2 records

[lidel]

This PR includes changes from #41 so it is easier to see the scope of overall diff.

This PR

This is a PoC that shows minimal set of changes to pass Validate check when "pure V2" IPNS records which have only two protobuf fields: IpnsEntry.Data and IpnsEntry.SignatureV2

• Create still creates V1+V2 for backward-compatibility
• Validate only cares about V2 fields (IpnsEntry.Data and IpnsEntry.SignatureV2) and nothing more.

Why?

Right now "V1+V2" in-between is required for successful validation: even tho SignatureV1 is not used, and we only check SignatureV2, some fields must have the same value in IpnsEntry protobuf AND dag-cbor in IpnsEntry.Data

If we ever want to fully ditch legacy of V1, we would have to start with removing the requirement of duplicated fields, allowing lean records.

Discovered Problems 💢

• This baloons the scope of refactors that need to happen.
• JS-IPFS, Kubo and go-namesys also needs to be refactored 🙈
  • go-namesys still uses V1 fields all over the place:
    https://github.com/ipfs/go-namesys/blob/1bf7d3d9cbe8f988b232b92288b24d25add85a00/routing.go#L118-L133
    Same for JS side of things.
• DAG-CBOR is deserialized multiple times
  • Sure, we can hack things around within this library, but then go-namesys has to deserialize data again.
• All third-party software using this library will break 💀
  • If we remove requirement of duplicating values between IpnsEntry (protobuf) and IpnsEntry.data (dag-cbor) most likely everything that uses IpnsEntry will effectively break.
  • We will have bad time as long the API is based around IpnsEntry protobuf. This needs to be replaced with higher level IpnsRecord API.

Way forward may exist, but will take significant effort

I don't think this can be done half-way. Will end up in hacky code, leaky abstractions and be very error prone.

If we want to support pure V2, then we need to introduce new protobuf with only data and signature (V2) fields and refactor all involved libraries, in both GO and JS. Instead of returning protobuf object, we should have IpnsRecord struct with funcs that provide easy access to DAG-CBOR fields.

This is significant effort which I don't have bandwidth to do atm, so parking this as a draft for now. #41 must be enough for now.

I will document the situation in ipfs/specs#319 so someone else will be able to pick this up with better clarity.

TODO

• Create IpnsRecord abstraction that hides V1 and V2 details
• ensure Create produces V1+V2 by default (so we don't break legacy consumers)
• ensure Validate requires presence of V1 fields presence only if signatureV1 is present, and fails when signatureV1 is missing, but value is present in protobuf – this ensures we move towards DAG-CBOR-only future.
• allow opt-in V2-only creation (just data and signatureV2 fields)

[aschmahmann]

Agreed, we cannot really allow pure v2 records without either doing some hacky/difficult to reason about things or resolving the TODO around switching to passing around IPNS record objects that are not tied to the protobuf itself

go-ipns/ipns.go

Lines 143 to 145 in 1df1d60

	// TODO: If we switch from pb.IpnsEntry to a more generic IpnsRecord type then perhaps we should only check
	// this if there is no v1 signature. In the meanwhile this helps avoid some potential rough edges around people
	// checking the entry fields instead of doing CBOR decoding everywhere.

.

The refactor here is kind of painful and is a breaking change. Doing this seems generally helpful anyway to help with:

1. Decoupling from the particular protobuf code generator ... IMO a general lesson is to not expose code-generation to external users at all and just wrap them in a struct or interface that your library will commit to, otherwise you'll find yourself with problems when the codegen tools improve.
2. It'll help users who want to use the extensible CBOR fields without repeated CBOR decoding not need to make up their own custom caching structs
3. It could help us differentiate between validated and unvalidated records through the codebase to make it easier for devs to not under or over validate the records
4. Resolving some of the awkwardness around record creating and parsing that came with slow evolution over time (e.g. separate IPNS record creation and public key embedding functions, a GetEOL function to do parsing, untyped TTL data, etc.)

I wonder if it would be reasonable to start here by making a new struct (or interface) IpnsRecord that can be created and/or validated from the IPNS record bytes and start pushing that to consumers while deprecating the existing functions before ultimately removing them. Although perhaps for more sensitive/security related issues like signature validation it's better to make a larger breaking change and deal with the upstream breakages.

That being said it's a pile of work to save us some bytes and complexity. It'd be nice to do it sooner than later since ecosystem changes to deprecate protocols like this take time, however there's lots of other important work to do even in the IPNS space itself so totally put the time wherever seems appropriate. Mostly flagging the above so the issue is known and can be tracked here and in the specs PR.

[lidel]

I've been thinking about practical ramifications of this PR, and I am concerned about breaking legacy consumer nodes. If there is software that uses IPNS for fetching updates, and the IPNS publisher node suddenly switches to pure V2 CBOR, legacy clients will not be able to validate these records and will be stuck on old version.

Pulling the rug like that is not good. I'd argue using IPNS for signaling updates is its main purpose.

I agree we should refactor this library, create IpnsRecord abstraction, and refactor everything downstream so working with V2 and extensible CBOR is easier. The only caveat is the default type of records created by this library.

• I believe Create should by default still create V1+V2, to maximize interop with legacy nodes.
• Creating V2-only records should be an opt-in.

I've added "TODO" to this PR and "backward compatibility" policy section to the spec PR: ipfs/specs#319 (comment) (ipfs/specs@b926e8c) to ensure we don't break systems that use IPNS for signaling updates.

[hacdias]

Tracked in ipfs/specs#376.