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

[NTRN-483] fix: claim generator rewards #49

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

sotnikov-s
Copy link
Contributor

@sotnikov-s sotnikov-s commented Jun 2, 2023

task: https://p2pvalidator.atlassian.net/browse/NTRN-483

this PR:

  • replaces cw20 query and exec msgs performed for generator's base reward token with native ones

motivation:
Generator's base reward token is a native astro token bridged via IBC, not a cw20 token. This cw20-based behaviour would broke user's claim calls to the lockdrop contract when generator is assigned a vesting account in the corresponding vesting contract.

related PRs:

@sotnikov-s
Copy link
Contributor Author

@sotnikov-s sotnikov-s marked this pull request as ready for review June 5, 2023 19:22
@sotnikov-s
Copy link
Contributor Author

rwi.base_reward_token.to_string(),
)?
.amount;
base_reward_received = reward_token_balance - prev_ntrn_balance;
Copy link
Contributor

Choose a reason for hiding this comment

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

While we are here, let's use checked_sub to feel 🦀 extra safe 🦀?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

contract: becomes extra safe after single minus-to-checked_sub replacement
also contract: still has dozens of unsafe sub and sum operations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@foxpy @swelf19 @oopcode you want it — you got it ;) please review

@sotnikov-s
Copy link
Contributor Author

@zavgorodnii zavgorodnii merged commit e6f4381 into main Jun 8, 2023
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.

5 participants