Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rewards/store: Fix claimed rewards' disappearance
*This PR is the product of some awesome team work. @chadoh helped me try to reproduce the issue locally; @stellarmagnet taught me how to inspect the state of a DAO on Rinkeby; @OttoG generously spent many of his sleeping hours going with me through the steps to publish my solution to Rinkeby and make it work there; and @topocount debugged an issue with claiming rewards. Thank you all so much!* On Rinkeby, when a reward is claimed it suddenly disappears from My Rewards, and reappears after switching tabs. Since I was unable to reproduce this on my local environment, I investigated the root cause of this problem and found the following facts: F1. The recently-claimed reward disappears because it gets filtered out from the `myRewards` object by the app state reducer. F2. The app state reducer filters out the reward because its `userRewardAmount` property suddenly becomes zero. F3. The reward with `userRewardAmount` set to zero comes from the store's `onRewardClaimed` function, which gets the reward's properties from `getRewardById`. F4. The `getRewardById` function gets the `userRewardAmount` property from the `rewardAmount` return value of the contract's `getReward` function. F5. The `rewardAmount` return value of the contract's `getReward` function is calculated dynamically by the contract's `_calculateRewardAmount` function. F6. `rewardAmount` is the reward's amount multiplied by the user's balance at disbursement date, divided by the reference token's supply at disbursement date. F7. In order for `rewardAmount` to be zero, either the reward's amount, the user's balance or the token's supply have to be zero. After determining this, I formulated the following hypothesis: H1. `rewardAmount` is zero because the user's balance is zero, because `_calculateRewardAmount` is receiving the wrong user; a user who doesn't possess any of that token. This happens when the store's `getRewardById` function doesn't receive a `userAddress` param. My hypothesis H1 is supported by the following facts: F8. On the UI, the reward reappears whenever tabs are switched. What happens underneath is that the store's `onRefreshRewards` function gets called. This function calls `getRewardById` just like `onRewardClaimed` (see F3), but unlike it, it specifies a user address. F9. Another store function, `onRewardAdded`, also calls `getRewardById` without specifying a user address, and the same behaviour on the UI can be observed as in `onRewardClaimed`: the reward's `rewardAmount` field is set to zero. This is less noticeable, but can be observed by creating a reward while on the My Rewards tab. F10. The `userRewardAmount` property of the recently-claimed reward is not the only property that becomes zero. The same happens to `timeClaimed`. On the contract, `timeClaimed` is a map from addresses to integers. The contract's `getReward` function uses the calling user's address to get the value. If the user's address didn't exist on the map, that could explain why it ends up being zero. F11. @topocount [solved a similar bug](11c9b3f?diff=split#diff-44e6c6b68470709dfa38518ab1f9c198R53) a while ago. The description he gave me of the bug matched perfectly H1. In order to test my hypothesis, @OttoG and I published my locally modified version of the Rewards app to Rinkeby, created a Rinkeby DAO and installed it there. The bug went away. You can test it [here](https://rinkeby.aragon.org/#/rewards45/0x632b710170835d55946cc717a50586267c2b6832/). So I can say with confidence that this PR fixes #1518 :) The only remaining mistery is why can't this be reproduced locally. I don't know enough about the differences between Ganache and Rinkeby (or between the local environment and other blockchain environments in general) in order to answer that question. But I do believe it's a relevant question that needs further inquiry in order to determine if the best path forward with similar issues is to somehow make the local environment more closely resemble the blockchain, or to acknowledge the need to sometimes deploy custom Rinkeby DAOs, just like in this case.
- Loading branch information