-
Notifications
You must be signed in to change notification settings - Fork 39
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
Abstract IncentivesContract logic, adapt Staked Incentives Controller and Pull Reward Incentives Controller #6
Merged
+1,928
−300
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
eacb14f
feat: Added base ERC20 incentives controller contract and deploy scri…
kartojal 6a80c15
feat: Added tests for base incentives controller
kartojal 78f2c15
fix: fix npm run test script
kartojal 26ff2e6
feat: Use an abstract contract to hold all the IncentivesController l…
kartojal 9e1653a
rename: rename incentives proposal to incentives controller in packag…
kartojal ad5274c
scripts: fix task name at tests script
kartojal 77b7c65
feat: Added claimRewardsToSelf function to BaseIncentivesController t…
kartojal 9395033
tests: added test case for claimRewardsToSelf
kartojal 8c0c657
feat: perform similar overflow check as DistributionManager
kartojal 13b78bb
feat: bypass accrue extra rewards at claim if amount is less than unc…
kartojal fc0b5fd
feat: fix tests, remove unused initialize parameter
kartojal c02a737
feat: remove large commented block
kartojal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
// SPDX-License-Identifier: agpl-3.0 | ||
pragma solidity 0.7.5; | ||
pragma experimental ABIEncoderV2; | ||
|
||
import {IERC20} from '@aave/aave-stake/contracts/interfaces/IERC20.sol'; | ||
import {SafeERC20} from '@aave/aave-stake/contracts/lib/SafeERC20.sol'; | ||
|
||
import {BaseIncentivesController} from './base/BaseIncentivesController.sol'; | ||
|
||
/** | ||
* @title PullRewardsIncentivesController | ||
* @notice Distributor contract for ERC20 rewards to the Aave protocol participants that pulls ERC20 from external account | ||
* @author Aave | ||
**/ | ||
contract PullRewardsIncentivesController is | ||
BaseIncentivesController | ||
{ | ||
using SafeERC20 for IERC20; | ||
|
||
address internal _rewardsVault; | ||
|
||
event RewardsVaultUpdated(address indexed vault); | ||
|
||
constructor(IERC20 rewardToken, address emissionManager) | ||
BaseIncentivesController(rewardToken, emissionManager) | ||
{} | ||
|
||
/** | ||
* @dev Initialize AaveIncentivesController | ||
* @param rewardsVault rewards vault to pull ERC20 funds | ||
**/ | ||
function initialize(address rewardsVault) external initializer { | ||
_rewardsVault = rewardsVault; | ||
emit RewardsVaultUpdated(_rewardsVault); | ||
} | ||
|
||
/** | ||
* @dev returns the current rewards vault contract | ||
* @return address | ||
*/ | ||
function getRewardsVault() external view returns (address) { | ||
return _rewardsVault; | ||
} | ||
|
||
/** | ||
* @dev update the rewards vault address, only allowed by the Rewards admin | ||
* @param rewardsVault The address of the rewards vault | ||
**/ | ||
function setRewardsVault(address rewardsVault) external onlyEmissionManager { | ||
_rewardsVault = rewardsVault; | ||
emit RewardsVaultUpdated(rewardsVault); | ||
} | ||
|
||
|
||
/// @inheritdoc BaseIncentivesController | ||
function _transferRewards(address to, uint256 amount) internal override { | ||
IERC20(REWARD_TOKEN).safeTransferFrom(_rewardsVault, to, amount); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is there a risk of updating the rewardsVault, but leaving existing token emissions? For token contract updates, would be fine, but if switching tokens could be an issue. Probably can just be handled with clear documentation but can consider include smart contract options.
Potential options
Clearly document that existing emissions will remain for the new token
Clear all emmissions on
setRewardsVault
Add a parameters to
setRewardsVault
and include a call to update emissionsThere 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.
If the EmissionManager changes the rewards vault, it will not change the current token as is inmmutable, just the source where "pull" and transfer the reward token. In my opinion emissions could still be the same and if the "reward vault" has sufficient tokens the incentives will not stop.
The rewards vault can be an EOA or smart contract with the sole purpose of pulling the reward tokens from this contract to the incentivized accounts.
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.
Makes sense +1