chore: fix audit issues

abi/bscgovernor.abi

@@ -1652,6 +1652,25 @@
     ],
     "anonymous": false
   },
+  {
+    "type": "event",
+    "name": "ProtectorChanged",
+    "inputs": [
+      {
+        "name": "oldProtector",
+        "type": "address",
+        "indexed": true,
+        "internalType": "address"
+      },
+      {
+        "name": "newProtector",
+        "type": "address",
+        "indexed": true,
+        "internalType": "address"
+      }
+    ],
+    "anonymous": false
+  },
   {
     "type": "event",
     "name": "QuorumNumeratorUpdated",

abi/stakehub.abi

@@ -1266,6 +1266,25 @@
     "inputs": [],
     "anonymous": false
   },
+  {
+    "type": "event",
+    "name": "ProtectorChanged",
+    "inputs": [
+      {
+        "name": "oldProtector",
+        "type": "address",
+        "indexed": true,
+        "internalType": "address"
+      },
+      {
+        "name": "newProtector",
+        "type": "address",
+        "indexed": true,
+        "internalType": "address"
+      }
+    ],
+    "anonymous": false
+  },
   {
     "type": "event",
     "name": "Redelegated",

contracts/BC_fusion/StakeHub.sol

@@ -351,7 +351,7 @@ contract StakeHub is System, Initializable, Protectable {
         ) revert InvalidCommission();
         if (!_checkMoniker(description.moniker)) revert InvalidMoniker();
         // proof-of-possession verify
-        if (!_checkVoteAddress(voteAddress, blsProof)) revert InvalidVoteAddress();
+        if (!_checkVoteAddress(operatorAddress, voteAddress, blsProof)) revert InvalidVoteAddress();
 
         // deploy stake credit proxy contract
         address creditContract = _deployStakeCredit(operatorAddress, description.moniker);
@@ -458,12 +458,12 @@ contract StakeHub is System, Initializable, Protectable {
         bytes calldata blsProof
     ) external whenNotPaused notInBlackList validatorExist(msg.sender) {
         // proof-of-possession verify
-        if (!_checkVoteAddress(newVoteAddress, blsProof)) revert InvalidVoteAddress();
+        address operatorAddress = msg.sender;
+        if (!_checkVoteAddress(operatorAddress, newVoteAddress, blsProof)) revert InvalidVoteAddress();
         if (voteToOperator[newVoteAddress] != address(0) || _legacyVoteAddress[newVoteAddress]) {
             revert DuplicateVoteAddress();
         }
 
-        address operatorAddress = msg.sender;
         Validator storage valInfo = _validators[operatorAddress];
         if (valInfo.updateTime + BREATHE_BLOCK_INTERVAL > block.timestamp) revert UpdateTooFrequently();
 
@@ -1065,13 +1065,17 @@ contract StakeHub is System, Initializable, Protectable {
         return true;
     }
 
-    function _checkVoteAddress(bytes calldata voteAddress, bytes calldata blsProof) internal view returns (bool) {
+    function _checkVoteAddress(
+        address operatorAddress,
+        bytes calldata voteAddress,
+        bytes calldata blsProof
+    ) internal view returns (bool) {
         if (voteAddress.length != BLS_PUBKEY_LENGTH || blsProof.length != BLS_SIG_LENGTH) {
             return false;
         }
 
         // get msg hash
-        bytes32 msgHash = keccak256(abi.encodePacked(voteAddress, block.chainid));
+        bytes32 msgHash = keccak256(abi.encodePacked(operatorAddress, voteAddress, block.chainid));
         bytes memory msgBz = new bytes(32);
         assembly {
             mstore(add(msgBz, 32), msgHash)

contracts/BC_fusion/extension/Protectable.sol

@@ -21,6 +21,7 @@ abstract contract Protectable is Initializable {
     /*----------------- events -----------------*/
     event Paused();
     event Resumed();
+    event ProtectorChanged(address indexed oldProtector, address indexed newProtector);
     event BlackListed(address indexed target);
     event UnBlackListed(address indexed target);
 
@@ -96,6 +97,7 @@ abstract contract Protectable is Initializable {
 
     /*----------------- internal functions -----------------*/
     function _setProtector(address protector) internal {
+        emit ProtectorChanged(_protector, protector);
         _protector = protector;
     }
 

contracts/BSCValidatorSet.sol

@@ -847,7 +847,7 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
     } else if (Memory.compareStrings(key, "burnRatio")) {
       require(value.length == 32, "length of burnRatio mismatch");
       uint256 newBurnRatio = BytesToTypes.bytesToUint256(32, value);
-      require(newBurnRatio + systemRewardRatio <= BLOCK_FEES_RATIO_SCALE, "the burnRatio plus systemRewardRatio must be no greater than 10000");
+      require(newBurnRatio.add(systemRewardRatio) <= BLOCK_FEES_RATIO_SCALE, "the burnRatio plus systemRewardRatio must be no greater than 10000");
       burnRatio = newBurnRatio;
     } else if (Memory.compareStrings(key, "maxNumOfMaintaining")) {
       require(value.length == 32, "length of maxNumOfMaintaining mismatch");
@@ -884,7 +884,7 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
     } else if (Memory.compareStrings(key, "systemRewardRatio")) {
       require(value.length == 32, "length of systemRewardRatio mismatch");
       uint256 newSystemRewardRatio = BytesToTypes.bytesToUint256(32, value);
-      require(newSystemRewardRatio + burnRatio <= BLOCK_FEES_RATIO_SCALE, "the systemRewardRatio plus burnRatio must be no greater than 10000");
+      require(newSystemRewardRatio.add(burnRatio) <= BLOCK_FEES_RATIO_SCALE, "the systemRewardRatio plus burnRatio must be no greater than 10000");
       systemRewardRatio = newSystemRewardRatio;
     } else {
       require(false, "unknown param");
@@ -930,11 +930,11 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
         validatorExtraSet[i].isMaintaining = false;
         validatorExtraSet[i].enterMaintenanceHeight = 0;
       } else {
+        currentValidatorSet[i].votingPower = newValidatorSet[i].votingPower;
         // update the vote address if it is different
         if (!BytesLib.equal(newVoteAddrs[i], validatorExtraSet[i].voteAddress)) {
           validatorExtraSet[i].voteAddress = newVoteAddrs[i];
         }
-        currentValidatorSet[i].jailed = newValidatorSet[i].jailed;
       }
     }
 
@@ -968,7 +968,7 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
    * Vote address is not considered
    */
   function isSameValidator(Validator memory v1, Validator memory v2) private pure returns(bool) {
-    return v1.consensusAddress == v2.consensusAddress && v1.feeAddress == v2.feeAddress && v1.BBCFeeAddress == v2.BBCFeeAddress && v1.votingPower == v2.votingPower;
+    return v1.consensusAddress == v2.consensusAddress && v1.feeAddress == v2.feeAddress && v1.BBCFeeAddress == v2.BBCFeeAddress;
   }
 
   function getVoteAddresses(address[] memory validators) internal view returns(bytes[] memory) {
@@ -1074,11 +1074,11 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
     uint256 averageDistribute = income / rest;
     if (averageDistribute != 0) {
       for (uint i; i<index; ++i) {
-        currentValidatorSet[i].incoming = currentValidatorSet[i].incoming + averageDistribute;
+        currentValidatorSet[i].incoming = currentValidatorSet[i].incoming.add(averageDistribute);
       }
       uint n = currentValidatorSet.length;
       for (uint i=index+1; i<n; ++i) {
-        currentValidatorSet[i].incoming = currentValidatorSet[i].incoming + averageDistribute;
+        currentValidatorSet[i].incoming = currentValidatorSet[i].incoming.add(averageDistribute);
       }
     }
 
@@ -1111,7 +1111,7 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
     if (averageDistribute != 0) {
       uint n = currentValidatorSet.length;
       for (uint i; i<n; ++i) {
-        currentValidatorSet[i].incoming = currentValidatorSet[i].incoming + averageDistribute;
+        currentValidatorSet[i].incoming = currentValidatorSet[i].incoming.add(averageDistribute);
       }
     }
     return true;
@@ -1122,6 +1122,13 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
     address validator;
     bool isFelony;
 
+    // count the number of felony validators before forcing maintaining validators exit
+    for (uint i; i<_validatorSet.length; ++i) {
+      if (_validatorSet[i].jailed) {
+        ++numOfFelony;
+      }
+    }
+
     // 1. validators exit maintenance
     uint256 i;
     // caution: it must calculate workingValidatorCount before _exitMaintenance loop
@@ -1147,8 +1154,10 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
       // record the jailed validator in validatorSet
       for (uint k; k<_validatorSet.length; ++k) {
         if (_validatorSet[k].consensusAddress == validator) {
-          _validatorSet[k].jailed = true;
-          ++numOfFelony;
+          if (!_validatorSet[k].jailed) {
+            _validatorSet[k].jailed = true;
+            ++numOfFelony;
+          }
           break;
         }
       }

contracts/SlashIndicator.sol

@@ -41,7 +41,7 @@ contract SlashIndicator is ISlashIndicator,System,IParamSubscriber, IApplication
   uint256 public felonySlashRewardRatio;
   bool public enableMaliciousVoteSlash;
 
-  uint256 public constant INIT_FELONY_SLASH_SCOPE = 86400;  // 3 days
+  uint256 public constant INIT_FELONY_SLASH_SCOPE = 28800;  // 1 days(block number)
 
   uint256 public felonySlashScope;
 

contracts/Staking.sol

@@ -651,6 +651,7 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
     if (isAutoUndelegate) {
       delegated[recipient] = delegated[recipient].sub(amount);
       delegatedOfValidator[recipient][validator] = delegatedOfValidator[recipient][validator].sub(amount);
+      emit undelegateSuccess(recipient, validator, amount);
     }
 
     emit undelegatedReceived(recipient, validator, amount);

test/TokenRecoverPortal.t.sol

@@ -32,7 +32,7 @@ contract TokenRecoverPortalTest is Deployer {
     address mockUser = address(0x2e9247B67ae885a8dcfBf77Eb6d0e93A32bea24C);
     bytes mockTokenOwner = hex"b713200f29effb427fb76a185b4ac73ea09a534b";
     bytes32 testTokenSymbol = hex"424e420000000000000000000000000000000000000000000000000000000000";
-    
+
     address protector = address(0x5C7c4b3ee76D1eD8a4341Ab07D87a2a88d81454A);
     address approvalAddress = address(0xb26859a7321AB7B2025E5E6a425D697e2eacbFB1);
     bytes merkleRoot = hex"59bb94f7047904a8fdaec42e4785295167f7fd63742b309afeb84bd71f8e6554";
@@ -64,8 +64,10 @@ contract TokenRecoverPortalTest is Deployer {
         bytes32 tokenSymbol = testTokenSymbol;
         uint256 amount = 14188000000;
         bytes memory ownerPubKey = hex"036d5d41cd7da2e96d39bcbd0390bfed461a86382f7a2923436ff16c65cabc7719";
-        bytes memory ownerSignature = hex"5f5391ba7f2b002b4746025f7e803a43e57a397ea66f3939d05302eb7851bbbc0773cda87aae0fbb1e2a29367b606209ed47dc5cba6d1a83f6b79cb70e56efdb";
-        bytes memory approvalSignature = hex"52a0a5ca80beb068d82413cac31c1df0540dc6a61eddec9f31b94419e60b6c586e5342552f4c8034a00c876d640abea8c5ba9c4d72145d0e562fedd09fe1e00a01";
+        bytes memory ownerSignature =
+            hex"5f5391ba7f2b002b4746025f7e803a43e57a397ea66f3939d05302eb7851bbbc0773cda87aae0fbb1e2a29367b606209ed47dc5cba6d1a83f6b79cb70e56efdb";
+        bytes memory approvalSignature =
+            hex"52a0a5ca80beb068d82413cac31c1df0540dc6a61eddec9f31b94419e60b6c586e5342552f4c8034a00c876d640abea8c5ba9c4d72145d0e562fedd09fe1e00a01";
         bytes32[] memory merkleProof = new bytes32[](17);
         merkleProof[0] = hex"03719d7863e4aba727d7030e7a1916b9be2245d447eb71fc683d3ac0ded5eecd";
         merkleProof[1] = hex"7f9aa9d8246251cbab3cc642416dec81d074d39a85be6ca8326a05ac422e74ab";
@@ -86,8 +88,7 @@ contract TokenRecoverPortalTest is Deployer {
         merkleProof[16] = hex"a2d456e52facaa953bfbc79a5a6ed7647dda59872b9b35c20183887eeb4640eb";
 
         // recover the token
-        tokenRecoverPortal.recover(
-            tokenSymbol, amount, ownerPubKey, ownerSignature, approvalSignature, merkleProof);
+        tokenRecoverPortal.recover(tokenSymbol, amount, ownerPubKey, ownerSignature, approvalSignature, merkleProof);
 
         // check if the token is recovered
         bytes32 node = keccak256(abi.encodePacked(mockTokenOwner, tokenSymbol, amount));

test/utils/interface/IBSCGovernor.sol

@@ -46,6 +46,7 @@ interface BSCGovernor {
     event ProposalExtended(uint256 indexed proposalId, uint64 extendedDeadline);
     event ProposalQueued(uint256 proposalId, uint256 eta);
     event ProposalThresholdSet(uint256 oldProposalThreshold, uint256 newProposalThreshold);
+    event ProtectorChanged(address indexed oldProtector, address indexed newProtector);
     event QuorumNumeratorUpdated(uint256 oldQuorumNumerator, uint256 newQuorumNumerator);
     event Resumed();
     event TimelockChange(address oldTimelock, address newTimelock);

test/utils/interface/IStakeHub.sol

@@ -65,6 +65,7 @@ interface StakeHub {
     event MigrateSuccess(address indexed operatorAddress, address indexed delegator, uint256 shares, uint256 bnbAmount);
     event ParamChange(string key, bytes value);
     event Paused();
+    event ProtectorChanged(address indexed oldProtector, address indexed newProtector);
     event Redelegated(
         address indexed srcValidator,
         address indexed dstValidator,

test/utils/interface/ITokenRecoverPortal.sol

@@ -1,26 +1,57 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.10;
 
-// TokenRecoverPortal anyone to recover a token if they exist in a Beacon Chain.
 interface TokenRecoverPortal {
-    // Returns the merkle root of the merkle tree containing account balances available to recover.
-    function merkleRoot() external view returns (bytes32);
-    // Returns the address of the contract that is allowed to confirm the recover.
+    error AlreadyPaused();
+    error AlreadyRecovered();
+    error ApprovalAddressNotInitialized();
+    error InBlackList();
+    error InvalidApprovalSignature();
+    error InvalidOwnerPubKeyLength();
+    error InvalidOwnerSignatureLength();
+    error InvalidProof();
+    error InvalidValue(string key, bytes value);
+    error MerkleRootAlreadyInitiated();
+    error MerkleRootNotInitialized();
+    error NotPaused();
+    error OnlyCoinbase();
+    error OnlyProtector();
+    error OnlySystemContract(address systemContract);
+    error OnlyZeroGasPrice();
+    error TokenRecoverPortalPaused();
+    error UnknownParam(string key, bytes value);
+
+    event BlackListed(address indexed target);
+    event Initialized(uint8 version);
+    event ParamChange(string key, bytes value);
+    event Paused();
+    event ProtectorChanged(address indexed oldProtector, address indexed newProtector);
+    event Resumed();
+    event TokenRecoverRequested(bytes32 tokenSymbol, address account, uint256 amount);
+    event UnBlackListed(address indexed target);
+
+    function BC_FUSION_CHANNELID() external view returns (uint8);
+    function SOURCE_CHAIN_ID() external view returns (string memory);
+    function STAKING_CHANNELID() external view returns (uint8);
+    function addToBlackList(address account) external;
     function approvalAddress() external view returns (address);
-    // Returns the address of the contract that is allowed to pause the recover.
-    function assetProtector() external view returns (address);
-    // Returns true if the index has been marked recovered.
-    function isRecovered(bytes32 index) external view returns (bool);
-    // recover the given amount of the token to the given address. Reverts if the inputs are invalid.
+    function blackList(address) external view returns (bool);
+    function cancelTokenRecover(bytes32 tokenSymbol, address attacker) external;
+    function initialize() external;
+    function isPaused() external view returns (bool);
+    function isRecovered(bytes32 node) external view returns (bool);
+    function merkleRoot() external view returns (bytes32);
+    function merkleRootAlreadyInit() external view returns (bool);
+    function pause() external;
     function recover(
         bytes32 tokenSymbol,
         uint256 amount,
-        bytes calldata ownerPubKey,
-        bytes calldata ownerSignature,
-        bytes calldata approvalSignature,
-        bytes32[] calldata merkleProof
+        bytes memory ownerPubKey,
+        bytes memory ownerSignature,
+        bytes memory approvalSignature,
+        bytes32[] memory merkleProof
     ) external;
-    // Cancel the user token recover request by the assetProtector.
-    function cancelTokenRecover(bytes32 tokenSymbol, address recipient) external;
-    function initialize() external;
+    function removeFromBlackList(address account) external;
+    function resume() external;
+    function updateParam(string memory key, bytes memory value) external;
 }