From 29db1cdc8455a04736fcc2fc5e5cf6221aa5050d Mon Sep 17 00:00:00 2001 From: GitGuru7 <128375421+GitGuru7@users.noreply.github.com> Date: Tue, 19 Dec 2023 19:01:20 +0530 Subject: [PATCH 1/5] feat: add TimeManagerV5 and TimeManagerV8 --- contracts/TimeManagerV5.sol | 58 ++++++++++++++++++++++++++++++++ contracts/TimeManagerV8.sol | 60 ++++++++++++++++++++++++++++++++++ hardhat.config.ts | 16 +++++++++ tests/hardhat/TimeManagerV5.ts | 34 +++++++++++++++++++ 4 files changed, 168 insertions(+) create mode 100644 contracts/TimeManagerV5.sol create mode 100644 contracts/TimeManagerV8.sol create mode 100644 tests/hardhat/TimeManagerV5.ts diff --git a/contracts/TimeManagerV5.sol b/contracts/TimeManagerV5.sol new file mode 100644 index 0000000..b8ba105 --- /dev/null +++ b/contracts/TimeManagerV5.sol @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity 0.5.16; + +contract TimeManagerV5 { + /// @dev The approximate number of seconds per year + uint256 public constant SECONDS_PER_YEAR = 31_536_000; + + /// @notice Number of blocks per year or seconds per year + uint256 public blocksOrSecondsPerYear; + + /// @dev Sets true when block timestamp is used + bool public isTimeBased; + + /** + * @dev Retrieves the current slot + * @return Current slot + */ + function() view returns (uint256) private _getCurrentSlot; + + /** + * @param timeBased_ A boolean indicating whether the contract is based on time or block. + * If timeBased is true than blocksPerYear_ param is ignored as blocksOrSecondsPerYear is set to SECONDS_PER_YEAR + * @param blocksPerYear_ The number of blocks per year + */ + constructor(bool timeBased_, uint256 blocksPerYear_) public { + if (!timeBased_ && blocksPerYear_ == 0) { + revert("Invalid Blocks Per year"); + } + isTimeBased = timeBased_; + blocksOrSecondsPerYear = timeBased_ ? SECONDS_PER_YEAR : blocksPerYear_; + _getCurrentSlot = timeBased_ ? _getBlockTimestamp : _getBlockNumber; + } + + /** + * @dev Function to simply retrieve block number or block timestamp + * This exists mainly for inheriting test contracts to stub this result. + * @return Current block number or block timestamp + */ + function getBlockNumberOrTimestamp() public view returns (uint256) { + return _getCurrentSlot(); + } + + /** + * @notice Returns the current timestamp in seconds + * @return The current timestamp + */ + function _getBlockTimestamp() private view returns (uint256) { + return block.timestamp; + } + + /** + * @notice Returns the current block number + * @return The current block number + */ + function _getBlockNumber() private view returns (uint256) { + return block.number; + } +} diff --git a/contracts/TimeManagerV8.sol b/contracts/TimeManagerV8.sol new file mode 100644 index 0000000..80194f8 --- /dev/null +++ b/contracts/TimeManagerV8.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: BSD-3-Clause +pragma solidity 0.8.13; + +import { SECONDS_PER_YEAR } from "./constants.sol"; + +abstract contract TimeManagerV8 { + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable + uint256 public immutable blocksOrSecondsPerYear; + + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable + bool public immutable isTimeBased; + + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable + function() view returns (uint256) private immutable _getCurrentSlot; + + /// @notice Thrown on invaid arguments + error InvalidBlocksPerYear(); + + /** + * @param timeBased_ A boolean indicating whether the contract is based on time or block. + * If timeBased is true than blocksPerYear_ param is ignored as blocksOrSecondsPerYear is set to SECONDS_PER_YEAR + * @param blocksPerYear_ The number of blocks per year + * @custom:error InvalidBlocksPerYear is thrown if blocksPerYear entered is zero + * @custom:oz-upgrades-unsafe-allow constructor + */ + constructor(bool timeBased_, uint256 blocksPerYear_) { + if (!timeBased_ && blocksPerYear_ == 0) { + revert InvalidBlocksPerYear(); + } + + isTimeBased = timeBased_; + blocksOrSecondsPerYear = timeBased_ ? SECONDS_PER_YEAR : blocksPerYear_; + _getCurrentSlot = timeBased_ ? _getBlockTimestamp : _getBlockNumber; + } + + /** + * @dev Function to simply retrieve block number or block timestamp + * This exists mainly for inheriting test contracts to stub this result. + * @return Current block number or block timestamp + */ + function getBlockNumberOrTimestamp() public view virtual returns (uint256) { + return _getCurrentSlot(); + } + + /** + * @notice Returns the current timestamp in seconds + * @return The current timestamp + */ + function _getBlockTimestamp() private view returns (uint256) { + return block.timestamp; + } + + /** + * @notice Returns the current block number + * @return The current block number + */ + function _getBlockNumber() private view returns (uint256) { + return block.number; + } +} diff --git a/hardhat.config.ts b/hardhat.config.ts index f80b120..29b5abd 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -47,6 +47,22 @@ const config: HardhatUserConfig = { }, }, }, + { + version: "0.5.16", + settings: { + optimizer: { + enabled: true, + details: { + yul: !process.env.CI, + }, + }, + outputSelection: { + "*": { + "*": ["storageLayout"], + }, + }, + }, + }, ], }, networks: { diff --git a/tests/hardhat/TimeManagerV5.ts b/tests/hardhat/TimeManagerV5.ts new file mode 100644 index 0000000..f27c852 --- /dev/null +++ b/tests/hardhat/TimeManagerV5.ts @@ -0,0 +1,34 @@ +import chai from "chai"; +import { ethers } from "hardhat"; + +const { expect } = chai; + +describe("TimeManagerV5: tests", async () => { + let timeManager: ethers.Contracts; + describe("For block number", async () => { + const blocksPerYear = 10512000; + beforeEach(async () => { + const timeManagerV5 = await ethers.getContractFactory("TimeManagerV5"); + timeManager = await timeManagerV5.deploy(false, blocksPerYear); + }); + + it("Retrieves block timestamp", async () => { + const currentBlockNumber = (await ethers.provider.getBlock("latest")).number; + expect(await timeManager.getBlockNumberOrTimestamp()).to.be.equal(currentBlockNumber); + expect(await timeManager.blocksOrSecondsPerYear()).to.be.equal(blocksPerYear); + }); + }); + describe("For block timestamp", async () => { + beforeEach(async () => { + const timeManagerV5 = await ethers.getContractFactory("TimeManagerV5"); + timeManager = await timeManagerV5.deploy(true, 0); + }); + + it("Retrieves block timestamp", async () => { + const secondsPerYear = await timeManager.SECONDS_PER_YEAR(); + const currentBlocktimestamp = (await ethers.provider.getBlock("latest")).timestamp; + expect(await timeManager.getBlockNumberOrTimestamp()).to.be.equal(currentBlocktimestamp); + expect(await timeManager.blocksOrSecondsPerYear()).to.be.equal(secondsPerYear); + }); + }); +}); From d037cf33114552e730aee83e209bd4c937d73703 Mon Sep 17 00:00:00 2001 From: GitGuru7 <128375421+GitGuru7@users.noreply.github.com> Date: Wed, 20 Dec 2023 11:11:49 +0530 Subject: [PATCH 2/5] fix: comments and revert condition --- contracts/TimeManagerV5.sol | 7 +++++-- contracts/TimeManagerV8.sol | 12 +++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/contracts/TimeManagerV5.sol b/contracts/TimeManagerV5.sol index b8ba105..6af8cd3 100644 --- a/contracts/TimeManagerV5.sol +++ b/contracts/TimeManagerV5.sol @@ -24,8 +24,12 @@ contract TimeManagerV5 { */ constructor(bool timeBased_, uint256 blocksPerYear_) public { if (!timeBased_ && blocksPerYear_ == 0) { - revert("Invalid Blocks Per year"); + revert("Invalid blocks per year"); } + if (timeBased_ && blocksPerYear_ != 0) { + revert("Invalid time based"); + } + isTimeBased = timeBased_; blocksOrSecondsPerYear = timeBased_ ? SECONDS_PER_YEAR : blocksPerYear_; _getCurrentSlot = timeBased_ ? _getBlockTimestamp : _getBlockNumber; @@ -33,7 +37,6 @@ contract TimeManagerV5 { /** * @dev Function to simply retrieve block number or block timestamp - * This exists mainly for inheriting test contracts to stub this result. * @return Current block number or block timestamp */ function getBlockNumberOrTimestamp() public view returns (uint256) { diff --git a/contracts/TimeManagerV8.sol b/contracts/TimeManagerV8.sol index 80194f8..e260368 100644 --- a/contracts/TimeManagerV8.sol +++ b/contracts/TimeManagerV8.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.13; import { SECONDS_PER_YEAR } from "./constants.sol"; -abstract contract TimeManagerV8 { +abstract contract TimeManager { /// @custom:oz-upgrades-unsafe-allow state-variable-immutable uint256 public immutable blocksOrSecondsPerYear; @@ -13,9 +13,12 @@ abstract contract TimeManagerV8 { /// @custom:oz-upgrades-unsafe-allow state-variable-immutable function() view returns (uint256) private immutable _getCurrentSlot; - /// @notice Thrown on invaid arguments + /// @notice Thrown when blocks per year is invalid error InvalidBlocksPerYear(); + /// @notice Thrown when time based but blocks per year is provided + error InvalidTimeBased(); + /** * @param timeBased_ A boolean indicating whether the contract is based on time or block. * If timeBased is true than blocksPerYear_ param is ignored as blocksOrSecondsPerYear is set to SECONDS_PER_YEAR @@ -28,6 +31,10 @@ abstract contract TimeManagerV8 { revert InvalidBlocksPerYear(); } + if (timeBased_ && blocksPerYear_ != 0) { + revert InvalidTimeBased(); + } + isTimeBased = timeBased_; blocksOrSecondsPerYear = timeBased_ ? SECONDS_PER_YEAR : blocksPerYear_; _getCurrentSlot = timeBased_ ? _getBlockTimestamp : _getBlockNumber; @@ -35,7 +42,6 @@ abstract contract TimeManagerV8 { /** * @dev Function to simply retrieve block number or block timestamp - * This exists mainly for inheriting test contracts to stub this result. * @return Current block number or block timestamp */ function getBlockNumberOrTimestamp() public view virtual returns (uint256) { From 066450f79d1c0872ed9baa39f55ebb2952ebb33f Mon Sep 17 00:00:00 2001 From: GitGuru7 <128375421+GitGuru7@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:32:11 +0530 Subject: [PATCH 3/5] fix: change contract name --- contracts/TimeManagerV8.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/TimeManagerV8.sol b/contracts/TimeManagerV8.sol index e260368..eb84c9f 100644 --- a/contracts/TimeManagerV8.sol +++ b/contracts/TimeManagerV8.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.13; import { SECONDS_PER_YEAR } from "./constants.sol"; -abstract contract TimeManager { +abstract contract TimeManagerV8 { /// @custom:oz-upgrades-unsafe-allow state-variable-immutable uint256 public immutable blocksOrSecondsPerYear; From 05d19627e649cc456e15dcf2da9671d19ccfe4fa Mon Sep 17 00:00:00 2001 From: GitGuru7 <128375421+GitGuru7@users.noreply.github.com> Date: Wed, 20 Dec 2023 15:56:57 +0530 Subject: [PATCH 4/5] fix: netscape --- contracts/TimeManagerV5.sol | 8 ++++---- contracts/TimeManagerV8.sol | 13 +++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/contracts/TimeManagerV5.sol b/contracts/TimeManagerV5.sol index 6af8cd3..033cc88 100644 --- a/contracts/TimeManagerV5.sol +++ b/contracts/TimeManagerV5.sol @@ -18,7 +18,7 @@ contract TimeManagerV5 { function() view returns (uint256) private _getCurrentSlot; /** - * @param timeBased_ A boolean indicating whether the contract is based on time or block. + * @param timeBased_ A boolean indicating whether the contract is based on time or block * If timeBased is true than blocksPerYear_ param is ignored as blocksOrSecondsPerYear is set to SECONDS_PER_YEAR * @param blocksPerYear_ The number of blocks per year */ @@ -27,7 +27,7 @@ contract TimeManagerV5 { revert("Invalid blocks per year"); } if (timeBased_ && blocksPerYear_ != 0) { - revert("Invalid time based"); + revert("Invalid time based configuration"); } isTimeBased = timeBased_; @@ -44,7 +44,7 @@ contract TimeManagerV5 { } /** - * @notice Returns the current timestamp in seconds + * @dev Returns the current timestamp in seconds * @return The current timestamp */ function _getBlockTimestamp() private view returns (uint256) { @@ -52,7 +52,7 @@ contract TimeManagerV5 { } /** - * @notice Returns the current block number + * @dev Returns the current block number * @return The current block number */ function _getBlockNumber() private view returns (uint256) { diff --git a/contracts/TimeManagerV8.sol b/contracts/TimeManagerV8.sol index eb84c9f..eaee7c4 100644 --- a/contracts/TimeManagerV8.sol +++ b/contracts/TimeManagerV8.sol @@ -17,13 +17,14 @@ abstract contract TimeManagerV8 { error InvalidBlocksPerYear(); /// @notice Thrown when time based but blocks per year is provided - error InvalidTimeBased(); + error InvalidTimeBasedConfiguration(); /** - * @param timeBased_ A boolean indicating whether the contract is based on time or block. + * @param timeBased_ A boolean indicating whether the contract is based on time or block * If timeBased is true than blocksPerYear_ param is ignored as blocksOrSecondsPerYear is set to SECONDS_PER_YEAR * @param blocksPerYear_ The number of blocks per year - * @custom:error InvalidBlocksPerYear is thrown if blocksPerYear entered is zero + * @custom:error InvalidBlocksPerYear is thrown if blocksPerYear entered is zero and timeBased is false + * @custom:error InvalidTimeBasedConfiguration is thrown if blocksPerYear entered is non zero and timeBased is true * @custom:oz-upgrades-unsafe-allow constructor */ constructor(bool timeBased_, uint256 blocksPerYear_) { @@ -32,7 +33,7 @@ abstract contract TimeManagerV8 { } if (timeBased_ && blocksPerYear_ != 0) { - revert InvalidTimeBased(); + revert InvalidTimeBasedConfiguration(); } isTimeBased = timeBased_; @@ -49,7 +50,7 @@ abstract contract TimeManagerV8 { } /** - * @notice Returns the current timestamp in seconds + * @dev Returns the current timestamp in seconds * @return The current timestamp */ function _getBlockTimestamp() private view returns (uint256) { @@ -57,7 +58,7 @@ abstract contract TimeManagerV8 { } /** - * @notice Returns the current block number + * @dev Returns the current block number * @return The current block number */ function _getBlockNumber() private view returns (uint256) { From e3f9df8370b72d3f0631b2547911a96d4451385c Mon Sep 17 00:00:00 2001 From: GitGuru7 <128375421+GitGuru7@users.noreply.github.com> Date: Thu, 21 Dec 2023 11:08:44 +0530 Subject: [PATCH 5/5] refactor: add gap --- contracts/TimeManagerV5.sol | 7 +++++++ contracts/TimeManagerV8.sol | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/contracts/TimeManagerV5.sol b/contracts/TimeManagerV5.sol index 033cc88..59ba7c3 100644 --- a/contracts/TimeManagerV5.sol +++ b/contracts/TimeManagerV5.sol @@ -11,6 +11,13 @@ contract TimeManagerV5 { /// @dev Sets true when block timestamp is used bool public isTimeBased; + /** + * @dev This empty reserved space is put in place to allow future versions to add new + * variables without shifting down storage in the inheritance chain + * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps + */ + uint256[47] private __gap; + /** * @dev Retrieves the current slot * @return Current slot diff --git a/contracts/TimeManagerV8.sol b/contracts/TimeManagerV8.sol index eaee7c4..e36f339 100644 --- a/contracts/TimeManagerV8.sol +++ b/contracts/TimeManagerV8.sol @@ -10,6 +10,13 @@ abstract contract TimeManagerV8 { /// @custom:oz-upgrades-unsafe-allow state-variable-immutable bool public immutable isTimeBased; + /** + * @dev This empty reserved space is put in place to allow future versions to add new + * variables without shifting down storage in the inheritance chain + * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps + */ + uint256[48] private __gap; + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable function() view returns (uint256) private immutable _getCurrentSlot;