Cosmos-sdk: Reserving error codes spaces is ridiculous

[sunnya97]

Namespace should be based somehow on module name (i.e. auth, bank, etc)

[cwgoes]

Agreed. Writing a module I have no idea what other modules clients of my module might want to use, so there's no way to ensure non-overlap. Even if some assertion were added in the SDK itself, clients using multiple modules with overlapping code spaces would have to modify the modules, which is unwieldy.

[cwgoes]

Error codes are passed through ABCI, so they must be integers. Relevant: https://github.com/tendermint/abci/issues/146

Is the goal to have client applications support particular modules (across applications), and recognize the error codes from those modules across a global namespace? That might be convenient, but then we really need error uniqueness across the global namespace (not just whatever modules are being used in an application), which seems difficult.

Possible to change ABCI spec to accept arbitrary strings for errors? That way modules could prefix error strings with the module name, and the strings could contain a bit of information for clients which don't implement full error code mappings.

[ebuchman]

I think the solution we came to is to use the tags system. The point of tags is to provide information back to the sender about what parts of the state machine were effected, so it could be used to namespace the errors. This way modules can all start from wherever, so long as the tags they return differentiate them.

So it means when we return error Results in the SDK we have to include the right tags.

[cwgoes]

Should there then be a common set of error codes, defined globally for the SDK, which modules may use (but modules may not define their own - they may only use tags)?

I guess we could use tags on top of the current system as long as we don't care that module error codes may overlap. Seems better to canonicalize if possible though.

[ebuchman]

I think we should do both.

Maybe the first 1000 error codes are reserved for the SDK itself.

Anything with an error code above 1000 implies that it uses tags to namespace itself.

[cwgoes]

Sounds reasonable.

Where are tags intended to be integrated? At the moment, Result has Tags []cmn.KVPair - https://github.com/cosmos/cosmos-sdk/blob/develop/types/result.go#L34 - but Error has no tags. Should they be added to the error type - https://github.com/cosmos/cosmos-sdk/blob/develop/types/errors.go#L140 - then passed to Result in error.Result() ? Then modules could change their error functions to return errors with tags, which shouldn't be too difficult.

Maybe we want to standardize particular tags, e.g. canBeRetried: boolean, identifier: string (unique), etc. - could be useful for clients.

Glad to implement this if you like.

[ebuchman]

We should probably add something to the Error that becomes the correct tag in error.Result.

I like the idea of standardizing tags but I think it might be too early to know exactly. Maybe we can start with a concrete proposal for how this should work ?

We really need an "SDK spec" to cover all the basics of how SDK apps work. This should be part of that.

[cwgoes]

This directory looks like the beginning of something like an "SDK spec", although it's a bit out-of-date - are you talking about developer-targeted documentation ("how do I write a module with X, Y, and Z features using the SDK") or something more like a glossary?

[ebuchman]

I think we need to differentiate "spec" from "docs", and we need both.

Let's write the spec for the way the Cosmos Hub uses the SDK. This includes everything about the multistore, the tx structures and encoding, the tx response components, etc. This stuff is the responsibility of the SDK team. But keep in mind it should be written independent of implementation detail - ie. it should serve as a guide for someone implementing the Cosmos Hub in another language, with or without a corresponding SDK, and it should serve as a definitive guide to auditors and folks trying to understand the Hub protocol.

We also need awesome docs for developers looking to use the SDK to build apps, though I think this might be more the responsibility of the Academy team. Part of this is a Glossary, for sure.

[ebuchman]

Had a long discussion with Jae about this. See his notes in the PR (https://github.com/cosmos/cosmos-sdk/pull/749) re why we shouldn't use tags.

The solution we've been discussing involves namespacing within the Code itself, say using the first 16 bits for the namespace, and the final 16 bits for the actual code.

Let's hold off on further work here until we flesh out a full scheme for what this should look like.

[jaekwon]

I made a concrete proposal here: https://github.com/cosmos/cosmos-sdk/issues/766

[cwgoes]

Closing in favor of https://github.com/cosmos/cosmos-sdk/issues/766