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.

Sign up for GitHub

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

Remove useless code from RewardFactory #172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Remove useless code from RewardFactory #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b1eee3
Remove useless code
bxmmm1 Jul 5, 2022
d669885
Merge branch 'main' of github.com:PrimeDAO/pools-contracts into 144-c…
bxmmm1 Jul 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 0 additions & 50 deletions contracts/factories/RewardFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import "../utils/MathUtil.sol";
contract RewardFactory is IRewardFactory {
using MathUtil for uint256;

event ExtraRewardAdded(address reward, uint256 pid);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, why do you think this code is not going to be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is related to ExtraRewardStashV2, and we are using v3, which doesn't need this anymore. And it is not used anywhere

event ExtraRewardRemoved(address reward, uint256 pid);
event StashAccessGranted(address stash);
event BaseRewardPoolCreated(address poolAddress);
event VirtualBalanceRewardPoolCreated(address baseRewardPool, address poolAddress, address token);
Expand All @@ -22,60 +20,12 @@ contract RewardFactory is IRewardFactory {
address public immutable operator;

mapping(address => bool) private rewardAccess;
mapping(address => uint256[]) public rewardActiveList;

constructor(address _operator, address _bal) {
operator = _operator;
bal = _bal;
}

/// @notice Get active rewards count
/// @return uint256 number of active rewards
function activeRewardCount(address _reward) external view returns (uint256) {
return rewardActiveList[_reward].length;
}

/// @notice Adds a new reward to the active list
/// @return true on success
function addActiveReward(address _reward, uint256 _pid) external returns (bool) {
if (!rewardAccess[msg.sender]) {
revert Unauthorized();
}
uint256 pid = _pid + 1; // offset by 1 so that we can use 0 as empty

uint256[] memory activeListMemory = rewardActiveList[_reward];
for (uint256 i = 0; i < activeListMemory.length; i = i.unsafeInc()) {
if (activeListMemory[i] == pid) return true;
}
rewardActiveList[_reward].push(pid);
emit ExtraRewardAdded(_reward, _pid);
return true;
}

/// @notice Removes active reward
/// @param _reward The address of the reward contract
/// @param _pid The pid of the pool
/// @return true on success
function removeActiveReward(address _reward, uint256 _pid) external returns (bool) {
if (!rewardAccess[msg.sender]) {
revert Unauthorized();
}
uint256 pid = _pid + 1; //offset by 1 so that we can use 0 as empty

uint256[] memory activeListMemory = rewardActiveList[_reward];
for (uint256 i = 0; i < activeListMemory.length; i = i.unsafeInc()) {
if (activeListMemory[i] == pid) {
if (i != activeListMemory.length - 1) {
rewardActiveList[_reward][i] = rewardActiveList[_reward][activeListMemory.length - 1];
}
rewardActiveList[_reward].pop();
emit ExtraRewardRemoved(_reward, _pid);
break;
}
}
return true;
}

/// @notice Grants rewardAccess to stash
/// @dev Stash contracts need access to create new Virtual balance pools for extra gauge incentives(ex. snx)
function grantRewardStashAccess(address _stash) external {
Expand Down
6 changes: 0 additions & 6 deletions contracts/utils/Interfaces.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,6 @@ interface IRewardFactory {
address,
address
) external returns (address);

function activeRewardCount(address) external view returns (uint256);

function addActiveReward(address, uint256) external returns (bool);

function removeActiveReward(address, uint256) external returns (bool);
}

interface IStashFactory {
Expand Down
86 changes: 1 addition & 85 deletions test/unit/reward-factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const { ZERO_ADDRESS } = require('@openzeppelin/test-helpers/src/constants');
const { expect, assert } = require('chai');
const { deployments, ethers } = require('hardhat');
const { ONE_ADDRESS, TWO_ADDRESS } = require('../helpers/constants.js');
Expand Down Expand Up @@ -33,97 +32,14 @@ describe('unit - RewardFactory', function () {
.to.emit(rewardFactoryContract, 'StashAccessGranted')
.withArgs(anotherUser.address);
});
});

context('Active rewards', async function () {
it('adds reward', async function () {
const { rewardFactoryContract, anotherUser, operator } = await setupTests();

// Give access to somebody
await expect(rewardFactoryContract.connect(operator).grantRewardStashAccess(anotherUser.address));

const pid = 1;

await expect(rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid))
.to.emit(rewardFactoryContract, 'ExtraRewardAdded')
.withArgs(ONE_ADDRESS, pid);
});

it('reverts if user is unauthorized to add reward', async function () {
const { rewardFactoryContract } = await setupTests();

await expect(rewardFactoryContract.addActiveReward(ONE_ADDRESS, 1)).to.be.revertedWith('Unauthorized()');
});

it('reverts if user is unauthorized to remove reward', async function () {
const { rewardFactoryContract } = await setupTests();

await expect(rewardFactoryContract.removeActiveReward(ZERO_ADDRESS, 1)).to.be.revertedWith('Unauthorized()');
});

it('reverts if user is unauthorized to add access', async function () {
it('reverts if user is unauthorized to add stash access', async function () {
const { rewardFactoryContract, anotherUser } = await setupTests();

await expect(rewardFactoryContract.grantRewardStashAccess(anotherUser.address)).to.be.revertedWith(
'Unauthorized()'
);
});

it('gets rewards count', async function () {
const { rewardFactoryContract, anotherUser, operator } = await setupTests();

// Give access to somebody
await expect(rewardFactoryContract.connect(operator).grantRewardStashAccess(anotherUser.address));

const pid = 1;

await expect(rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid))
.to.emit(rewardFactoryContract, 'ExtraRewardAdded')
.withArgs(ONE_ADDRESS, pid);

// should not revert if we try to add the same reward again, it returns early
expect(await rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid));

// returns early
expect(await rewardFactoryContract.connect(anotherUser).addActiveReward(ZERO_ADDRESS, pid));

expect(await rewardFactoryContract.activeRewardCount(ONE_ADDRESS)).to.equal(1);
});

it('removes reward', async function () {
const { rewardFactoryContract, anotherUser, operator } = await setupTests();

// Give access to somebody
await expect(rewardFactoryContract.connect(operator).grantRewardStashAccess(anotherUser.address));

const pid = 1;

// add extra rewards
await expect(rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid)).to.emit(
rewardFactoryContract,
'ExtraRewardAdded'
);
await expect(rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid + 1)).to.emit(
rewardFactoryContract,
'ExtraRewardAdded'
);
await expect(rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid + 2)).to.emit(
rewardFactoryContract,
'ExtraRewardAdded'
);
await expect(rewardFactoryContract.connect(anotherUser).addActiveReward(ONE_ADDRESS, pid + 3)).to.emit(
rewardFactoryContract,
'ExtraRewardAdded'
);

// returns early zero address test
await expect(rewardFactoryContract.connect(anotherUser).removeActiveReward(ZERO_ADDRESS, pid));
await expect(rewardFactoryContract.connect(anotherUser).removeActiveReward(ZERO_ADDRESS, pid));

await expect(rewardFactoryContract.connect(anotherUser).removeActiveReward(ONE_ADDRESS, pid))
.to.emit(rewardFactoryContract, 'ExtraRewardRemoved')
.withArgs(ONE_ADDRESS, pid);
});
});

context('Create pools', async function () {
Expand Down