Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 43 additions & 19 deletions contracts/AcceleratingDistributor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -182,26 +182,24 @@ contract AcceleratingDistributor is ReentrancyGuard, Ownable, Multicall {
* @param amount The amount of the token to stake.
*/
function stake(address stakedToken, uint256 amount) external nonReentrant onlyEnabled(stakedToken) {
_updateReward(stakedToken, msg.sender);
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be copied exactly to new _stake internal method


UserDeposit storage userDeposit = stakingTokens[stakedToken].stakingBalances[msg.sender];

uint256 averageDepositTime = getAverageDepositTimePostDeposit(stakedToken, msg.sender, amount);

userDeposit.averageDepositTime = averageDepositTime;
userDeposit.cumulativeBalance += amount;
stakingTokens[stakedToken].cumulativeStaked += amount;

IERC20(stakedToken).safeTransferFrom(msg.sender, address(this), amount);
_stake(stakedToken, amount, msg.sender);
}

emit Stake(
stakedToken,
msg.sender,
amount,
averageDepositTime,
userDeposit.cumulativeBalance,
stakingTokens[stakedToken].cumulativeStaked
);
/**
* @notice Stake tokens for rewards on behalf of `beneficiary`.
* @dev The caller of this function must approve this contract to spend amount of stakedToken.
* @dev The caller of this function is effectively donating their tokens to the beneficiary. The beneficiary
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

* can then unstake or claim rewards as they wish.
* @param stakedToken The address of the token to stake.
* @param amount The amount of the token to stake.
* @param beneficiary User that caller wants to stake on behalf of.
*/
function stakeFor(
address stakedToken,
uint256 amount,
address beneficiary
) external nonReentrant onlyEnabled(stakedToken) {
_stake(stakedToken, amount, beneficiary);
}

/**
Expand Down Expand Up @@ -391,4 +389,30 @@ contract AcceleratingDistributor is ReentrancyGuard, Ownable, Multicall {
userDeposit.rewardsAccumulatedPerToken = stakingToken.rewardPerTokenStored;
}
}

function _stake(
address stakedToken,
uint256 amount,
address staker
) internal {
_updateReward(stakedToken, staker);

UserDeposit storage userDeposit = stakingTokens[stakedToken].stakingBalances[staker];

uint256 averageDepositTime = getAverageDepositTimePostDeposit(stakedToken, staker, amount);
Copy link
Contributor

@kevinuma kevinuma Nov 2, 2022

Choose a reason for hiding this comment

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

This is existing code but in getAverageDepositTimePostDeposit, I see

uint256 amountWeightedTime = (((amount * 1e18) / (userDeposit.cumulativeBalance + amount)) *
(getTimeSinceAverageDeposit(stakedToken, account))) / 1e18;

Why not multiple first before dividing by (userDeposit.cumulativeBalance + amount) to minimize rounding errors? Also there are way too many parentheses there.

uint256 amountWeightedTime = (amount * 1e18) * getTimeSinceAverageDeposit(stakedToken, account) / (userDeposit.cumulativeBalance + amount) / 1e18;

Copy link
Member Author

Choose a reason for hiding this comment

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

These are good points but I don't love modifying existing already-audited code. @chrismaree wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

i dont think we touch this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kevinuma In general, we would prefer to modify the code as little as possible. The only exception is if we would classify this as a bug. Is the rounding error substantial enough to consider it a bug?

Copy link
Contributor

Choose a reason for hiding this comment

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

The rounding error is likely not significant unless a user has staked a significant amount of tokens and then add a very small amount.


userDeposit.averageDepositTime = averageDepositTime;
userDeposit.cumulativeBalance += amount;
stakingTokens[stakedToken].cumulativeStaked += amount;

IERC20(stakedToken).safeTransferFrom(msg.sender, address(this), amount);
emit Stake(
stakedToken,
staker,
amount,
averageDepositTime,
userDeposit.cumulativeBalance,
stakingTokens[stakedToken].cumulativeStaked
);
}
}
50 changes: 50 additions & 0 deletions contracts/ClaimAndStake.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/utils/Multicall.sol";
import "@across-protocol/contracts-v2/contracts/merkle-distributor/AcrossMerkleDistributor.sol";
import "./AcceleratingDistributor.sol";

/**
* @notice Allows claimer to claim tokens from AcrossMerkleDistributor and stake into AcceleratingDistributor
* atomically in a single transaction. This intermediary contract also removes the need for claimer to approve
* AcceleratingDistributor to spend its staking tokens.
*/

contract ClaimAndStake is ReentrancyGuard, Multicall {
using SafeERC20 for IERC20;

// Contract which rewards tokens to users that they can then stake.
AcrossMerkleDistributor public immutable merkleDistributor;

// Contract that user stakes claimed tokens into.
AcceleratingDistributor public immutable acceleratingDistributor;

constructor(AcrossMerkleDistributor _merkleDistributor, AcceleratingDistributor _acceleratingDistributor) {
merkleDistributor = _merkleDistributor;
acceleratingDistributor = _acceleratingDistributor;
}

/**************************************
* ADMIN FUNCTIONS *
**************************************/

/**
* @notice Claim tokens from a MerkleDistributor contract and stake them for rewards in AcceleratingDistributor.
* @dev Will revert if `merkleDistributor` is not set to valid MerkleDistributor contract.
* @dev Will revert if the claim recipient account is not equal to caller, or if the reward token
* for claim is not a valid staking token.
* @dev Will revert if this contract is not a "whitelisted claimer" on the MerkleDistributor contract.
* @param _claim Claim leaf to retrieve from MerkleDistributor.
*/
function claimAndStake(MerkleDistributorInterface.Claim memory _claim) external nonReentrant {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can add a multi claim function as well since merkle distributor supports claimMulti anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

This contract is Multicaller so we can just multicall it. I think this is preferable to implementing claimMultiFor which would require re-writing most of the logic in claimMulti to be rewritten. This is because the claimMulti code doesn't simply call claim()

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright sounds good

Copy link
Member Author

Choose a reason for hiding this comment

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

New claimAndStake function that atomically calls claimFor on AcrossMerkleDistributor and then calls stakeFor for original claimer.

require(_claim.account == msg.sender, "claim account not caller");
address stakedToken = merkleDistributor.getRewardTokenForWindow(_claim.windowIndex);
merkleDistributor.claimFor(_claim);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we assert that this contract has received the right claim amount for additional safety?

Copy link
Member Author

Choose a reason for hiding this comment

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

i don't think we need to since we can assume that claimFor works as expected and because the merkleDistributor address is a trusted one (and set upon deployment). If we assume claimFor works

Copy link
Contributor

Choose a reason for hiding this comment

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

To call claimFor, this contract has to be whitelisted in the merkle distributor contract: https://github.com/across-protocol/contracts-v2/blob/master/contracts/merkle-distributor/AcrossMerkleDistributor.sol#L41. I don't see this in the deployment step. Is this something we'll do separately?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep! Usually we use the deployment scripts jsut to deploy contracts, and all the setup is done afterwards either manually or via scripts

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking it could be a step in the same deployment script as ClaimAndStake if the deployer still has ownership if merkle distributor. It's less manual and ensures that we don't need to make sure the addresses are right later.

IERC20(stakedToken).safeIncreaseAllowance(address(acceleratingDistributor), _claim.amount);
acceleratingDistributor.stakeFor(stakedToken, _claim.amount, msg.sender);
}
}
2 changes: 1 addition & 1 deletion contracts/test/AcceleratingDistributor_Testable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "../AcceleratingDistributor.sol";
import "./Testable.sol";

/**
* @notice // Tesable version of the AcceleratingDistributor that enables time to be overridden with a Testable contract.
* @notice // Testable version of the AcceleratingDistributor that enables time to be overridden with a Testable contract.
*/

contract AcceleratingDistributor_Testable is AcceleratingDistributor, Testable {
Expand Down
10 changes: 10 additions & 0 deletions contracts/test/MerkleDistributorTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// SPDX-License-Identifier: GPL-3.0-only
import "@across-protocol/contracts-v2/contracts/merkle-distributor/AcrossMerkleDistributor.sol";

pragma solidity ^0.8.0;

/// @notice Pass through contract that allows tests to access MerkleDistributor from /artifacts via
// utils.getContractFactory()
contract MerkleDistributorTest is AcrossMerkleDistributor {

}
18 changes: 18 additions & 0 deletions deploy/003_deploy_across_merkle_distributor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import "hardhat-deploy";
import { HardhatRuntimeEnvironment } from "hardhat/types/runtime";

const func = async function (hre: HardhatRuntimeEnvironment) {
const { deployments, getNamedAccounts } = hre;
const { deploy } = deployments;
const { deployer } = await getNamedAccounts();

await deploy("AcrossMerkleDistributor", {
from: deployer,
log: true,
skipIfAlreadyDeployed: true,
args: [],
});
};

module.exports = func;
func.tags = ["AcrossMerkleDistributor", "mainnet"];
21 changes: 21 additions & 0 deletions deploy/004_deploy_claim_and_stake.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import "hardhat-deploy";
import { HardhatRuntimeEnvironment } from "hardhat/types/runtime";

const func = async function (hre: HardhatRuntimeEnvironment) {
const { deployments, getNamedAccounts } = hre;
const { deploy } = deployments;
const { deployer } = await getNamedAccounts();

const merkleDistributor = await deployments.get("AcrossMerkleDistributor");
const acceleratingDistributor = await deployments.get("AcceleratingDistributor");

await deploy("ClaimAndStake", {
from: deployer,
log: true,
skipIfAlreadyDeployed: true,
args: [merkleDistributor.address, acceleratingDistributor.address],
});
};

module.exports = func;
func.tags = ["ClaimAndStake", "mainnet"];
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"prepublish": "yarn build && yarn generate-contract-types"
},
"devDependencies": {
"@across-protocol/contracts-v2": "^1.0.7",
"@nomiclabs/hardhat-ethers": "^2.0.0",
"@nomiclabs/hardhat-etherscan": "^3.0.0",
"@nomiclabs/hardhat-waffle": "^2.0.0",
Expand All @@ -41,6 +42,8 @@
"@types/node": "^12.0.0",
"@typescript-eslint/eslint-plugin": "^4.29.1",
"@typescript-eslint/parser": "^4.29.1",
"@uma/core": "^2.41.0",
"@uma/merkle-distributor": "^1.3.38",
"chai": "^4.2.0",
"dotenv": "^10.0.0",
"eslint": "^7.29.0",
Expand All @@ -60,8 +63,8 @@
"pretty-quick": "^2.0.1",
"solhint": "^3.3.6",
"solidity-coverage": "^0.7.16",
"ts-node": "^10.1.0",
"ts-mocha": "^9.0.2",
"ts-node": "^10.1.0",
"typechain": "^5.1.2",
"typescript": "^4.5.2"
},
Expand Down
3 changes: 0 additions & 3 deletions test/AcceleratingDistributor.Admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ describe("AcceleratingDistributor: Admin Functions", async function () {
await lpToken1.mint(rando.address, toWei(69));
await lpToken1.connect(rando).approve(distributor.address, toWei(69));
await distributor.connect(rando).stake(lpToken1.address, toWei(69));
console.log("a");
await expect(distributor.recoverToken(lpToken1.address)).to.be.revertedWith("Can't recover 0 tokens");
console.log("b");
// Mint additional tokens to the contract to simulate someone dropping them accidentally. This should be recoverable.
await lpToken1.mint(distributor.address, toWei(696));
await expect(() => distributor.recoverToken(lpToken1.address)).to.changeTokenBalances(
Expand All @@ -82,7 +80,6 @@ describe("AcceleratingDistributor: Admin Functions", async function () {
// The contract should be left with the original stake amount in it as this was not recoverable.
expect(await lpToken1.balanceOf(distributor.address)).to.equal(toWei(69));
await expect(distributor.recoverToken(lpToken1.address)).to.be.revertedWith("Can't recover 0 tokens");
console.log("c");
});
it("Can skim any amount of a random token", async function () {
const randomToken = await (await getContractFactory("TestToken", owner)).deploy("RANDO", "RANDO");
Expand Down
25 changes: 24 additions & 1 deletion test/AcceleratingDistributor.Events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,30 @@ describe("AcceleratingDistributor: Events", async function () {
stakeAmount.mul(3)
);
});
it("StakeFor", async function () {
const time1 = await distributor.getCurrentTime();

await expect(distributor.connect(depositor1).stakeFor(lpToken1.address, stakeAmount, rando.address))
.to.emit(distributor, "Stake")
.withArgs(lpToken1.address, rando.address, stakeAmount, time1, stakeAmount, stakeAmount);

// Subsequent stakes emit expected event. Advance time 420 seconds and stake 2x the amount.
await advanceTime(timer, 420);
const time2 = await distributor.getCurrentTime();
const avgDepositTime = time1.add(
stakeAmount.mul(2).mul(toWei(1)).div(stakeAmount.mul(3)).mul(time2.sub(time1)).div(toWei(1))
);
await expect(distributor.connect(depositor1).stakeFor(lpToken1.address, stakeAmount.mul(2), rando.address))
.to.emit(distributor, "Stake")
.withArgs(
lpToken1.address,
rando.address,
stakeAmount.mul(2),
avgDepositTime,
stakeAmount.mul(3),
stakeAmount.mul(3)
);
});
it("Unstake", async function () {
await distributor.connect(depositor1).stake(lpToken1.address, stakeAmount);

Expand Down Expand Up @@ -95,4 +119,3 @@ describe("AcceleratingDistributor: Events", async function () {
.withArgs(lpToken1.address, depositor1.address, 0);
});
});
``;
8 changes: 7 additions & 1 deletion test/AcceleratingDistributor.Fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ export const acceleratingDistributorFixture = hre.deployments.createFixture(asyn
const lpToken1 = await (await getContractFactory("TestToken", deployerWallet)).deploy("LP1", "LP Token 1");
const lpToken2 = await (await getContractFactory("TestToken", deployerWallet)).deploy("LP2", "LP Token 2");

return { timer, acrossToken, distributor, lpToken1, lpToken2 };
const merkleDistributor = await (await getContractFactory("MerkleDistributorTest", deployerWallet)).deploy();
const claimAndStake = await (
await getContractFactory("ClaimAndStake", deployerWallet)
).deploy(merkleDistributor.address, distributor.address);
await merkleDistributor.whitelistClaimer(claimAndStake.address, true);

return { timer, acrossToken, distributor, lpToken1, lpToken2, merkleDistributor, claimAndStake };
});

export async function enableTokenForStaking(distributor: Contract, lpToken: Contract, acrossToken: Contract) {
Expand Down
35 changes: 35 additions & 0 deletions test/AcceleratingDistributor.RewardAccumulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,41 @@ describe("AcceleratingDistributor: Staking Rewards", async function () {
// Rewards entitled to the user should be baseRewardsPerToken * multiplier * stakedBalance as 1 * 5 * 10 = 50
expect(await distributor.getOutstandingRewards(lpToken1.address, depositor1.address)).to.equal(toWei(50));
});
it("Staking on behalf of user", async function () {
await expect(() => distributor.connect(depositor1).stakeFor(lpToken1.address, stakeAmount, depositor2.address))
// Token balances should change as expected
.to.changeTokenBalances(lpToken1, [distributor, depositor1], [stakeAmount, stakeAmount.mul(-1)]);
// Should have correct staked amount.
expect((await distributor.getUserStake(lpToken1.address, depositor2.address)).cumulativeBalance).to.equal(
stakeAmount
);

// As no time has elapsed the rewards entitled to the user should be 0.
expect(await distributor.getOutstandingRewards(lpToken1.address, depositor2.address)).to.equal(toBN(0));

// The user should start with the reward multiplier of 1.
expect(await distributor.getUserRewardMultiplier(lpToken1.address, depositor2.address)).to.equal(toWei(1));

// Advance time forward 200 seconds. The user should be entitled to the entire amount of emissions (no pro-rata
// split as they are the only staker) * the increase from their multiplier. They were in the pool for 2/10 of the
// time to get to the max multiplier of 5 so the multiplier should be set to 1 + 200 / 1000 * (5 - 1) = 1.8. Therefore this
// should be duration * baseEmissionRate * multiplier = 200 * 0.01 * 1.8 = 3.6.
await advanceTime(timer, 200);
expect(await distributor.getUserRewardMultiplier(lpToken1.address, depositor2.address)).to.equal(toWei(1.8));
expect(await distributor.getOutstandingRewards(lpToken1.address, depositor2.address)).to.equal(toWei(3.6));

// The baseRewardPerToken should now be the deltaInTime * the baseEmissionRate / cumulativeStaked.
// i.e baseRewardPerToken = 200 * 0.01 / 10 = 0.1
expect(await distributor.baseRewardPerToken(lpToken1.address)).to.equal(toWei(0.2));

// Advance time forward another 800 seconds. We should now be at the max multiplier for the user of 5.
await advanceTime(timer, 800);
expect(await distributor.getUserRewardMultiplier(lpToken1.address, depositor2.address)).to.equal(toWei(5));
// baseRewardPerToken = previous baseRewardPerToken + 800 * 0.01 / 10 = 1
expect(await distributor.baseRewardPerToken(lpToken1.address)).to.equal(toWei(1));
// Rewards entitled to the user should be baseRewardsPerToken * multiplier * stakedBalance as 1 * 5 * 10 = 50
expect(await distributor.getOutstandingRewards(lpToken1.address, depositor2.address)).to.equal(toWei(50));
});
it("Changing staking token configuration can impact outstanding rewards", async function () {
// Same starting point as previous test after 200 seconds.
await distributor.connect(depositor1).stake(lpToken1.address, stakeAmount);
Expand Down
Loading