chore: fix issues from audit report (#465)

* chore: fix issues from audit report * fix unit tests
bnb-chain · Jan 15, 2024 · 8e5e491 · 8e5e491
1 parent c069e9e
commit 8e5e491
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 11 deletions.
diff --git a/abi/stakecredit.abi b/abi/stakecredit.abi
@@ -842,6 +842,11 @@
     "name": "ZeroAmount",
     "inputs": []
   },
+  {
+    "type": "error",
+    "name": "ZeroShares",
+    "inputs": []
+  },
   {
     "type": "error",
     "name": "ZeroTotalPooledBNB",

diff --git a/abi/stakehub.abi b/abi/stakehub.abi
@@ -1353,6 +1353,25 @@
     ],
     "anonymous": false
   },
+  {
+    "type": "event",
+    "name": "StakeCreditInitialized",
+    "inputs": [
+      {
+        "name": "operatorAddress",
+        "type": "address",
+        "indexed": true,
+        "internalType": "address"
+      },
+      {
+        "name": "creditContract",
+        "type": "address",
+        "indexed": true,
+        "internalType": "address"
+      }
+    ],
+    "anonymous": false
+  },
   {
     "type": "event",
     "name": "Undelegated",

diff --git a/contracts/BC_fusion/StakeCredit.sol b/contracts/BC_fusion/StakeCredit.sol
@@ -31,6 +31,8 @@ contract StakeCredit is System, Initializable, ReentrancyGuardUpgradeable, ERC20
     error TransferFailed();
     // @notice signature: 0x1f2a2005
     error ZeroAmount();
+    // @notice signature: 0x9811e0c7
+    error ZeroShares();
     // @notice signature: 0xf4d678b8
     error InsufficientBalance();
     // @notice signature: 0xad418937
@@ -98,6 +100,7 @@ contract StakeCredit is System, Initializable, ReentrancyGuardUpgradeable, ERC20
     function delegate(address delegator) external payable onlyStakeHub returns (uint256 shares) {
         if (msg.value == 0) revert ZeroAmount();
         shares = _mintAndSync(delegator, msg.value);
+        if (shares == 0) revert ZeroShares();
     }
 
     /**
@@ -106,7 +109,7 @@ contract StakeCredit is System, Initializable, ReentrancyGuardUpgradeable, ERC20
      * @return bnbAmount the amount of BNB to be unlocked
      */
     function undelegate(address delegator, uint256 shares) external onlyStakeHub returns (uint256 bnbAmount) {
-        if (shares == 0) revert ZeroAmount();
+        if (shares == 0) revert ZeroShares();
         if (shares > balanceOf(delegator)) revert InsufficientBalance();
 
         // add to the queue
@@ -128,7 +131,7 @@ contract StakeCredit is System, Initializable, ReentrancyGuardUpgradeable, ERC20
      * @return bnbAmount the amount of BNB unlocked
      */
     function unbond(address delegator, uint256 shares) external onlyStakeHub returns (uint256 bnbAmount) {
-        if (shares == 0) revert ZeroAmount();
+        if (shares == 0) revert ZeroShares();
         if (shares > balanceOf(delegator)) revert InsufficientBalance();
 
         bnbAmount = _burnAndSync(delegator, shares);
@@ -314,6 +317,7 @@ contract StakeCredit is System, Initializable, ReentrancyGuardUpgradeable, ERC20
     }
 
     function _mintAndSync(address account, uint256 bnbAmount) internal returns (uint256 shares) {
+        // shares here could be zero
         shares = getSharesByPooledBNB(bnbAmount);
         _mint(account, shares);
         totalPooledBNB += bnbAmount;

diff --git a/contracts/BC_fusion/StakeHub.sol b/contracts/BC_fusion/StakeHub.sol
@@ -89,6 +89,7 @@ contract StakeHub is System, Initializable {
     error VoteAddressExpired();
     // @notice signature: 0xc2aee074
     error ConsensusAddressExpired();
+    // @notice signature: 0x0d7b78d4
     error InvalidSynPackage();
 
     /*----------------- storage -----------------*/
@@ -152,8 +153,7 @@ contract StakeHub is System, Initializable {
         MIGRATE_SUCCESS,
         CLAIM_FUND_FAILED,
         VALIDATOR_NOT_EXISTED,
-        VALIDATOR_JAILED,
-        BALANCE_NOT_ENOUGH
+        VALIDATOR_JAILED
     }
 
     struct Validator {
@@ -195,6 +195,7 @@ contract StakeHub is System, Initializable {
         address indexed creditContract,
         bytes voteAddress
     );
+    event StakeCreditInitialized(address indexed operatorAddress, address indexed creditContract);
     event ConsensusAddressEdited(address indexed operatorAddress, address indexed newConsensusAddress);
     event CommissionRateEdited(address indexed operatorAddress, uint64 newCommissionRate);
     event DescriptionEdited(address indexed operatorAddress);
@@ -491,7 +492,7 @@ contract StakeHub is System, Initializable {
     /**
      * @param operatorAddress the operator address of the validator to be unjailed
      */
-    function unjail(address operatorAddress) external whenNotPaused validatorExist(operatorAddress) {
+    function unjail(address operatorAddress) external whenNotPaused notInBlackList validatorExist(operatorAddress) {
         Validator storage valInfo = _validators[operatorAddress];
         if (!valInfo.jailed) revert ValidatorNotJailed();
 
@@ -1042,10 +1043,6 @@ contract StakeHub is System, Initializable {
             return StakeMigrationRespCode.VALIDATOR_JAILED;
         }
 
-        if (address(this).balance < migrationPkg.amount) {
-            return StakeMigrationRespCode.BALANCE_NOT_ENOUGH;
-        }
-
         uint256 shares =
             IStakeCredit(valInfo.creditContract).delegate{ value: migrationPkg.amount }(migrationPkg.delegator);
         emit Delegated(migrationPkg.operatorAddress, migrationPkg.delegator, shares, migrationPkg.amount);
@@ -1115,6 +1112,7 @@ contract StakeHub is System, Initializable {
     function _deployStakeCredit(address operatorAddress, string memory moniker) internal returns (address) {
         address creditProxy = address(new TransparentUpgradeableProxy(STAKE_CREDIT_ADDR, DEAD_ADDRESS, ""));
         IStakeCredit(creditProxy).initialize{ value: msg.value }(operatorAddress, moniker);
+        emit StakeCreditInitialized(operatorAddress, creditProxy);
 
         return creditProxy;
     }

diff --git a/test/StakeHub.t.sol b/test/StakeHub.t.sol
@@ -32,8 +32,7 @@ contract StakeHubTest is Deployer {
         MIGRATE_SUCCESS,
         CLAIM_FUND_FAILED,
         VALIDATOR_NOT_EXISTED,
-        VALIDATOR_JAILED,
-        BALANCE_NOT_ENOUGH
+        VALIDATOR_JAILED
     }
 
     receive() external payable { }
@@ -301,6 +300,7 @@ contract StakeHubTest is Deployer {
         (address consensusAddress,,,,,) = stakeHub.getValidatorBasicInfo(validator);
         vm.expectEmit(true, true, false, true, address(stakeHub));
         emit RewardDistributed(validator, reward);
+        vm.deal(VALIDATOR_CONTRACT_ADDR, VALIDATOR_CONTRACT_ADDR.balance + reward);
         vm.prank(VALIDATOR_CONTRACT_ADDR);
         stakeHub.distributeReward{ value: reward }(consensusAddress);
 
@@ -348,6 +348,7 @@ contract StakeHubTest is Deployer {
         stakeHub.delegate{ value: 100 ether }(validator, false);
 
         (address consensusAddress,,,,,) = stakeHub.getValidatorBasicInfo(validator);
+        vm.deal(VALIDATOR_CONTRACT_ADDR, VALIDATOR_CONTRACT_ADDR.balance + reward);
         vm.prank(VALIDATOR_CONTRACT_ADDR);
         stakeHub.distributeReward{ value: reward }(consensusAddress);
 
@@ -400,6 +401,7 @@ contract StakeHubTest is Deployer {
         stakeHub.delegate{ value: 100 ether }(validator, false);
 
         (address consensusAddress,,,,,) = stakeHub.getValidatorBasicInfo(validator);
+        vm.deal(VALIDATOR_CONTRACT_ADDR, VALIDATOR_CONTRACT_ADDR.balance + reward);
         vm.prank(VALIDATOR_CONTRACT_ADDR);
         stakeHub.distributeReward{ value: reward }(consensusAddress);
 
@@ -431,6 +433,7 @@ contract StakeHubTest is Deployer {
         stakeHub.delegate{ value: 100 ether }(validator, false);
 
         (address consensusAddress,,, bytes memory voteAddr,,) = stakeHub.getValidatorBasicInfo(validator);
+        vm.deal(VALIDATOR_CONTRACT_ADDR, VALIDATOR_CONTRACT_ADDR.balance + reward);
         vm.prank(VALIDATOR_CONTRACT_ADDR);
         stakeHub.distributeReward{ value: reward }(consensusAddress);
 

diff --git a/test/utils/interface/IStakeCredit.sol b/test/utils/interface/IStakeCredit.sol
@@ -24,6 +24,7 @@ interface StakeCredit {
     error UnknownParam(string key, bytes value);
     error WrongInitContext();
     error ZeroAmount();
+    error ZeroShares();
     error ZeroTotalPooledBNB();
     error ZeroTotalShares();
 

diff --git a/test/utils/interface/IStakeHub.sol b/test/utils/interface/IStakeHub.sol
@@ -74,6 +74,7 @@ interface StakeHub {
     event Resumed();
     event RewardDistributeFailed(address indexed operatorAddress, bytes failReason);
     event RewardDistributed(address indexed operatorAddress, uint256 reward);
+    event StakeCreditInitialized(address indexed operatorAddress, address indexed creditContract);
     event Undelegated(address indexed operatorAddress, address indexed delegator, uint256 shares, uint256 bnbAmount);
     event ValidatorCreated(
         address indexed consensusAddress,