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

Gas Optimisation Fixes #8

Open
wants to merge 4 commits into
base: 7-gas-optimization
Choose a base branch
from
Open
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
32 changes: 16 additions & 16 deletions contracts/Staking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -54,24 +53,25 @@ 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;

delete staked[tokenId];
uint256 stakedSeconds = block.timestamp - staking.initTimestamp;

dappCampWarriors.transferFrom(address(this), msg.sender, tokenId);
delete staking;

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 */
6 changes: 6 additions & 0 deletions contracts/interfaces/ICamp.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

interface ICamp {
function mint(address account, uint256 amount) external;
}
37 changes: 30 additions & 7 deletions test/Staking.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,39 @@ 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 contract creation is below minimum threshold", async () => {
const GAS_THRESHOLD = 755_000;

await stakingContract.unstake(1);
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;

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);
});
});
});