regen-ledger icon indicating copy to clipboard operation
regen-ledger copied to clipboard

Update utility functions to check empty and return consistent errors

Open ryanchristo opened this issue 2 years ago • 3 comments

Summary

Ref: https://github.com/regen-network/regen-ledger/pull/1233#discussion_r916139233

Currently we check for an empty string inside basic message validation for credit class, project id, batch denom, etc. We should check for empty string within the validation functions defined in utils.go. We should also make sure we are returning consistent errors for these validation functions, i.e. all should include the expected format and consistent sdk errors.

Problem Definition

We are duplicating a lot of empty string checks in message validation and returning inconsistent errors and errors that could provide more information about the expected format.

Proposal

Add empty string checks for validation functions in utils.go (for core, basket, and marketplace) and remove the empty string checks in basic message validation. Also make sure all errors returned include the expected format within an sdk error.


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned

ryanchristo avatar Jul 07 '22 18:07 ryanchristo

@ryanchristo i'm interested in working on this one if you agree it could be a good one to work on.

wgwz avatar Jul 18 '22 18:07 wgwz

Yea, I think this would be a good one. I'll assign to you and feel free to reach out with any questions.

ryanchristo avatar Jul 18 '22 18:07 ryanchristo

Hey @ryanchristo I have opened up a draft PR that is a start on this. I think I'm headed in the right direction. So far I started with the core module and refactored the validation functions within core/utils.go. Let me know what you think, thanks!

wgwz avatar Jul 29 '22 03:07 wgwz

@wgwz thanks again for tackling the largest chunk of work in #1317! I think there are only a couple left for basket and it would be nice to get those updated now that we've added state validation checks that use the same utils. Unless you've started, do you mind if I finish off those changes?

ryanchristo avatar Aug 23 '22 15:08 ryanchristo

@ryanchristo np, please feel free, i'll be glad to help with review!

wgwz avatar Aug 23 '22 15:08 wgwz