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

Remove requirement for gentx to be only 1 msg #8954

Merged
merged 6 commits into from
Mar 24, 2021
Merged

Conversation

jackzampolin
Copy link
Member

No description provided.

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

Looks fine. It might make sense to add a flag for the amount of gentxs to avoid accidental errors.

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

untested ACK

@@ -134,9 +134,6 @@ func CollectTxs(cdc codec.JSONMarshaler, txJSONDecoder sdk.TxDecoder, moniker, g

// genesis transactions must be single-message
msgs := genTx.GetMsgs()
if len(msgs) != 1 {
return appGenTxs, persistentPeers, errors.New("each genesis transaction must provide a single genesis message")
}

// TODO abstract out staking message validation back to staking
msg := msgs[0].(*stakingtypes.MsgCreateValidator)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see we're casting it to CreateValidator, shouldn't we change this if we want to support different messages?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should at least check the type assertion if it is ok.

Copy link
Member

@tac0turtle tac0turtle Mar 23, 2021

Choose a reason for hiding this comment

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

Out of scope of this PR, lets open an issue

@tac0turtle tac0turtle merged commit 5476327 into master Mar 24, 2021
@tac0turtle tac0turtle deleted the jack/multi-gentx branch March 24, 2021 10:02
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 this pull request may close these issues.

5 participants