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 calculation differs per msg type #3336

Closed
4 tasks
faboweb opened this issue Jan 21, 2019 · 4 comments · Fixed by #3347
Closed
4 tasks

SignBytes calculation differs per msg type #3336

faboweb opened this issue Jan 21, 2019 · 4 comments · Fixed by #3347
Assignees

Comments

@faboweb
Copy link
Contributor

faboweb commented Jan 21, 2019

Summary of Bug

Serializing bank.MsgSend and staking.Delegation differs. In bank.MsgSend the msg is serialized without type and value:

{"account_number":"0","chain_id":"local-testnet","fee":{"amount":[],"gas":"500000"},"memo":"","msgs":[{"inputs":[{"address":"cosmos1sn0f0phpv9rzkf0956uwu5ja88ramcv7u2dx2y","coins":[{"amount":"10","denom":"photino"}]}],"outputs":[{"address":"cosmos1sn0f0phpv9rzkf0956uwu5ja88ramcv7u2dx2y","coins":[{"amount":"10","denom":"photino"}]}]}],"sequence":"4"}

For staking.Delegation we need to leave type and value in:

{"account_number":"0","chain_id":"local-testnet","fee":{"amount":[],"gas":"500000"},"memo":"","msgs":[{"type":"cosmos-sdk/MsgDelegate","value":{"delegation":{"amount":"10","denom":"stake"},"delegator_addr":"cosmos1sn0f0phpv9rzkf0956uwu5ja88ramcv7u2dx2y","validator_addr":"cosmosvaloper1sn0f0phpv9rzkf0956uwu5ja88ramcv7e7enxh"}}],"sequence":"3"}

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@cwgoes cwgoes added the T:Bug label Jan 21, 2019
@cwgoes
Copy link
Contributor

cwgoes commented Jan 21, 2019

That's a bug, great catch.

type and value should be left in as replay-protection against different modules.

We ought to review this across all of our modules.

@alessio
Copy link
Contributor

alessio commented Jan 21, 2019

$ gaiacli tx send --from=foo --to=cosmos103ee72v6m096y2xq3rrxv5uex7pawh5y6uykt4 --amount=1stake --generate-only 
{"type":"auth/StdTx","value":{"msg":[{"type":"cosmos-sdk/Send","value":{"inputs":[{"address":"cosmos1570v2fq3twt0f0x02vhxpuzc9jc4yl30q2qned","coins":[{"denom":"stake","amount":"1"}]}],"outputs":[{"address":"cosmos103ee72v6m096y2xq3rrxv5uex7pawh5y6uykt4","coins":[{"denom":"stake","amount":"1"}]}]}}],"fee":{"amount":null,"gas":"200000"},"signatures":null,"memo":""}}

send seems to work just fine. Haven't tested delegate quite yet though

@faboweb
Copy link
Contributor Author

faboweb commented Jan 21, 2019

It's not about the generate_only it is about the signBytes you need to calculate before creating the signature of those signBytes.

@alexanderbez
Copy link
Contributor

Is it because GetSignBytes for MsgSend uses a custom inline struct not resisted with the codec?

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.

4 participants