-
Notifications
You must be signed in to change notification settings - Fork 145
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
Add stake_cw721 and cw721-controllers #309
Conversation
Codecov Report
@@ Coverage Diff @@
## main #309 +/- ##
==========================================
- Coverage 96.37% 95.76% -0.61%
==========================================
Files 29 32 +3
Lines 4797 5290 +493
==========================================
+ Hits 4623 5066 +443
- Misses 174 224 +50
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really great. Thanks for putting this up! Made a pass and added some comments. Will make another one once those are discussed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewards are hard and I think this has a bug where one can drain rewards from the contract. I missed this in the first review and an older version of the staking contract actually had the same problem. Many have struggled with this one. :)
My recommendation would be that we just rip rewards out of this contract for now. The nice thing about these contracts is that they can always be upgraded later so we can start simple. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some small nits
61fc084
to
2dd493b
Compare
Adds the ability to stake Cw721 Tokens. Allows for staked Nft use cases.
Closes #131