Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentinel Errors #9598

Closed
4 tasks
jhernandezb opened this issue Jun 28, 2021 · 3 comments · Fixed by #9990
Closed
4 tasks

Sentinel Errors #9598

jhernandezb opened this issue Jun 28, 2021 · 3 comments · Fixed by #9990

Comments

@jhernandezb
Copy link

Summary of Bug

sdk errors package allows new modules to register their own sentinel errors through sdkerrors.Register(ModuleName, Code, Message) but documentation around how to use it is inconsistent since all sdk modules start with code 2 without explaining why it should.

I found a while back that if you use code 1 for a sentinel error while building a module there is nothing that prevents from doing it, but if the module ever returns such error when you submit a tx that will trigger it the raw_log message its replaced, because its a reserved code for ABCI(?)

Example:
{"height":"104","txhash":"B4D2A9C80ADB2C6147FBD9336423A67CD65E9D072B511DCF085D3EAC9BD28FAC","codespace":"sdkerrordemo","code":1,"data":"","raw_log":"internal","logs":[],"info":"","gas_wanted":"200000","gas_used":"44695","tx":null,"timestamp":""}

Either this should have better docs or sdkerrors.Register should prevent it (breaking change)

Documentation
https://docs.cosmos.network/v0.42/building-modules/errors.html#registration

Modules using CodeError 1
https://github.com/tendermint/liquidity/blob/develop/x/liquidity/types/errors.go#L9
https://github.com/desmos-labs/desmos/blob/master/x/profiles/types/error.go#L10 (other modules from same codebase)

Version

v0.42+

Steps to Reproduce

  • create a sentinel error with code 1
  • submit a tx that triggers this error
  • query the tx and see the message
    NOTE: This doesn't happens during tests since the expected error from the msg handlers its the one expected.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

@jhernandezb what exactly are you looking for concretely here? The docs state 1 is reserved and that you cannot use it.

@jhernandezb
Copy link
Author

not really sure if thats been there but I missed it but the code snippet is wrong and like me I think that's what people might see first as an example see the the links of modules I posted.

Probably this is the wrong repo for docs issues

@alexanderbez
Copy link
Contributor

Good catch, the code snippet should be fixed. But the use of not allowing 1 has been there for a while.

/cc @alessio

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants