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.

Sign up for GitHub

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

Jump to bottom

fix: resolve review comments #454

Merged
merged 4 commits into from
Jan 3, 2024
Merged

fix: resolve review comments #454

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
64c57dc
fix: resolve review comments
pythonberg1997 Dec 28, 2023
348af6b
add check to `_jailValidator`
pythonberg1997 Jan 3, 2024
fca3c57
fix lint issue
pythonberg1997 Jan 3, 2024
7f022fd
fix review comments
pythonberg1997 Jan 3, 2024
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
26 changes: 13 additions & 13 deletions abi/bscvalidatorset.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,19 +1062,6 @@
],
"stateMutability": "view"
},
{
"type": "function",
"name": "jailValidator",
"inputs": [
{
"name": "consensusAddress",
"type": "address",
"internalType": "address"
}
],
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "maintainSlashScale",
Expand Down Expand Up @@ -1224,6 +1211,19 @@
],
"stateMutability": "view"
},
{
"type": "function",
"name": "removeTmpMigratedValidator",
"inputs": [
{
"name": "validator",
"type": "address",
"internalType": "address"
}
],
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "systemRewardRatio",
Expand Down
19 changes: 0 additions & 19 deletions abi/slashindicator.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,25 +865,6 @@
],
"anonymous": false
},
{
"type": "event",
"name": "failedMaliciousVoteSlash",
"inputs": [
{
"name": "voteAddrSlice",
"type": "bytes32",
"indexed": true,
"internalType": "bytes32"
},
{
"name": "failReason",
"type": "bytes",
"indexed": false,
"internalType": "bytes"
}
],
"anonymous": false
},
{
"type": "event",
"name": "indicatorCleaned",
Expand Down
2 changes: 1 addition & 1 deletion abi/stakehub.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@
"internalType": "address"
},
{
"name": "commissionRate",
"name": "newCommissionRate",
"type": "uint64",
"indexed": false,
"internalType": "uint64"
Expand Down
37 changes: 0 additions & 37 deletions abi/systemreward.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,19 +159,6 @@
],
"stateMutability": "view"
},
{
"type": "function",
"name": "MAX_REWARDS_FOR_FINALITY",
"inputs": [],
"outputs": [
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "view"
},
{
"type": "function",
"name": "RELAYERHUB_CONTRACT_ADDR",
Expand Down Expand Up @@ -417,30 +404,6 @@
],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "claimRewardsforFinality",
"inputs": [
{
"name": "to",
"type": "address",
"internalType": "address payable"
},
{
"name": "amount",
"type": "uint256",
"internalType": "uint256"
}
],
"outputs": [
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "isOperator",
Expand Down
24 changes: 0 additions & 24 deletions abi/tokenhub.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,30 +750,6 @@
],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "claimRewardsforFinality",
"inputs": [
{
"name": "",
"type": "address",
"internalType": "address payable"
},
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"outputs": [
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "getBep2SymbolByContractAddr",
Expand Down
13 changes: 9 additions & 4 deletions contracts/BC_fusion/StakeHub.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import "./System.sol";
import "./lib/Utils.sol";
import "./interface/IBSCValidatorSet.sol";
import "./interface/ICrossChain.sol";
import "./interface/IGovToken.sol";
import "./interface/IStakeCredit.sol";

Expand Down Expand Up @@ -176,7 +177,7 @@ contract StakeHub is System, Initializable {
bytes voteAddress
);
event ConsensusAddressEdited(address indexed operatorAddress, address indexed newConsensusAddress);
event CommissionRateEdited(address indexed operatorAddress, uint64 commissionRate);
event CommissionRateEdited(address indexed operatorAddress, uint64 newCommissionRate);
event DescriptionEdited(address indexed operatorAddress);
event VoteAddressEdited(address indexed operatorAddress, bytes newVoteAddress);
event Delegated(address indexed operatorAddress, address indexed delegator, uint256 shares, uint256 bnbAmount);
Expand Down Expand Up @@ -994,8 +995,8 @@ contract StakeHub is System, Initializable {
}
if (IStakeCredit(valInfo.creditContract).getPooledBNB(operatorAddress) < minSelfDelegationBNB) {
_jailValidator(valInfo, downtimeJailTime);
// need to inform BSCValidatorSet contract to remove the validator
IBSCValidatorSet(VALIDATOR_CONTRACT_ADDR).jailValidator(valInfo.consensusAddress);
IBSCValidatorSet(VALIDATOR_CONTRACT_ADDR).removeTmpMigratedValidator(valInfo.consensusAddress);
IBSCValidatorSet(VALIDATOR_CONTRACT_ADDR).felony(valInfo.consensusAddress);
}
}

Expand All @@ -1015,7 +1016,11 @@ contract StakeHub is System, Initializable {
function _jailValidator(Validator storage valInfo, uint256 jailUntil) internal {
// keep the last eligible validator
bool isLast = (numOfJailed >= _validatorSet.length() - 1);
if (isLast) {
// 0x08 is the staking channel id. If this channel is closed, then BC-fusion is finished and we should keep the last eligible validator here
if (
!ICrossChain(CROSS_CHAIN_CONTRACT_ADDR).registeredContractChannelMap(VALIDATOR_CONTRACT_ADDR, 0x08)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should put isLast before the channel check, because in most cases isLast will be false, then channel check will be unnecessary, it can save gas.

&& isLast
) {
emit ValidatorEmptyJailed(valInfo.operatorAddress);
return;
}
Expand Down
3 changes: 2 additions & 1 deletion contracts/BC_fusion/interface/IBSCValidatorSet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
pragma solidity 0.8.17;

interface IBSCValidatorSet {
function jailValidator(address consensusAddress) external;
function felony(address consensusAddress) external;
function removeTmpMigratedValidator(address consensusAddress) external;
}
6 changes: 6 additions & 0 deletions contracts/BC_fusion/interface/ICrossChain.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.17;

interface ICrossChain {
function registeredContractChannelMap(address, uint8) external view returns (bool);
}
68 changes: 27 additions & 41 deletions contracts/BSCValidatorSet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
receive() external payable {}

/*********************** Cross Chain App Implement **************************/
// TODO: add `onlyCrossChainContract`
function handleSynPackage(uint8, bytes calldata msgBytes) onlyInit initValidatorExtraSet external override returns(bytes memory responsePayload) {
function handleSynPackage(uint8, bytes calldata msgBytes) onlyInit onlyCrossChainContract initValidatorExtraSet external override returns(bytes memory responsePayload) {
(IbcValidatorSetPackage memory validatorSetPackage, bool ok) = decodeValidatorSetSynPackage(msgBytes);
if (!ok) {
return CmnPkg.encodeCommonAckPackage(ERROR_FAIL_DECODE);
Expand All @@ -202,13 +201,18 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
emit failReasonWithStr("length of jail validators must be one");
resCode = ERROR_LEN_OF_VAL_MISMATCH;
} else {
uint256 index = currentValidatorSetMap[validatorSetPackage.validatorSet[0].consensusAddress];
if (index==0 || currentValidatorSet[index-1].jailed) {
emit validatorEmptyJailed(validatorSetPackage.validatorSet[0].consensusAddress);
resCode = CODE_OK;
address validator = validatorSetPackage.validatorSet[0].consensusAddress;
uint256 index = currentValidatorSetMap[validator];
if (index == 0 || currentValidatorSet[index-1].jailed) {
emit validatorEmptyJailed(validator);
} else {
resCode = _jailValidator(index);
// felony will failed if the validator is the only one in the validator set
bool success = _felony(validator, index-1);
if (!success) {
emit validatorEmptyJailed(validator);
}
}
resCode = CODE_OK;
}
} else {
resCode = ERROR_UNKNOWN_PACKAGE_TYPE;
Expand Down Expand Up @@ -357,36 +361,6 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
}
}

function jailValidator(address consensusAddress) external onlyStakeHub {
for (uint256 i; i < _tmpMigratedValidatorSet.length; ++i) {
if (_tmpMigratedValidatorSet[i].consensusAddress == consensusAddress) {
_tmpMigratedValidatorSet[i].jailed = true;
break;
}
}

uint256 index = currentValidatorSetMap[consensusAddress];
if (index==0 || currentValidatorSet[index-1].jailed) {
emit validatorEmptyJailed(consensusAddress);
} else {
_jailValidator(index);
}
}

function _jailValidator(uint256 index) internal returns (uint32) {
uint n = currentValidatorSet.length;
bool shouldKeep = (numOfJailed >= n-1);
// will not jail if it is the last valid validator
if (shouldKeep) {
emit validatorEmptyJailed(currentValidatorSet[index-1].consensusAddress);
return CODE_OK;
}
++numOfJailed;
currentValidatorSet[index-1].jailed = true;
emit validatorJailed(currentValidatorSet[index-1].consensusAddress);
return CODE_OK;
}

function updateValidatorSet(Validator[] memory validatorSet, bytes[] memory voteAddrs) internal returns (uint32) {
{
// do verify.
Expand Down Expand Up @@ -701,7 +675,7 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
return;
}

totalValue = ISystemReward(SYSTEM_REWARD_ADDR).claimRewardsforFinality(payable(address(this)), totalValue);
totalValue = ISystemReward(SYSTEM_REWARD_ADDR).claimRewards(payable(address(this)), totalValue);
if (totalValue == 0) {
return;
}
Expand Down Expand Up @@ -758,7 +732,9 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
}
}

function felony(address validator)external onlySlash initValidatorExtraSet override{
function felony(address validator) external initValidatorExtraSet override {
require(msg.sender == SLASH_CONTRACT_ADDR || msg.sender == STAKE_HUB_ADDR, "only slash or stakeHub contract");

uint256 index = currentValidatorSetMap[validator];
if (index <= 0) {
return;
Expand All @@ -772,6 +748,15 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
}
}

function removeTmpMigratedValidator(address validator) external onlyStakeHub {
for (uint256 i; i < _tmpMigratedValidatorSet.length; ++i) {
if (_tmpMigratedValidatorSet[i].consensusAddress == validator) {
_tmpMigratedValidatorSet[i].jailed = true;
break;
}
}
}

/*********************** For Temporary Maintenance **************************/
function getCurrentValidatorIndex(address _validator) public view returns (uint256) {
uint256 index = currentValidatorSetMap[_validator];
Expand Down Expand Up @@ -1064,6 +1049,8 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
// should not happen, but still protect
return index;
}

// averageDistribute*rest may less than income, but it is ok, the dust income will go to system reward eventually.
uint256 averageDistribute = income / rest;
if (averageDistribute != 0) {
for (uint i; i<index; ++i) {
Expand All @@ -1074,12 +1061,11 @@ contract BSCValidatorSet is IBSCValidatorSet, System, IParamSubscriber, IApplica
currentValidatorSet[i].incoming = currentValidatorSet[i].incoming + averageDistribute;
}
}
// averageDistribute*rest may less than income, but it is ok, the dust income will go to system reward eventually.

return index;
}

function _felony(address validator, uint256 index) private returns (bool){
function _felony(address validator, uint256 index) private returns (bool) {
uint256 income = currentValidatorSet[index].incoming;
uint256 rest = currentValidatorSet.length - 1;
if (getValidators().length <= 1) {
Expand Down
3 changes: 1 addition & 2 deletions contracts/GovHub.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ contract GovHub is System, IApplication{
address target;
}

//TODO: add onlyCrossChainContract
function handleSynPackage(uint8, bytes calldata msgBytes) external override returns(bytes memory responsePayload) {
function handleSynPackage(uint8, bytes calldata msgBytes) external onlyCrossChainContract override returns(bytes memory responsePayload) {
(ParamChangePackage memory proposal, bool success) = decodeSynPackage(msgBytes);
if (!success) {
return CmnPkg.encodeCommonAckPackage(ERROR_FAIL_DECODE);
Expand Down
Loading
Loading