Comments (6)
OK, I was too curious and continued to look at it 🙂 I believe the problem is that the code doesn't take into account that for example in this scenario:
- Alice updated
- Bob updated
- Alice updated
Of the reward that was added in the last step, part of it belongs to Alice because of her first deposit. We are neglecting that part when we set the rewardDebt at the end.
To fix, we need to calculate how much reward Alice earned from her new reward - playerEarned
:
uint256 playerEarned;
// Update acc values before updating contributions so players don't get rewards for their own penalties
if (totalContribution != 0) { // The first reward will be permanently locked on the contract
_increaseRewardPerContribution(reward);
playerEarned = player.contribution * reward / totalContribution;
}
(Note that at this point player.contribution
still doesn't contain the new contribution)
And then substract it from rewardDebt so the user can claim it.
player.rewardDebt = _totalPlayerRewardSoFar(player.contribution) - pendingReward - playerEarned;
Perhaps more testing is needed, but this fixes the issue based on a quick test that I did.
from dixel-contract.
It seems that this happens also when a user update twice, not just in a row.
For example: Alice updates 2 pixels, Bob updates 2 pixels, Alice updates 2 pixels - reward will be lost.
See here for a simple test scenario that shows this by printing Dixel's balance.
But if Alice updates, then Bob updates, then Carol updates - then the calculation is ok.
So it doesn't need to be twice in a row.
I'll try looking at it further if you won't fix it by the time I'm able to.
from dixel-contract.
Alright, cool. So this first fix that I suggested will fix it.
I have created test script for the scenario you just described.
See here. Seems to be working with the fix. You can add the test to Dixel.test.js.
[Note: there's some repetition in the test, maybe you'd like to extract some of it to a different function to remove the repetition.
I tried to do it but honestly I haven't used JS in a while, so so far I didn't manage to get the right scoping to be able to access all the variables.
Anyway this way is not a disaster and you can see what's happening.]
from dixel-contract.
Thank you so much for your help on this issue @assafom 👍🏻
It looks fine now, but please let me know if there's more edge cases you can think of.
from dixel-contract.
And if you don't want that Alice's second update will reward Alice's contribution from first update, I believe this is the change that is needed:
Update _increaseRewardPerContribution
to also take the totalContribution/shares as parameter:
function _increaseRewardPerContribution(uint256 rewardAdded, uint256 totalShares) private {
unchecked {
accRewardPerContribution += (1e18 * rewardAdded) / totalShares;
}
}
After updating the increase of shares reward and calculating how much of it belongs to the updating user, we use _increaseRewardPerContribution
to add that reward to the rest of the users:
uint256 userEarned;
// Update acc values before updating contributions so players don't get rewards for their own penalties
if (totalContribution != 0) { // The first reward will be permanently locked on the contract
_increaseRewardPerContribution(reward, totalContribution);
userEarned = player.contribution * reward / totalContribution;
_increaseRewardPerContribution(userEarned, totalContribution-player.contribution);
}
And we keep rewardDebt the same - so it already takes userEarned
into account and the user can't claim it.
player.rewardDebt = _totalPlayerRewardSoFar(player.contribution) - pendingReward;
from dixel-contract.
Oh Alice's second update should reward Alice's first update. I just tried to prevent rewarding from the current update.
For example,
- Alice creates 10 reward at the beginning -> No one is prior to Alice, so reward is just locked on the contract forever
- Alice creates another 20 reward -> Alice gets the 20 reward from her first update
- Bob creates another 40 reward -> Alice gets all 40 because Bob shouldn't be rewarded by his own contribution
- Bob creates another 30 reward -> So far, Alice contribution: 2 / Bob contribution: 1 -> Alice gets 20 / Bob gets 10
- Carol creates 40 reward -> Alice contribution: 2 / Bob contribution: 2 -> Alice gets 20 / Bob gets 20 / Carol gets nothing because she doesn't have any previous updates
Assumption: All event increases the player's contribution by 1 (1 pixel update)
from dixel-contract.
Related Issues (11)
- After burning token, original minter continues to receive rewards HOT 1
- Consider removing usage of _msgSender() HOT 2
- airdrop reopening HOT 1
- Running all tests not working any more HOT 1
- Reward calculation error - lost rewards HOT 1
- Reward distribution for community audits 💸 HOT 9
- gas saving HOT 2
- Prevent duplicated pixel params HOT 1
- Prevent updating pixel with the same color HOT 1
- playerWallets can contain duplicate values HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from dixel-contract.