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

Missing message validation checks #233

Closed
3 tasks done
Tracked by #554
hu55a1n1 opened this issue Nov 9, 2022 · 1 comment · Fixed by #650
Closed
3 tasks done
Tracked by #554

Missing message validation checks #233

hu55a1n1 opened this issue Nov 9, 2022 · 1 comment · Fixed by #650
Assignees
Labels
A: breaking Admin: breaking change that may impact operators A: bug Admin: something isn't working
Milestone

Comments

@hu55a1n1
Copy link
Contributor

hu55a1n1 commented Nov 9, 2022

Summary of Bug

ibc-go performs additional checks on messages, for e.g. -> https://github.com/cosmos/ibc-go/blob/main/modules/core/04-channel/types/msgs.go, that are currently missing on our side.
I think the TryFrom<RawMsg> is a good place for these checks. Would be nice to make all domain message types correct by construction by having private fields and getters along with a ctor that only allows a user to create a valid message instance.

Version

v0.21.1

Acceptance Criteria

All missing validation checks are added.

Sub Issues

Tasks

Preview Give feedback
  1. A: breaking A: bug O: maintainability O: reliability
    Farhad-Shabani
  2. A: breaking A: bug
    Farhad-Shabani
  3. O: security
    Farhad-Shabani

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@hu55a1n1 hu55a1n1 added the A: bug Admin: something isn't working label Nov 9, 2022
@plafer plafer mentioned this issue Nov 9, 2022
7 tasks
@github-project-automation github-project-automation bot moved this to 📥 To Do in ibc-rs Feb 2, 2023
@Farhad-Shabani Farhad-Shabani modified the milestone: Fix known bugs and issues Feb 3, 2023
@Farhad-Shabani Farhad-Shabani self-assigned this Mar 28, 2023
@plafer
Copy link
Contributor

plafer commented Apr 3, 2023

Note: we are also missing some checks for tendermint's Misbehaviour.

This one is especially important.

The ValidateBasic() checks are meant to be executed in tendermint's checkTx; we don't really have that right now. I need to look into this more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: breaking Admin: breaking change that may impact operators A: bug Admin: something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants