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

Nil unbonding time #473

Merged
merged 13 commits into from
Jun 26, 2023
Merged

Nil unbonding time #473

merged 13 commits into from
Jun 26, 2023

Conversation

joe-bowman
Copy link
Contributor

@joe-bowman joe-bowman commented Jun 22, 2023

1. Summary

Fixes # QCK-500

Permit nil completion times for unbondings and redelegations.

On receipt of nil redelegation completion time, remove the redelegation record.

Additional clean up of Acknowledgement and txResponse unmarshalling.

2.Type of change

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

3. Implementation details

In HandleBeginUnbonding and HandleBeginRedelegation (ibc_packet_handlers.go) we remove the check for a nil completion time.

In HandleBeginRedelegation, on receipt of nil redelegation completion time, remove the redelegation record.

Use built-in ibc-go methods to inspect failure/sucess of incoming acknowledgement.

Fixes relating to delegation updates on redelegation completion.

@codecov-commenter
Copy link

codecov-commenter commented Jun 22, 2023

Codecov Report

Merging #473 (ae90882) into develop (b277098) will increase coverage by 0.63%.
The diff coverage is 41.86%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #473      +/-   ##
===========================================
+ Coverage    55.64%   56.27%   +0.63%     
===========================================
  Files          165      165              
  Lines        12435    12378      -57     
===========================================
+ Hits          6919     6966      +47     
+ Misses        5002     4895     -107     
- Partials       514      517       +3     
Flag Coverage Δ
unittests 56.27% <41.86%> (+0.63%) ⬆️

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.80% <36.84%> (+6.91%) ⬆️
x/interchainstaking/types/ibc_packet.go 66.66% <75.00%> (+4.16%) ⬆️
x/interchainstaking/keeper/keeper.go 69.80% <100.00%> (ø)
x/interchainstaking/keeper/redemptions.go 74.28% <100.00%> (ø)

@joe-bowman
Copy link
Contributor Author

Need to lint and add test case.

@joe-bowman joe-bowman force-pushed the nil-unbonding-time branch from 2856908 to d6eb665 Compare June 23, 2023 13:45
@joe-bowman joe-bowman marked this pull request as draft June 23, 2023 13:53
@joe-bowman joe-bowman marked this pull request as ready for review June 23, 2023 20:08
@joe-bowman
Copy link
Contributor Author

@ajansari95 @aljo242 @muku314115 - ready for review now!

ajansari95
ajansari95 previously approved these changes Jun 26, 2023
Copy link
Contributor

@ajansari95 ajansari95 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@ajansari95 ajansari95 left a comment

Choose a reason for hiding this comment

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

lint failing

Copy link

@aljo242 aljo242 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@ajansari95 ajansari95 left a comment

Choose a reason for hiding this comment

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

lgtm

@joe-bowman joe-bowman merged commit a21fc4a into develop Jun 26, 2023
@joe-bowman joe-bowman deleted the nil-unbonding-time branch June 26, 2023 21:22
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