-
Notifications
You must be signed in to change notification settings - Fork 409
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: allow no admin in instantiation through proposal #828
fix: allow no admin in instantiation through proposal #828
Conversation
Codecov Report
@@ Coverage Diff @@
## main #828 +/- ##
==========================================
+ Coverage 59.28% 59.36% +0.07%
==========================================
Files 51 51
Lines 5883 5884 +1
==========================================
+ Hits 3488 3493 +5
+ Misses 2143 2141 -2
+ Partials 252 250 -2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding and fixing this 🐛 !
LGTM 💯
if err != nil { | ||
return sdkerrors.Wrap(err, "admin") | ||
var adminAddr sdk.AccAddress | ||
if p.Admin != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 this was a bug
@@ -123,6 +123,80 @@ func TestInstantiateProposal(t *testing.T) { | |||
require.NotEmpty(t, em.Events()[2].Attributes[0]) | |||
} | |||
|
|||
func TestInstantiateProposal_NoAdmin(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for covering the additional cases! If you feel table tests are a better fit, then please feel free to refactor tests as well.
currently is not possible to instantiate without admin through governance proposal.