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 claims auto-release of vesting amounts #198

Merged
merged 6 commits into from
Dec 16, 2022

Conversation

maurolacy
Copy link
Contributor

@maurolacy maurolacy commented Dec 9, 2022

Fix claims auto-release of vesting amounts.

@maurolacy maurolacy self-assigned this Dec 9, 2022
@maurolacy maurolacy requested a review from ethanfrey December 9, 2022 18:45
@maurolacy maurolacy force-pushed the fix/auto-release-undelegate branch from 041a71a to 192f36b Compare December 9, 2022 18:56
@maurolacy maurolacy force-pushed the fix/auto-release-undelegate branch from a7f7ac6 to 9a95f35 Compare December 11, 2022 08:20
@maurolacy maurolacy force-pushed the fix/auto-release-undelegate branch from 9a95f35 to 074e49f Compare December 11, 2022 08:38
@maurolacy maurolacy added the bug Something isn't working label Dec 13, 2022
Copy link
Collaborator

@hashedone hashedone left a comment

Choose a reason for hiding this comment

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

Overall it's ok. I am unsure about just silencing up good clippys points on production code. Also, it looks like test cases are not unit, but testing flow, including communication with blockchain, which IMHO should be done with MT (I know there is a bit of work to do with custom messages, but for now, we are stockpiling this kind of usages for the sake of "we need to implement it at some point" - we are building up tech debt).

No acceptance because I want to understand the flow and reasoning to verify if logic makes sense. I will do it this evening.

Copy link
Collaborator

@hashedone hashedone left a comment

Choose a reason for hiding this comment

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

Besides my previous comments, it is good. Decide if you want to apply them.

Fix type_complexity warning
@maurolacy maurolacy merged commit 9342c64 into main Dec 16, 2022
@maurolacy maurolacy deleted the fix/auto-release-undelegate branch December 16, 2022 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants