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

fix: track acknowledgements via a field #507

Merged
merged 6 commits into from
Jul 19, 2023
Merged

fix: track acknowledgements via a field #507

merged 6 commits into from
Jul 19, 2023

Conversation

ajansari95
Copy link
Contributor

1. Summary

Fixes # (issue)

  • better way to track the acknowledged UnDelegateMsg

2.Type of change

  • Bug fix (non-breaking change which fixes an issue)

3. Implementation details

4. How to test/use

  • cant think of a better way then writing e2e test cause all the mocked states are being tested
  • mocking the asynch flow wont help in covering anything substantial

5. Checklist

  • Does the Readme need to be updated?

6. Limitations (optional)

7. Future Work (optional)

@joe-bowman
Copy link
Contributor

Logic looks good. Test case as Al says, plus we will need an upgrade handler for any existing in process unbondings else they will never complete

@codecov-commenter
Copy link

Codecov Report

Merging #507 (cc01d3d) into develop (a40e473) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #507      +/-   ##
===========================================
+ Coverage    56.20%   56.29%   +0.09%     
===========================================
  Files          169      169              
  Lines        13185    13187       +2     
===========================================
+ Hits          7410     7424      +14     
+ Misses        5187     5173      -14     
- Partials       588      590       +2     
Flag Coverage Δ
unittests 56.29% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
x/interchainstaking/keeper/ibc_packet_handlers.go 41.70% <100.00%> (+1.31%) ⬆️

Copy link
Contributor

@joe-bowman joe-bowman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ajansari95 ajansari95 merged commit c0a0dd5 into develop Jul 19, 2023
@ajansari95 ajansari95 deleted the fix/ack branch July 19, 2023 18:47
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.

4 participants