-
Notifications
You must be signed in to change notification settings - Fork 586
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
chore (api)!: remove capabilities from SendPacket #7213
Conversation
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.
Woo! Great work! 🎉
For reference to other viewers. While we have not adjusted the SendPacket
to go through an rpc on core IBC, this is still safe to remove as we rely on the security principles guarded by the bank keeper. Modules which can access the go api must not be malicious (otherwise they can send coins from any account to any other account). The same principle is being applied by removing this code. It was always true that a malicious module could bypass the channel capability anyways (by embedding the scoped keeper of another module)
We will need to add a changelog + migration docs, but happy to do that at the end. Maybe we can make a note on the remove capabilities issue |
It's probably better to have this one merged #7089 before we merge any removal of capabilities, to avoid merge conflicts on that PR |
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.
Nice 🔴 diffs!
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.
🪓 love the diff!
I'd say go for the merge and dont let other PR block you (currently reviewing it that and trying to find some issue with tests, I'll take care of merge conflicts there) |
Quality Gate passed for 'ibc-go'Issues Measures |
* chore (api)\!: remove capabilities from SendPacket * add changelog + note to add migration documentation --------- Co-authored-by: Carlos Rodriguez <[email protected]>
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.