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

feat: add coin argument to get_escrow_account method #986

Closed
wants to merge 2 commits into from

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented Nov 24, 2023

To support blockchains and other systems where escrow account depends
on the coin being transferred (in addition to port and channel), add
coin argument to get_escrow_account.

Closes: #985


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests.
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

To support blockchains and other systems where escrow account depends on
the coin being transferred (in addition to port and channel), add `coin`
argument to `get_escrow_account`.

Closes: cosmos#985
Copy link

codecov bot commented Nov 24, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (8f2ddfe) 70.76% compared to head (d5baa3a) 70.77%.

Files Patch % Lines
ibc-apps/ics20-transfer/src/handler/mod.rs 0.00% 2 Missing ⚠️
...-apps/ics20-transfer/src/handler/on_recv_packet.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #986   +/-   ##
=======================================
  Coverage   70.76%   70.77%           
=======================================
  Files         178      178           
  Lines       17943    17944    +1     
=======================================
+ Hits        12698    12699    +1     
  Misses       5245     5245           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Farhad-Shabani
Copy link
Member

Hey @mina86, I somehow missed to update you on this PR. This is connected with PR #836 and it’s on our radar already. I’ll keep you posted as soon as we're out of there.

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

Successfully merging this pull request may close these issues.

Support escrow accounts which depend on coin
3 participants