From b7fbc56932e97fe920e2e107a57c4de341264727 Mon Sep 17 00:00:00 2001 From: shubham-kanodia Date: Sat, 7 May 2022 22:50:10 +0530 Subject: [PATCH 1/4] Optimize unstake for gas --- contracts/Staking.sol | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/contracts/Staking.sol b/contracts/Staking.sol index 9b13fa5..f1e4cf5 100644 --- a/contracts/Staking.sol +++ b/contracts/Staking.sol @@ -54,23 +54,24 @@ contract Staking { * @dev This prevents 3 things: * - Unstaking non-existing NFTs (ERC721 ownerOf validates existence) * - Unstaking if msg.sender is not the owner - * - Unstaking NFTs that are not staked (since staked[tokenId].owner will be the zero address) + * - Unstaking NFTs that are not staked (since staking.owner will be the zero address) */ + Stake memory staking = staked[tokenId]; + require( - staked[tokenId].owner == msg.sender, + staking.owner == msg.sender, "Staking: only the owner can unstake an NFT" ); require( - staked[tokenId].initTimestamp != block.timestamp, + staking.initTimestamp != block.timestamp, "Staking: cannot unstake an NFT in the same block it was staked" ); - uint256 stakedSeconds = block.timestamp - staked[tokenId].initTimestamp; + uint256 stakedSeconds = block.timestamp - staking.initTimestamp; - delete staked[tokenId]; + delete staking; dappCampWarriors.transferFrom(address(this), msg.sender, tokenId); - camp.mint(msg.sender, stakedSeconds * rewardPerSecondInWei); } } From ba53e1d9a7d1e6788eafb47868831df88a9516c1 Mon Sep 17 00:00:00 2001 From: shubham-kanodia Date: Sat, 7 May 2022 23:08:16 +0530 Subject: [PATCH 2/4] Add gas check for unstake --- test/Staking.spec.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/Staking.spec.ts b/test/Staking.spec.ts index c2408ca..b6692aa 100644 --- a/test/Staking.spec.ts +++ b/test/Staking.spec.ts @@ -154,16 +154,16 @@ describe("Camp tests", () => { ethers.utils.parseEther("1") ); }); + }); - it("Should remove the NFT from the `staked` mapping", async () => { - await createValidStake(); - const stakedData1 = await stakingContract.staked(1); - expect(stakedData1.tokenId).to.equal(1); + describe("gasCheck", () => { + it("Should check the gas used for unstake is below minimum threshold", async () => { + const GAS_THRESHOLD = 120_000; - await stakingContract.unstake(1); + await createValidStake(); + const gasUsed = await stakingContract.estimateGas.unstake(1); - const stakedData2 = await stakingContract.staked(1); - expect(stakedData2.tokenId).to.equal(0); + expect(gasUsed).lt(GAS_THRESHOLD); }); }); }); From a7787b8bc63ae4b2dbdb229dde4cd48b2e10c276 Mon Sep 17 00:00:00 2001 From: shubham-kanodia Date: Sun, 8 May 2022 00:12:22 +0530 Subject: [PATCH 3/4] Add ICamp interface to reduce gas cost --- contracts/Staking.sol | 19 +++++++++---------- contracts/interfaces/ICamp.sol | 6 ++++++ 2 files changed, 15 insertions(+), 10 deletions(-) create mode 100644 contracts/interfaces/ICamp.sol diff --git a/contracts/Staking.sol b/contracts/Staking.sol index f1e4cf5..3edf53e 100644 --- a/contracts/Staking.sol +++ b/contracts/Staking.sol @@ -2,13 +2,12 @@ pragma solidity ^0.8.4; import "@openzeppelin/contracts/interfaces/IERC721.sol"; - -import "./Camp.sol"; +import "./interfaces/ICamp.sol"; /* solhint-disable not-rely-on-time */ contract Staking { - Camp public camp; - IERC721 public dappCampWarriors; + address public campAddress; + address public dappCampWarriorsAddress; struct Stake { address owner; @@ -27,8 +26,8 @@ contract Staking { uint256 public rewardPerSecondInWei = 1000000000000000000; constructor(address _camp, address _dappCampWarriors) { - camp = Camp(_camp); - dappCampWarriors = IERC721(_dappCampWarriors); + campAddress = _camp; + dappCampWarriorsAddress = _dappCampWarriors; } function stake(uint256 tokenId) public { @@ -39,11 +38,11 @@ contract Staking { * - Staking NFTs that are already staked (since the owner is this contract) */ require( - dappCampWarriors.ownerOf(tokenId) == msg.sender, + IERC721(dappCampWarriorsAddress).ownerOf(tokenId) == msg.sender, "Staking: only the owner can stake an NFT" ); - dappCampWarriors.transferFrom(msg.sender, address(this), tokenId); + IERC721(dappCampWarriorsAddress).transferFrom(msg.sender, address(this), tokenId); // solhint-disable-next-line not-rely-on-time staked[tokenId] = Stake(msg.sender, tokenId, block.timestamp); @@ -71,8 +70,8 @@ contract Staking { delete staking; - dappCampWarriors.transferFrom(address(this), msg.sender, tokenId); - camp.mint(msg.sender, stakedSeconds * rewardPerSecondInWei); + IERC721(dappCampWarriorsAddress).transferFrom(address(this), msg.sender, tokenId); + ICamp(campAddress).mint(msg.sender, stakedSeconds * rewardPerSecondInWei); } } /* solhint-enable not-rely-on-time */ diff --git a/contracts/interfaces/ICamp.sol b/contracts/interfaces/ICamp.sol new file mode 100644 index 0000000..816a0fd --- /dev/null +++ b/contracts/interfaces/ICamp.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.4; + +interface ICamp { + function mint(address account, uint256 amount) external; +} From 44e37fbc4b4232e7433843747d1bfa8b757bad54 Mon Sep 17 00:00:00 2001 From: shubham-kanodia Date: Sun, 8 May 2022 00:20:53 +0530 Subject: [PATCH 4/4] Add gas checks for contract creation and staking --- test/Staking.spec.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/Staking.spec.ts b/test/Staking.spec.ts index b6692aa..5764791 100644 --- a/test/Staking.spec.ts +++ b/test/Staking.spec.ts @@ -157,6 +157,29 @@ describe("Camp tests", () => { }); describe("gasCheck", () => { + it("Should check the gas used for contract creation is below minimum threshold", async () => { + const GAS_THRESHOLD = 755_000; + + const StakingFactory = await ethers.getContractFactory("Staking"); + const stakingContract = await StakingFactory.deploy(campContract.address, dappCampWarriorsContract.address); + + const gasLimit = stakingContract.deployTransaction.gasLimit; + + expect(gasLimit).lt(GAS_THRESHOLD); + }); + + it("Should check the gas used for staking is below minimum threshold", async () => { + const GAS_THRESHOLD = 142_000; + + await ( + await dappCampWarriorsContract.approve(stakingContract.address, 1) + ).wait(); + + const gasUsed = await stakingContract.estimateGas.stake(1); + + expect(gasUsed).lt(GAS_THRESHOLD); + }); + it("Should check the gas used for unstake is below minimum threshold", async () => { const GAS_THRESHOLD = 120_000;