Merge branch 'sonic_sfc' into mike/constants_manager_errors

# Conflicts: # contracts/ownership/Ownable.sol # contracts/sfc/ConstantsManager.sol # test/NodeDriver.ts # test/SFC.ts
Fantom-foundation · Oct 22, 2024 · 8304ec0 · 8304ec0
2 parents d8fcd6c + 78101a8
commit 8304ec0
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 15 deletions.
diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol
@@ -15,14 +15,14 @@ contract Ownable is Initializable {
     address private _owner;
 
     /**
-     * @dev The caller is not the owner.
+     * @dev The caller account is not authorized to perform an operation.
      */
-    error NotOwner();
+    error OwnableUnauthorizedAccount(address account);
 
     /**
-     * @dev Given zero address.
+     * @dev The owner is not a valid owner account. (eg. `address(0)`)
      */
-    error ZeroAddress();
+    error OwnableInvalidOwner(address owner);
 
     event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
 
@@ -46,7 +46,7 @@ contract Ownable is Initializable {
      */
     modifier onlyOwner() {
         if (!isOwner()) {
-            revert NotOwner();
+            revert OwnableUnauthorizedAccount(msg.sender);
         }
         _;
     }
@@ -83,7 +83,7 @@ contract Ownable is Initializable {
      */
     function _transferOwnership(address newOwner) internal {
         if (newOwner == address(0)) {
-            revert ZeroAddress();
+            revert OwnableInvalidOwner(address(0));
         }
         emit OwnershipTransferred(_owner, newOwner);
         _owner = newOwner;

diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol
@@ -75,6 +75,10 @@ contract SFC is SFCBase, Version {
         return getEpochSnapshot[epoch].offlineBlocks[validatorID];
     }
 
+    function getEpochEndBlock(uint256 epoch) public view returns (uint256) {
+        return getEpochSnapshot[epoch].endBlock;
+    }
+
     function rewardsStash(address delegator, uint256 validatorID) public view returns (uint256) {
         Rewards memory stash = _rewardsStash[delegator][validatorID];
         return stash.lockupBaseReward + stash.lockupExtraReward + stash.unlockedReward;
@@ -361,6 +365,7 @@ contract SFC is SFCBase, Version {
 
         currentSealedEpoch = currentEpoch();
         snapshot.endTime = _now();
+        snapshot.endBlock = block.number;
         snapshot.baseRewardPerSecond = c.baseRewardPerSecond();
         snapshot.totalSupply = totalSupply;
     }

diff --git a/contracts/sfc/SFCI.sol b/contracts/sfc/SFCI.sol
@@ -44,6 +44,7 @@ interface SFCI {
         view
         returns (
             uint256 endTime,
+            uint256 endBlock,
             uint256 epochFee,
             uint256 totalBaseRewardWeight,
             uint256 totalTxRewardWeight,
@@ -137,6 +138,8 @@ interface SFCI {
 
     function getEpochOfflineBlocks(uint256 epoch, uint256 validatorID) external view returns (uint256);
 
+    function getEpochEndBlock(uint256 epoch) external view returns (uint256);
+
     function rewardsStash(address delegator, uint256 validatorID) external view returns (uint256);
 
     function getLockedStake(address delegator, uint256 toValidatorID) external view returns (uint256);

diff --git a/contracts/sfc/SFCState.sol b/contracts/sfc/SFCState.sol
@@ -71,6 +71,7 @@ contract SFCState is Initializable, Ownable {
         mapping(uint256 => uint256) offlineBlocks;
         uint256[] validatorIDs;
         uint256 endTime;
+        uint256 endBlock;
         uint256 epochFee;
         uint256 totalBaseRewardWeight;
         uint256 totalTxRewardWeight;

diff --git a/contracts/test/UnitTestSFC.sol b/contracts/test/UnitTestSFC.sol
@@ -120,6 +120,7 @@ interface SFCUnitTestI {
         view
         returns (
             uint256 endTime,
+            uint256 endBlock,
             uint256 epochFee,
             uint256 totalBaseRewardWeight,
             uint256 totalTxRewardWeight,
@@ -215,6 +216,8 @@ interface SFCUnitTestI {
 
     function getEpochOfflineBlocks(uint256 epoch, uint256 validatorID) external view returns (uint256);
 
+    function getEpochEndBlock(uint256 epoch) external view returns (uint256);
+
     function rewardsStash(address delegator, uint256 validatorID) external view returns (uint256);
 
     function getLockedStake(address delegator, uint256 toValidatorID) external view returns (uint256);

diff --git a/test/NodeDriver.ts b/test/NodeDriver.ts
@@ -40,7 +40,7 @@ describe('NodeDriver', () => {
       const account = ethers.Wallet.createRandom();
       await expect(this.nodeDriverAuth.connect(this.nonOwner).migrateTo(account)).to.be.revertedWithCustomError(
         this.nodeDriverAuth,
-        'NotOwner',
+        'OwnableUnauthorizedAccount',
       );
     });
   });
@@ -55,7 +55,7 @@ describe('NodeDriver', () => {
       const address = ethers.Wallet.createRandom();
       await expect(
         this.nodeDriverAuth.connect(this.nonOwner).copyCode(this.sfc, address),
-      ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'NotOwner');
+      ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'OwnableUnauthorizedAccount');
     });
   });
 
@@ -69,7 +69,7 @@ describe('NodeDriver', () => {
     it('Should revert when not owner', async function () {
       await expect(this.nodeDriverAuth.connect(this.nonOwner).updateNetworkVersion(1)).to.be.revertedWithCustomError(
         this.nodeDriverAuth,
-        'NotOwner',
+        'OwnableUnauthorizedAccount',
       );
     });
   });
@@ -82,7 +82,7 @@ describe('NodeDriver', () => {
     it('Should revert when not owner', async function () {
       await expect(this.nodeDriverAuth.connect(this.nonOwner).advanceEpochs(10)).to.be.revertedWithCustomError(
         this.nodeDriverAuth,
-        'NotOwner',
+        'OwnableUnauthorizedAccount',
       );
     });
   });

diff --git a/test/SFC.ts b/test/SFC.ts
@@ -292,15 +292,14 @@ describe('SFC', () => {
     it('Should revert when transferring ownership if not owner', async function () {
       await expect(this.sfc.connect(this.user).transferOwnership(ethers.ZeroAddress)).to.be.revertedWithCustomError(
         this.nodeDriverAuth,
-        'NotOwner',
+        'OwnableUnauthorizedAccount',
       );
     });
 
     it('Should revert when transferring ownership to zero address', async function () {
-      await expect(this.sfc.transferOwnership(ethers.ZeroAddress)).to.be.revertedWithCustomError(
-        this.nodeDriverAuth,
-        'ZeroAddress',
-      );
+      await expect(this.sfc.transferOwnership(ethers.ZeroAddress))
+        .to.be.revertedWithCustomError(this.nodeDriverAuth, 'OwnableInvalidOwner')
+        .withArgs(ethers.ZeroAddress);
     });
   });
 
@@ -415,6 +414,16 @@ describe('SFC', () => {
         expect(await this.sfc.currentEpoch.call()).to.equal(6);
         expect(await this.sfc.currentSealedEpoch()).to.equal(5);
       });
+
+      it('Should succeed and return endBlock', async function () {
+        const epochNumber = await this.sfc.currentEpoch();
+        await this.sfc.enableNonNodeCalls();
+        await this.sfc.sealEpoch([100, 101, 102], [100, 101, 102], [100, 101, 102], [100, 101, 102], 0);
+        const lastBlock = await ethers.provider.getBlockNumber();
+        // endBlock is on second position
+        expect((await this.sfc.getEpochSnapshot(epochNumber))[1]).to.equal(lastBlock);
+        expect(await this.sfc.getEpochEndBlock(epochNumber)).to.equal(lastBlock);
+      });
     });
   });