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.

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

feat: Add ClaimAndStake contract #32

Merged
merged 18 commits into from
Nov 3, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 28 additions & 20 deletions contracts/AcceleratingDistributor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -182,26 +182,7 @@ 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);

emit Stake(
stakedToken,
msg.sender,
amount,
averageDepositTime,
userDeposit.cumulativeBalance,
stakingTokens[stakedToken].cumulativeStaked
);
_stake(stakedToken, amount, msg.sender);
}

/**
Expand Down Expand Up @@ -391,4 +372,31 @@ 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(staker, address(this), amount);

emit Stake(
stakedToken,
staker,
amount,
averageDepositTime,
userDeposit.cumulativeBalance,
stakingTokens[stakedToken].cumulativeStaked
);
}
}
81 changes: 81 additions & 0 deletions contracts/AcceleratingDistributorClaimAndStake.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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/access/Ownable.sol";
import "@openzeppelin/contracts/utils/Multicall.sol";
import "@uma/core/contracts/merkle-distributor/implementation/MerkleDistributorInterface.sol";
import "./AcceleratingDistributor.sol";

/**
* @notice Across token distribution contract. Contract is inspired by Synthetix staking contract and Ampleforth geyser.
* Stakers start by earning their pro-rata share of a baseEmissionRate per second which increases based on how long
* they have staked in the contract, up to a max emission rate of baseEmissionRate * maxMultiplier. Multiple LP tokens
* can be staked in this contract enabling depositors to batch stake and claim via multicall. Note that this contract is
* only compatible with standard ERC20 tokens, and not tokens that charge fees on transfers, dynamically change
* balance, or have double entry-points. It's up to the contract owner to ensure they only add supported tokens.
*/

contract AcceleratingDistributorClaimAndStake is AcceleratingDistributor {
// Contract which rewards tokens to users that they can then stake. MerkleDistributor logic does not impact
// this contract at all, but its stored here for convenience to allow claimAndStake to be called by a user to
// claim their staking tokens and stake atomically.
MerkleDistributorInterface public merkleDistributor;

/**************************************
* EVENTS *
**************************************/

event SetMerkleDistributor(address indexed newMerkleDistributor);

constructor(address _rewardToken) AcceleratingDistributor(_rewardToken) {}

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

/**
* @notice Resets merkle distributor contract called in claimAndStake()
* @param _merkleDistributor Address to set merkleDistributor to.
*/
function setMerkleDistributor(address _merkleDistributor) external onlyOwner {
merkleDistributor = MerkleDistributorInterface(_merkleDistributor);
emit SetMerkleDistributor(_merkleDistributor);
}

/**************************************
* STAKER FUNCTIONS *
**************************************/

/**
* @notice Claim tokens from a MerkleDistributor contract and stake them for rewards.
* @dev Will revert if `merkleDistributor` is not set to valid MerkleDistributor contract.
* @dev Will revert if any of the claims recipient accounts are not equal to caller, or if any reward token
* for claim is not a valid staking token or are not the same token as the other claims.
* @dev Will revert if this contract is not a "whitelisted claimer" on the MerkleDistributor contract.
* @dev The caller of this function must approve this contract to spend total amount of stakedToken.
* @param claims Claim leaves to retrieve from MerkleDistributor.
* @param stakedToken The address of the token to stake.
*/
function claimAndStake(MerkleDistributorInterface.Claim[] memory claims, address stakedToken)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want to throw a wrench into anything, but I was brainstorming a bit and wanted to ask about a slightly different implementation/strategy.

This would introduce less code in this contract, but may add more code (or more complex code) in the merkle distributor and may have a slightly better business outcome (one transaction, no approval):

  • Add a stakeFor() method in this contract (basically just a wrapper function around _stake() that you have above).
  • Drop the claimAndStake() method.
  • Add a claimFor method in the merkle distributor that allows a contract to claim on behalf of a user (and actually receive the tokens). This could be done via signature or whitelist.
  • Build a special intermediary contract that does the following:
    • This contract would have a single method to do 2 things:
      • Claim their airdrop (and pull it into this contract).
      • Call stakeFor() to stake on behalf of the user.

This process has a few advantages:

  1. Lighter modifications to the AcceleratingDistributor (still some, though), which holds user funds.
  2. Single transaction claim (no approvals).

Downsides:

  1. More cross contract calls.
  2. One more contract to deploy.

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 think this is a good idea I'll start work on the AcrossMerkleDistributor

external
nonReentrant
onlyEnabled(stakedToken)
{
uint256 batchedAmount;
uint256 claimCount = claims.length;
for (uint256 i = 0; i < claimCount; i++) {
MerkleDistributorInterface.Claim memory _claim = claims[i];
require(_claim.account == msg.sender, "claim account not caller");
require(
merkleDistributor.getRewardTokenForWindow(_claim.windowIndex) == stakedToken,
kevinuma marked this conversation as resolved.
Show resolved Hide resolved
"unexpected claim token"
);
batchedAmount += _claim.amount;
}
merkleDistributor.claimMulti(claims);
_stake(stakedToken, batchedAmount, msg.sender);
}
}
9 changes: 6 additions & 3 deletions contracts/test/AcceleratingDistributor_Testable.sol
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity ^0.8.0;

