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

SignBytes serialization for all governance transaction wasn't standardized #3443

Closed
4 tasks done
faboweb opened this issue Jan 30, 2019 · 5 comments · Fixed by #3446
Closed
4 tasks done

SignBytes serialization for all governance transaction wasn't standardized #3443

faboweb opened this issue Jan 30, 2019 · 5 comments · Fixed by #3446

Comments

@faboweb
Copy link
Contributor

faboweb commented Jan 30, 2019

Summary of Bug

See #3336

This apparently wasn't fixed for governance transactions.

Gaia commit: 30aebc1173ed6dae2848d1e845ec2f4e22750355

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

What makes you say this @faboweb? Just took a look at the latest develop and the commit you mentioned and all the GetSignBytes look correct to me:

e.g.

func (msg MsgSubmitProposal) GetSignBytes() []byte {
	bz := msgCdc.MustMarshalJSON(msg)
	return sdk.MustSortJSON(bz)
}

Each of the three total x/gov messages uses msgCdc.MustMarshalJSON(msg).

@alexanderbez alexanderbez added the S:needs more info This bug can't be addressed until more information is provided by the reporter. label Jan 30, 2019
@faboweb
Copy link
Contributor Author

faboweb commented Jan 30, 2019

Because I needed to re-add the hack to remove {type, value} from the message for the created signature to be accepted.

See: https://github.com/cosmos/voyager/blob/develop/app/src/renderer/scripts/wallet.js#L85
Needed on commit: 30aebc1173ed6dae2848d1e845ec2f4e22750355

@alexanderbez
Copy link
Contributor

@faboweb so sorry, I completely didn't realize x/gov does NOT register these types via an init call. Thank you. Trivial fix.

@alexanderbez alexanderbez self-assigned this Jan 30, 2019
@alexanderbez alexanderbez added T:Bug C:x/gov and removed S:needs more info This bug can't be addressed until more information is provided by the reporter. labels Jan 30, 2019
@alexanderbez alexanderbez added this to the v0.31.0 (Launch RC) milestone Jan 30, 2019
@faboweb
Copy link
Contributor Author

faboweb commented Jan 30, 2019

Thx for the effort.

@ZhiyuHelloWorld
Copy link

thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants