Evergreen fox farm - smart contract work

0xean commented 2 months ago

(Ticketing here, since we will want to create a new repo for the SC)

Background

The DFC has requested that engineering deploy a different smart contract that can be used for LP farms and doesn't require a continual migration each time the program renews.

One such contract (which is a fork of our currently forked contracts) can be found here https://github.com/Synthetixio/synthetix/blob/develop/contracts/StakingRewards.sol

It allows the DFC to renew the program indefinitely using the same contract.

Problems

One downside of this is the version of the contract has some additional functionality with the RewardsDistribution.sol contract, that we do no want or need.

This contract used to be called the StakingRewardsFactory which you can see our version of here.

Solution

Since we only need to deploy the StakingRewards.sol contract once, we really do not need the factory or the RewardsDistribution.sol contract at all. StakingRewards can be funded and called directly from the multisig if deployed correctly.

Risks

This is a bit of a departure from how Synthetix interacts with their contract, but with some basic unit tests we should be able to comfortable with this approach.

AC

for the StakingRewards contract into a new repo
copy over hardhat infra from synthetix
create basic unit tests to show our interaction pattern (some of these could be copied from synthetic)
raise any concerns and if none deploy the contract to mainnet and transfer ownership to the DAO and create a basic README for the msig signers to follow for interaction
copy over ABIs and address to web for further implementation

Unit tests:

deployment and funding from an arbitrary account vs RewardsDistribution (calling notifyRewardsAmount)
running a basic rewards program to its end
creating and funding a new rewards program
transferring ownership

ideally leave solidity version intact

0xean commented 2 months ago

current rewards expire on sep 26

0xApotheosis commented 2 months ago

I hit a stopping point yesterday attempting to get the forked contract working with the Foundry/Forge test suite whilst maintaining the contract's version of ^0.5.16. I got it compiling with forge build, however we are unable to utilize Forge's testing library, forge-std/Test.sol, as it has a minimum version of 0.6.2. This negates most of the benefit of using the Foundry toolchain.

We have 3 options to move forward:

Hand-roll our own testing utilts to work with Foundry
Bump the Solidity version of the forked contract
Use Hardhat

Discussed with @woodenfurniture and decided that using Hardhat with the contract's current Solidity version is the best path forward. I'll proceed accordingly.

0xean commented 2 months ago

I think this sounds good. Hardhat for what we are doing shouldn't be too hard to work with. Thanks for documenting your decisions here.

0xApotheosis commented 1 month ago

Repo with full test suit and deployment script here: https://github.com/shapeshift/fox-farm-sc

Contract deployed via Hardhat Ignition here: https://etherscan.io/address/0xe7e16e2b05440c2e484c5c41ac3e5a4d15da2744 in TX https://etherscan.io/tx/0x9996cb97f9415ac5761bae8bef06b8cfbe4483280dd0e792ecce22fa7e427595

Rewards token set to 0xc770EEfAd204B5180dF6a14Ee197D99d808ee52d (FOX)
Staking token set to 0x470e8de2eBaef52014A47Cb5E6aF86884947F08c (ETH/FOX pool)
Owner set to 0x90A48D5CF7343B08dA12E067680B4C6dbfE551Be // DAO multi-sig

TODO from DAO multi-sig:

[ ] Accept ownership via acceptOwnership
[ ] Set rewardsDuration via setRewardsDuration to the desired duration (in seconds)

@0xean could be do a check-in on this when you are next available?

I haven't yet verified the contract on Etherscan, but I'm guessing we'll want to do this.

0xApotheosis commented 1 month ago

✅ Contract verified on Etherscan: https://etherscan.io/address/0xe7e16e2b05440c2e484c5c41ac3e5a4d15da2744#code

0xean commented 1 month ago

Great work @0xApotheosis - taking a look at all this today.

0xean commented 1 month ago

Hey @0xApotheosis I think there is an issue that we need to address, test, and then re-deploy.

This was probably a bit hard to understand from the ticket and we can certainly chat about it synchronously, but will try to summarize here also.

Problem

https://github.com/shapeshift/evergreen-fox-contract/blob/d0e3270d1ee2f3a0edb373d89ce6e7290b5ebcca/ignition/modules/StakingRewards.ts#L3

Currently in the deployment script you are deploying the new contract deploying a previous version of the Factory contract. If that contract is then used again with the same staking token, it will send all fox to the OLD rewards contract as seen here

https://github.com/shapeshift/fox-staking-3/blob/7486c68ae0ca5e1f765b38509b6492bff9fe89fc/contracts/StakingRewardsFactory.sol#L75

This is obviously problematic and would brick a bunch of fox.

Solution

I hinted at this in the ticket,

Since we only need to deploy the StakingRewards.sol contract once, we really do not need the factory or the RewardsDistribution.sol contract at all. StakingRewards can be funded and called directly from the multisig if deployed correctly.

But certainly wasn't super clear. We actually don't need a Factory / rewardsDistribution contract at all. We can just deploy the StakingRewards contract as you have done and then pass in the msig address in place of the Factory! Once this is done, the msig can simply transfer FOX into the StakingRewards contract, set the desired duration and call notifyRewardAmount as desired.

This is the piece that could use a little bit of additional testing. In hardhat, can an arbitrary address do exactly what I am suggesting above and have it all work. I think the answer is yes, but testing is a great idea.

Let me know if this makes sense, happy to work on some of the tests together or review in a pile whenever.

0xApotheosis commented 1 month ago

Thanks @0xean, that makes total sense. The miscommunication is my bad - I copied the reward address from the previous contract but the intent was for the msig to update it to the desired address via setRewardsDistribution.

However - knowing now the owner & reward addresses should be the same (both the msig), I've updated the deploy script in https://github.com/shapeshift/evergreen-fox-contract/commit/10dff43b12f20cccc7766404fa0298c850da32a9 to do just this.

I also added a test to ensure that the same arbitrary address can be used as both the owner and the rewards address, and that the notifyRewardAmount can then be called from that address: https://github.com/shapeshift/evergreen-fox-contract/commit/692be46ea2e132a891ab670601a34220b4aec714.

We don't necessarily need to redeploy as the rewards address can be updated by the owner (msig) via setRewardsDistribution to itself, but we certainly can if that's easier.

Happy to pile on this tomorrow if you're back in to ensure we're all aligned.

0xean commented 1 month ago

We don't necessarily need to redeploy as the rewards address can be updated by the owner (msig) via setRewardsDistribution to itself, but we certainly can if that's easier.

Ahh, you are correct, I didn't see that setter. In that case, I don't think we need to redeploy.

0xean commented 1 month ago

Closing as complete pending update to readme for accepting ownership by msig signers (@0xApotheosis)

0xApotheosis commented 1 month ago

Confirming README updated with acceptOwnership instructions in https://github.com/shapeshift/evergreen-fox-contract/commit/8009ad46f55ba31e20b4b67f0f50a9084cfe7fba.

shapeshift / web