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

fix : removed the unnecessary usage of ErrInvalidMsg #1311

Merged
merged 3 commits into from
Apr 4, 2023

Conversation

vishal-kanna
Copy link
Contributor

@vishal-kanna vishal-kanna commented Apr 3, 2023

Part of #1310

@vishal-kanna vishal-kanna requested a review from alpe as a code owner April 3, 2023 10:17
@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

Merging #1311 (7da4d9c) into main (887ee0e) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1311      +/-   ##
==========================================
+ Coverage   63.44%   63.47%   +0.02%     
==========================================
  Files          56       56              
  Lines        6971     6971              
==========================================
+ Hits         4423     4425       +2     
+ Misses       2219     2218       -1     
+ Partials      329      328       -1     
Impacted Files Coverage Δ
x/wasm/types/tx.go 46.52% <100.00%> (ø)

... and 1 file with indirect coverage changes

@pinosu
Copy link
Contributor

pinosu commented Apr 3, 2023

Thanks for your pr!
As part of #1310 , I don't think we need a new error type for each possible error. Probably the more generic ErrInvalid could be used.
In this case it could be something like:
if strings.EqualFold(msg.Sender, msg.NewAdmin) { return errorsmod.Wrap(ErrInvalid, "new admin is the same as the old") }
It would be nice to check if there are other places where ErrInvalidMsg usage is not proper.

@vishal-kanna vishal-kanna changed the title feat:added a new error type fix : removed the unnecessary usage of ErrInvalidMsg Apr 4, 2023
Copy link
Contributor

@pinosu pinosu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! 💯

@pinosu pinosu added the backport/v0.3x Backport patches to sdk45 release branch label Apr 4, 2023
@pinosu pinosu merged commit 4c906d5 into CosmWasm:main Apr 4, 2023
mergify bot pushed a commit that referenced this pull request Apr 4, 2023
* feat:added a new error type

* feat:added a new error type

* fix: removed unnecessary usage of error type ErrInvalidMsg

---------

Co-authored-by: Vishal <kannavish329@gamil.com>
(cherry picked from commit 4c906d5)

# Conflicts:
#	x/wasm/types/tx.go
pinosu added a commit that referenced this pull request Apr 4, 2023
…#1317)

* fix : removed the unnecessary usage of  ErrInvalidMsg  (#1311)

* feat:added a new error type

* feat:added a new error type

* fix: removed unnecessary usage of error type ErrInvalidMsg

---------

Co-authored-by: Vishal <kannavish329@gamil.com>
(cherry picked from commit 4c906d5)

# Conflicts:
#	x/wasm/types/tx.go

* fix conflicts

---------

Co-authored-by: vishal-kanna <71565171+vishal-kanna@users.noreply.github.com>
Co-authored-by: Pino' Surace <pino.surace@live.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.3x Backport patches to sdk45 release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants