go/common/keyformat: Ensure consensus and runtime prefixes are unique

[peternose]

No description provided.

[kostko]

Could you maybe change this so that you can basically specify a namespace where it needs to be unique? Consensus looks fine, but runtime as it is currently doesn't make sense (e.g. history db keys and runtime transaction tree don't need to have unique prefixes -- those only need to be unique within that IO tree).

[peternose]

history db keys and runtime transaction tree don't need to have unique prefixes

Wasn't sure about that and wanted to simplify. Will add namespaces.

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (bfff729) 66.26% compared to head (191d346) 66.91%.
Report is 1 commits behind head on master.

Files	Patch %	Lines
go/common/grpc/service.go	0.00%	0 Missing and 1 partial ⚠️
go/consensus/api/transaction/transaction.go	0.00%	0 Missing and 1 partial ⚠️
go/upgrade/migrations/migrations.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5548      +/-   ##
==========================================
+ Coverage   66.26%   66.91%   +0.64%     
==========================================
  Files         539      539              
  Lines       56877    56883       +6     
==========================================
+ Hits        37691    38064     +373     
+ Misses      14760    14379     -381     
- Partials     4426     4440      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kostko]

This should not be consensus, this is another database for storing runtime and consensus state (both use the same but these keys here are internal). I don't think we should hardcode "runtime history" and all the database backend namespaces in the key format package though?

Could we have a notion of namespaces that are specified externally? E.g. something like:

// Somewhere we would define the namespace.
var Consensus = keyformat.NewNamespace()
// Then we can uset the namespace to create new key formats.
var fooFmt = Consensus.New(0x00, ...)

This (or similar) would make it more flexible to use in external packages without the need to change the keyformat package.

[peternose]

You are right. I reverted some changes and added namespace constructor (with a namespace name to have clearer panic message). Now there are 4 namespaces: consensus, runtime db, runtime tx, and badger. Key formats defined in tests and in migration don't belong to any namespace (duplicates are allowed).

[kostko]

Looks good now, just two minor comment fixes.