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.

Sign up for GitHub

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

Jump to bottom

Make EscrowPacketFee private #1059

Closed
3 tasks
colin-axner opened this issue Mar 3, 2022 · 1 comment · Fixed by #1252
Closed
3 tasks

Make EscrowPacketFee private #1059

colin-axner opened this issue Mar 3, 2022 · 1 comment · Fixed by #1252
Assignees
@damiannolan
Labels
29-fee
Milestone
Fee middleware beta

Comments

@colin-axner
Copy link
Contributor

Summary

We should move the IsFeeEnabled check to the message server and make EscrowPacketFee private. This makes the message MsgPayPacketFee and MsgPayPacketFeeAsync the only entry points to this function.

I noticed this might be a better approach while adding an IsLocked() check. I think it makes sense to remove security checks from EscrowPacketFee and have that function simply escrow the packet fee.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner mentioned this issue Mar 3, 2022
Closed
9 tasks
@crodriguezvega crodriguezvega moved this to In progress in ibc-go Mar 4, 2022
@crodriguezvega crodriguezvega added this to the Fee Middleware milestone Mar 4, 2022
@crodriguezvega crodriguezvega moved this from In progress to Backlog in ibc-go Mar 7, 2022
@crodriguezvega crodriguezvega moved this from Backlog to Todo in ibc-go Apr 4, 2022
@seantking seantking added the needs discussion Issues that need discussion before they can be worked on label Apr 6, 2022
@damiannolan
Copy link
Member

Seems like EscrowPacketFee is used extensively throughout the tests. We may be able to work around it by setting fees in store manually, as that appears to be the general purpose in most test cases.

@seantking seantking moved this from Todo to On hold in ibc-go Apr 7, 2022
@damiannolan damiannolan self-assigned this Apr 12, 2022
@damiannolan damiannolan moved this from On hold to In progress in ibc-go Apr 12, 2022
@damiannolan damiannolan mentioned this issue Apr 13, 2022
Merged
9 tasks
@damiannolan damiannolan removed the needs discussion Issues that need discussion before they can be worked on label Apr 13, 2022
@damiannolan damiannolan moved this from In progress to In review in ibc-go Apr 13, 2022
Repository owner moved this from In review to Done in ibc-go Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@damiannolan damiannolan

Labels
29-fee
Projects
ibc-go
Archived in project
Milestone
Fee middleware beta
Development

Successfully merging a pull request may close this issue.

chore: make EscrowPacketFee private cosmos/ibc-go
4 participants
@damiannolan @crodriguezvega @seantking @colin-axner