import "../AcceleratingDistributor.sol";
import "../AcceleratingDistributorClaimAndStake.sol";
import "./Testable.sol";

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

contract AcceleratingDistributor_Testable is AcceleratingDistributor, Testable {
constructor(address _rewardToken, address _timer) AcceleratingDistributor(_rewardToken) Testable(_timer) {}
contract AcceleratingDistributor_Testable is AcceleratingDistributorClaimAndStake, Testable {
constructor(address _rewardToken, address _timer)
AcceleratingDistributorClaimAndStake(_rewardToken)
Testable(_timer)
{}

function getCurrentTime() public view override(AcceleratingDistributor, Testable) returns (uint256) {
return Testable.getCurrentTime();
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 {

}
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.6",
"@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.40.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
162 changes: 162 additions & 0 deletions test/AcceleratingDistributor.ClaimAndStake.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
import { expect, ethers, Contract, SignerWithAddress, toWei, toBN } from "./utils";
import { acceleratingDistributorFixture, enableTokenForStaking } from "./AcceleratingDistributor.Fixture";
import { MAX_UINT_VAL } from "@uma/common";
import { MerkleTree } from "@uma/merkle-distributor";
import { baseEmissionRate, maxMultiplier, secondsToMaxMultiplier } from "./constants";

let acrossToken: Contract, distributor: Contract, lpToken1: Contract, claimer: SignerWithAddress;
let merkleDistributor: Contract, contractCreator: SignerWithAddress, lpToken2: Contract;

type Recipient = {
account: string;
amount: string;
accountIndex: number;
};

type RecipientWithProof = Recipient & {
windowIndex: number;
merkleProof: Buffer[];
};

const createLeaf = (recipient: Recipient) => {
expect(Object.keys(recipient).every((val) => ["account", "amount", "accountIndex"].includes(val))).to.be.true;

return Buffer.from(
ethers.utils
.solidityKeccak256(
["address", "uint256", "uint256"],
[recipient.account, recipient.amount, recipient.accountIndex]
)
.slice(2),
"hex"
);
};

const window1RewardAmount = toBN(toWei("100"));
const window2RewardAmount = toBN(toWei("300"));
const totalBatchRewards = window1RewardAmount.add(window2RewardAmount);
let batchedClaims: RecipientWithProof[];

describe("AcceleratingDistributor: Atomic Claim and Stake", async function () {
beforeEach(async function () {
[contractCreator, claimer] = await ethers.getSigners();
({ distributor, acrossToken, lpToken1, lpToken2, merkleDistributor } = await acceleratingDistributorFixture());
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved

// Enable reward token for staking.
await enableTokenForStaking(distributor, lpToken1, acrossToken);

// Seed MerkleDistributor with reward tokens.
await lpToken1.connect(contractCreator).mint(contractCreator.address, MAX_UINT_VAL);
await lpToken1.connect(contractCreator).approve(merkleDistributor.address, MAX_UINT_VAL);

// Set two windows with trivial one leaf trees.
const reward1Recipients = [
{
account: claimer.address,
amount: window1RewardAmount.toString(),
accountIndex: 0,
},
];
const reward2Recipients = [
{
account: claimer.address,
amount: window2RewardAmount.toString(),
accountIndex: 0,
},
];

const merkleTree1 = new MerkleTree(reward1Recipients.map((item) => createLeaf(item)));
await merkleDistributor
.connect(contractCreator)
.setWindow(window1RewardAmount, lpToken1.address, merkleTree1.getRoot(), "");

const merkleTree2 = new MerkleTree(reward2Recipients.map((item) => createLeaf(item)));
await merkleDistributor
.connect(contractCreator)
.setWindow(window2RewardAmount, lpToken1.address, merkleTree2.getRoot(), "");

// Construct claims for all trees assuming that each tree index is equal to its window index.
batchedClaims = [
{
windowIndex: 0,
account: reward1Recipients[0].account,
accountIndex: reward1Recipients[0].accountIndex,
amount: reward1Recipients[0].amount,
merkleProof: merkleTree1.getProof(createLeaf(reward1Recipients[0])),
},
{
windowIndex: 1,
account: reward2Recipients[0].account,
accountIndex: reward2Recipients[0].accountIndex,
amount: reward2Recipients[0].amount,
merkleProof: merkleTree2.getProof(createLeaf(reward2Recipients[0])),
},
];
expect(await lpToken1.balanceOf(claimer.address)).to.equal(toBN(0));

// Tests require staker to have approved contract
await lpToken1.connect(claimer).approve(distributor.address, MAX_UINT_VAL);
});

it("Happy path", async function () {
const time = await distributor.getCurrentTime();
await expect(distributor.connect(claimer).claimAndStake(batchedClaims, lpToken1.address))
.to.emit(distributor, "Stake")
.withArgs(lpToken1.address, claimer.address, totalBatchRewards, time, totalBatchRewards, totalBatchRewards);
expect((await distributor.getUserStake(lpToken1.address, claimer.address)).cumulativeBalance).to.equal(
totalBatchRewards
);
expect(await lpToken1.balanceOf(merkleDistributor.address)).to.equal(toBN(0));
expect(await lpToken1.balanceOf(claimer.address)).to.equal(toBN(0));
});
it("Fails if AcceleratingDistributor is not whitelisted claimer on MerkleDistributor", async function () {
await merkleDistributor.whitelistClaimer(distributor.address, false);
await expect(distributor.connect(claimer).claimAndStake(batchedClaims, lpToken1.address)).to.be.revertedWith(
"invalid claimer"
);
});
it("MerkleDistributor set to invalid address", async function () {
await distributor.setMerkleDistributor(distributor.address);
// distributor is not a valid MerkleDistributor and error explains that.
await expect(distributor.connect(claimer).claimAndStake(batchedClaims, lpToken1.address)).to.be.revertedWith(
"function selector was not recognized and there's no fallback function"
);
});
it("Only owner can set MerkleDistributor address", async function () {
await expect(distributor.connect(claimer).setMerkleDistributor(distributor.address)).to.be.revertedWith(
"Ownable: caller is not the owner"
);
});
it("One claim account is not caller", async function () {
// Claiming with account that isn't receiving the claims causes revert
await expect(
distributor.connect(contractCreator).claimAndStake(batchedClaims, lpToken1.address)
).to.be.revertedWith("claim account not caller");
});
it("One claim reward token is not staked token", async function () {
// Enable new staking token that doesn't match claims.
await distributor.configureStakingToken(
lpToken2.address,
true,
baseEmissionRate,
maxMultiplier,
secondsToMaxMultiplier
);
await expect(distributor.connect(claimer).claimAndStake(batchedClaims, lpToken2.address)).to.be.revertedWith(
"unexpected claim token"
);
});
it("Claimed token is not eligible for staking", async function () {
kevinuma marked this conversation as resolved.
Show resolved Hide resolved
// Disable staking token
await distributor.configureStakingToken(
lpToken1.address,
false,
baseEmissionRate,
maxMultiplier,
secondsToMaxMultiplier
);
await expect(distributor.connect(claimer).claimAndStake(batchedClaims, lpToken1.address)).to.be.revertedWith(
"stakedToken not enabled"
);
});
});
1 change: 0 additions & 1 deletion test/AcceleratingDistributor.Events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,3 @@ describe("AcceleratingDistributor: Events", async function () {
.withArgs(lpToken1.address, depositor1.address, 0);
});
});
``;