Skip to content

Fix execution hashes #22

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

Open
c-future wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Fix execution hashes #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
39f81d0
Arbitrary rewards and fixes
St4rgarden Jul 20, 2022
699b717
Multi distro rewards
St4rgarden Jul 20, 2022
06cf4e7
migrations integrated
St4rgarden Jul 20, 2022
3a8ac8c
earmarkFees working
St4rgarden Jul 22, 2022
8d0fcea
[WP-H3] totalClaimed is not reset in cancel(), making the same user u…
Jiverrr Aug 19, 2022
4b7272f
[WP-M5] vestedSupply will be inaccurate when there are canceled vestings
Jiverrr Aug 19, 2022
a81bccf
Added extra tests to validate audit fixes
Jiverrr Aug 21, 2022
e48821e
Merge pull request #17 from veTokenFinance/fix-vested-escrow-second-r…
Jiverrr Aug 21, 2022
7d9212a
Merge remote-tracking branch 'origin/main' into fix_executionHashes
St4rgarden Aug 22, 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
87 changes: 63 additions & 24 deletions contracts/Booster.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ contract Booster is ReentrancyGuardUpgradeable {
address public staker;
address public minter;
address public veAsset;
address public feeDistro;
// address public feeDistro;
address public rewardFactory;
address public stashFactory;
address public tokenFactory;
Expand All @@ -55,8 +55,8 @@ contract Booster is ReentrancyGuardUpgradeable {
address public stakerRewards; //vetoken rewards
address public stakerLockRewards; // veToken lock rewards xVE3D
address public lockRewards; //ve3Token rewards(veAsset)
address public lockFees; //ve3Token veVeAsset fees
address public feeToken;
// address public lockFees; //ve3Token veVeAsset fees
// address public feeToken;

bool public isShutdown;

Expand All @@ -69,6 +69,18 @@ contract Booster is ReentrancyGuardUpgradeable {
bool shutdown;
}

struct FeeDistro {
address distro;
address rewards;
bytes32 executionHash;
bool active;
}

address[] public allFeeTokens;

//reward identifier -> distro, execution and virtual pool data
mapping(address => FeeDistro) public feeTokens;
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

we don't need a mapping for all fee distro info for each veasset project , because we have one booster for each veasset project , so it should be a single state for fee distro , execution hash and array of fee tokens (in case there is multiple fee tokens) you could use openzipplein EnumerableSet for fee tokens , and add new mapping for fee token => lockfee (reward address)


//index(pid) -> pool
PoolInfo[] public poolInfo;
mapping(address => bool) public gaugeMap;
Expand Down Expand Up @@ -110,8 +122,8 @@ contract Booster is ReentrancyGuardUpgradeable {
function __Booster_init(
address _staker,
address _minter,
address _veAsset,
address _feeDistro
address _veAsset
// address _feeDistro
) external initializer {
isShutdown = false;
staker = _staker;
Expand All @@ -121,7 +133,7 @@ contract Booster is ReentrancyGuardUpgradeable {
poolManager = msg.sender;
minter = _minter;
veAsset = _veAsset;
feeDistro = _feeDistro;
// feeDistro = _feeDistro;
lockIncentive = 1000;
stakerIncentive = 450;
earmarkIncentive = 50;
Expand Down Expand Up @@ -190,34 +202,39 @@ contract Booster is ReentrancyGuardUpgradeable {

//reward contracts are immutable or else the owner
//has a means to redeploy and mint cvx via rewardClaimed()
if (lockRewards == address(0)) {
require(_rewards != address(0), "Not allowed!");
if (lockRewards == address(0) && _rewards != address(0)) {
lockRewards = _rewards;
}
if (stakerRewards == address(0)) {
require(_stakerRewards != address(0), "Not allowed!");
if (stakerRewards == address(0) && _stakerRewards != address(0)) {
stakerRewards = _stakerRewards;
}
if (stakerLockRewards == address(0)) {
require(_stakerLockRewards != address(0), "Not allowed!");
if (stakerLockRewards == address(0) && _stakerLockRewards != address(0)) {
stakerLockRewards = _stakerLockRewards;
}

emit RewardContractsUpdated(_rewards, _stakerRewards, _stakerLockRewards);
}

// Set reward token and claim contract, get from Curve's registry
function setFeeInfo(uint256 _lockFeesIncentive, uint256 _stakerLockFeesIncentive) external {
// Set reward token and distro claim contract; create new virtual reward pool and sets execution hash
// Also used to add or update distro contracts and execution hashes for an already active reward token
function setFeeInfo(
uint256 _lockFeesIncentive,
uint256 _stakerLockFeesIncentive,
address _feeToken,
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

we have to pass array of fee tokens even if it is only one to support multiple fee tokens

address _distro,
bytes32 _executionHash
) external {
require(msg.sender == feeManager, "!auth");
require(_lockFeesIncentive.add(_stakerLockFeesIncentive) == FEE_DENOMINATOR);
require(_lockFeesIncentive.add(_stakerLockFeesIncentive) == FEE_DENOMINATOR, "!fee");

lockFeesIncentive = _lockFeesIncentive;
stakerLockFeesIncentive = _stakerLockFeesIncentive;

address _feeToken = IFeeDistro(feeDistro).token();
if (feeToken != _feeToken) {
if (feeTokens[_feeToken].active != true) {
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

so here we need an iteration on fee tokens and check if it is exist or not

require(!gaugeMap[_feeToken], "!token");
//require that we initialize at the zero index
//create a new reward contract for the new token
lockFees = IRewardFactory(rewardFactory).CreateTokenRewards(_feeToken, lockRewards);
address lockFees = IRewardFactory(rewardFactory).CreateTokenRewards(_feeToken, lockRewards);

if (_feeToken != veAsset) {
IRewards(stakerLockRewards).addReward(
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

it will create new reward pool (lookFees) for each fee tokens and use mapping to save it

Expand All @@ -230,7 +247,14 @@ contract Booster is ReentrancyGuardUpgradeable {
);
}

feeToken = _feeToken;
feeTokens[_feeToken] = FeeDistro(_distro, lockFees, _executionHash, true);

allFeeTokens.push(_feeToken);

} else {
feeTokens[_feeToken].distro = _distro;
feeTokens[_feeToken].executionHash = _executionHash;
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

this could be move to up with these two lines for update the state

lockFeesIncentive = _lockFeesIncentive;
stakerLockFeesIncentive = _stakerLockFeesIncentive;


}
}

Expand Down Expand Up @@ -407,6 +431,13 @@ contract Booster is ReentrancyGuardUpgradeable {
address token = pool.token;
ITokenMinter(token).burn(_from, _amount);

// @dev handle staking factor for Angle ,
// use try and catch as not all Angle gauges have scaling factor
if (IVoteEscrow(staker).escrowModle() == IVoteEscrow.EscrowModle.ANGLE) {
try IGauge(gauge).scaling_factor() {
_amount = _amount.mul(IGauge(gauge).scaling_factor()).div(10**18);
} catch {}
}
//pull from gauge if not shutdown
// if shutdown tokens will be in this contract
if (!pool.shutdown) {
Expand Down Expand Up @@ -524,6 +555,7 @@ contract Booster is ReentrancyGuardUpgradeable {
_claimStashReward(stash);
}
}

//veAsset balance
uint256 veAssetBal = IERC20Upgradeable(veAsset).balanceOf(address(this));

Expand Down Expand Up @@ -594,20 +626,27 @@ contract Booster is ReentrancyGuardUpgradeable {
}

//claim fees from fee distro contract, put in lockers' reward contract
function earmarkFees() external returns (bool) {
function earmarkFees(address feeToken, bytes calldata _executionData) external returns (bool) {
// hash our execution data for comparison
bytes32 hashedExecutionData = keccak256(_executionData);
// enforce that the execution data is approved
require(hashedExecutionData == feeTokens[feeToken].executionHash, "!auth");
//claim fee rewards
IStaker(staker).claimFees(feeDistro, feeToken);
//send fee rewards to reward contract
IStaker(staker).claimFees(feeTokens[feeToken].distro, feeToken, _executionData);
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

as I understand _executionData will have the function signature without the inputs , so I think the inputs are missing here when pass it to function claimFee

Copy link
Copy Markdown
Collaborator

@St4rgarden St4rgarden Sep 21, 2022

Choose a reason for hiding this comment

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

_executionData is both the function signature and the arguments. This is how we enforce that the arguments passed into claimFees can't be tampered with <3

// access contract token balance after claiming rewards
uint256 _balance = IERC20Upgradeable(feeToken).balanceOf(address(this));

// calculate incentive split for reward contracts
uint256 _lockFeesIncentive = _balance.mul(lockFeesIncentive).div(FEE_DENOMINATOR);
uint256 _stakerLockFeesIncentive = _balance.mul(stakerLockFeesIncentive).div(
FEE_DENOMINATOR
);
// transfer to virtual reward pool and queue the new rewards
if (_lockFeesIncentive > 0) {
IERC20Upgradeable(feeToken).safeTransfer(lockFees, _lockFeesIncentive);
IRewards(lockFees).queueNewRewards(_lockFeesIncentive);
IERC20Upgradeable(feeToken).safeTransfer(feeTokens[feeToken].rewards, _lockFeesIncentive);
IRewards(feeTokens[feeToken].rewards).queueNewRewards(_lockFeesIncentive);
}
// transfer to the VE3D Locker and queue the new rewards
if (_stakerLockFeesIncentive > 0) {
IERC20Upgradeable(feeToken).safeTransfer(stakerLockRewards, _stakerLockFeesIncentive);
IRewards(stakerLockRewards).queueNewRewards(feeToken, _stakerLockFeesIncentive);
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

after call the function , it needs iteration for the fee tokens list

Expand Down
2 changes: 1 addition & 1 deletion contracts/Interfaces/IStaker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ interface IStaker {

function claimRewards(address) external;

function claimFees(address, address) external;
function claimFees(address, address, bytes calldata) external;

function setStashAccess(address, bool) external;

Expand Down
2 changes: 2 additions & 0 deletions contracts/VestedEscrow.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,11 @@ contract VestedEscrow is Ownable, ReentrancyGuard {

if (delta != 0) {
rewardToken.safeTransfer(owner(), delta);
initialLockedSupply = initialLockedSupply.sub(delta);
}

initialLocked[_recipient] = 0;
totalClaimed[_recipient] = 0;
}

function _totalVestedOf(address _recipient, uint256 _time) internal view returns (uint256) {
Expand Down
14 changes: 11 additions & 3 deletions contracts/VoterProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,17 @@ contract VoterProxy {
return true;
}

function claimFees(address _distroContract, address _token) external returns (uint256) {
// execute low level contract call on the distro contract and forward claimed rewards to Booster
function claimFees(
address _distroContract,
address _token,
bytes calldata executionData)
external returns (uint256) {
// enforce contract call comes from the correct source
require(msg.sender == operator, "!auth");
IFeeDistro(_distroContract).claim();
// execute arbitrary enforced claim
(bool success, bytes memory result) = _distroContract.call(executionData);
require(success, "!fail");
Copy link
Copy Markdown
Collaborator Author

@c-future c-future Sep 21, 2022

Choose a reason for hiding this comment

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

did you test it

Copy link
Copy Markdown
Collaborator

@St4rgarden St4rgarden Sep 21, 2022

Choose a reason for hiding this comment

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

I tested that we were successfully able to claim rewards yup

uint256 _balance = IERC20(_token).balanceOf(address(this));
IERC20(_token).safeTransfer(operator, _balance);
return _balance;
Expand All @@ -275,4 +283,4 @@ contract VoterProxy {

return (success, result);
}
}
}
24 changes: 16 additions & 8 deletions migrations/6_deploy_contracts_idle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ const { addContract, getContract } = require("./helper/addContracts");
const escrowABI = require("./helper/escrowABI.json");
const { deployProxy } = require("@openzeppelin/truffle-upgrades");

const VoterProxyV2 = artifacts.require("VoterProxyV2");
// const VoterProxyV2 = artifacts.require("VoterProxyV2");
const VoterProxy = artifacts.require("VoterProxy");
const VeTokenMinter = artifacts.require("VeTokenMinter");
const RewardFactory = artifacts.require("RewardFactory");
const VE3Token = artifacts.require("VE3Token");
Expand All @@ -26,7 +27,13 @@ function toBN(number) {
module.exports = async function (deployer, network, accounts) {
global.created = true;
const contractList = getContract();
let executionInterface = {"name":"claim","outputs":[{"type":"uint256","name":""}],"inputs":[],"stateMutability":"nonpayable","type":"function"};
let executionData = web3.eth.abi.encodeFunctionCall(executionInterface, []);
let executionHash = web3.utils.keccak256(executionData);
// the commented line below also works for function calls requiring no parameters
// let executionData = web3.eth.abi.encodeFunctionSignature("claim()");
let smartWalletWhitelistAddress = "0x2D8b5b65c6464651403955aC6D71f9c0204169D3";
let idleAddress = "0x875773784Af8135eA0ef43b5a374AaD105c5D39e";
let idle = await IERC20.at("0x875773784Af8135eA0ef43b5a374AaD105c5D39e");
let checkerAdmin = "0xFb3bD022D5DAcF95eE28a6B07825D4Ff9C5b3814";
let idleAdmin = "0xd6dabbc2b275114a2366555d6c481ef08fdc2556";
Expand Down Expand Up @@ -62,11 +69,7 @@ module.exports = async function (deployer, network, accounts) {
await web3.eth.sendTransaction({ from: admin, to: stashRewardTokenUser, value: web3.utils.toWei("1") });

// voter proxy
const voter = await deployProxy(
VoterProxyV2,
["idleVoterProxy", idle.address, stkIDLE, gaugeController, idleMintr, 3],
{ deployer, initializer: "__VoterProxyV2_init" }
);
const voter = await deployer.deploy(VoterProxy, "idleVoterProxy", idle.address, stkIDLE, gaugeController, idleMintr, 3);

// whitelist the voter proxy
const whitelist = await SmartWalletWhitelist.at(smartWalletWhitelistAddress);
Expand All @@ -88,7 +91,7 @@ module.exports = async function (deployer, network, accounts) {
// booster
const booster = await deployProxy(
Booster,
[voter.address, contractList.system.vetokenMinter, idle.address, feeDistro],
[voter.address, contractList.system.vetokenMinter, idle.address],
{ deployer, initializer: "__Booster_init" }
);

Expand Down Expand Up @@ -157,7 +160,12 @@ module.exports = async function (deployer, network, accounts) {
await booster.setFactories(rFactory.address, sFactory.address, tFactory.address),
"booster setFactories"
);
logTransaction(await booster.setFeeInfo(toBN(10000), toBN(0)), "booster setFeeInfo");
logTransaction(await booster.setFeeInfo(
toBN(10000),
toBN(0),
idleAddress,
feeDistro,
executionHash), "booster setFeeInfo");
//vetoken minter setup
const vetokenMinter = await VeTokenMinter.at(contractList.system.vetokenMinter);
logTransaction(
Expand Down
14 changes: 11 additions & 3 deletions test/booster-lp-short.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ contract("Booster LP Stake", async (accounts) => {
let veassetToken;
let ve3dLocker;
let escrow;
let feeDistro;
let lpToken;
let voterProxy;
let booster;
Expand All @@ -57,6 +56,11 @@ contract("Booster LP Stake", async (accounts) => {
let network;
let uniExchangeRouterAddress = "0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D";
const sushiExchangeRouterAddress = "0xd9e1cE17f2641f24aE83637ab66a2cca9C378B9F";
let idleAddress = "0x875773784Af8135eA0ef43b5a374AaD105c5D39e";
let feeDistro = "0xbabb82456c013fd7e3f25857e0729de8207f80e2";
let executionInterface = {"name":"claim","outputs":[{"type":"uint256","name":""}],"inputs":[],"stateMutability":"nonpayable","type":"function"}
let executionData = web3.eth.abi.encodeFunctionCall(executionInterface, []);
let executionHash = web3.utils.keccak256(executionData);

before("setup", async () => {
network = await loadContracts();
Expand All @@ -77,7 +81,6 @@ contract("Booster LP Stake", async (accounts) => {
ve3Token = await VE3Token.at(contractAddresseList[5]);
veassetDepositer = await VeAssetDepositor.at(contractAddresseList[6]);
ve3TokenRewardPool = await BaseRewardPool.at(contractAddresseList[7]);
feeDistro = await booster.feeDistro();
uniExchange = new web3.eth.Contract(uniswapV2Router, uniExchangeRouterAddress);
sushiExchange = new web3.eth.Contract(uniswapV2Router, sushiExchangeRouterAddress);
ve3dLocker = await VE3DLocker.at(baseContractList.system.ve3dLocker);
Expand Down Expand Up @@ -829,7 +832,12 @@ contract("Booster LP Stake", async (accounts) => {
// });

it("check setFeeInfo (try to set more than FEE_DENOMINATOR)", async () => {
await truffleAssert.reverts(booster.setFeeInfo(toBN(10001), toBN(0)), "status 0");
await truffleAssert.reverts(booster.setFeeInfo(
toBN(10001),
toBN(0),
idleAddress,
feeDistro,
executionHash), "!fee");
// Seems not failing at all!
});

Expand Down
Loading