-
Notifications
You must be signed in to change notification settings - Fork 340
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(rollapp): missing rollappid validation on rollapp creation #712
fix(rollapp): missing rollappid validation on rollapp creation #712
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #712 +/- ##
==========================================
+ Coverage 30.64% 30.72% +0.07%
==========================================
Files 233 234 +1
Lines 32580 32603 +23
==========================================
+ Hits 9984 10017 +33
+ Misses 21031 21022 -9
+ Partials 1565 1564 -1 ☔ View full report in Codecov by Sentry. |
There's a decent amount of duping here dymension/x/rollapp/keeper/msg_server_create_rollapp.go Lines 33 to 49 in 7427c40
dymension/x/rollapp/types/message_create_rollapp.go Lines 48 to 63 in 76dc37a
It's not the end of the world but might be good to cleanup |
Co-authored-by: Daniel T <[email protected]>
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.
gj
return sdkerrors.Wrapf(ErrInvalidAddress, "invalid creator address (%s)", err) | ||
func (msg *MsgCreateRollapp) GetRollapp() Rollapp { | ||
// Build the genesis state from the genesis accounts | ||
var rollappGenesisState *RollappGenesisState |
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.
if genesis state remains nil
I think we would get an error in the genesis event later on:
https://github.com/dymensionxyz/dymension/blob/main/x/rollapp/keeper/keeper.go#L68
Not sure if that's expected, if not, we should instantiate rollappGenesisState
regardless of genesis accounts being empty or not.
Unless it's expected that for some reason we enforce genesis accounts not being empty. In that case, we should probably fail on validation or something.
Description
Closes #710
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleased
section inCHANGELOG.md
godoc
commentsSDK Checklist
map
time.Now()
sendCoin
and notSendCoins
Full security checklist here
----;
For Reviewer:
---;
After reviewer approval: