Skip to content

Commit

Permalink
fix: some audit comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@pythonberg1997
pythonberg1997 committed Sep 22, 2022
1 parent 0ff1f6d commit eb85ade
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 20 deletions.
28 changes: 21 additions & 7 deletions contracts/Staking.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
uint256 public constant INIT_RELAYER_FEE = 16 * 1e15;
uint256 public constant INIT_BSC_RELAYER_FEE = 1 * 1e16;
uint256 public constant INIT_MIN_DELEGATION = 100 * 1e18;
uint256 public constant INIT_TRANSFER_GAS = 2300;

uint256 public relayerFee;
uint256 public bSCRelayerFee;
uint256 public minDelegation;
uint256 public transferGas;

mapping(address => uint256) delegated; // delegator => totalAmount
mapping(address => mapping(address => uint256)) delegatedOfValidator; // delegator => validator => amount
Expand Down Expand Up @@ -78,6 +80,7 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
relayerFee = INIT_RELAYER_FEE;
bSCRelayerFee = INIT_BSC_RELAYER_FEE;
minDelegation = INIT_MIN_DELEGATION;
transferGas = INIT_TRANSFER_GAS;
alreadyInit = true;
}
_;
Expand Down Expand Up @@ -192,7 +195,8 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
function delegate(address validator, uint256 amount) override external payable noReentrant tenDecimalPrecision(amount) initParams {
require(amount >= minDelegation, "invalid delegate amount");
require(msg.value >= amount.add(relayerFee), "not enough msg value");
require(payable(msg.sender).send(0), "invalid delegator"); // the msg sender must be payable
(bool success,) = msg.sender.call{gas: transferGas}("");
require(success, "invalid delegator"); // the msg sender must be payable

uint256 convertedAmount = amount.div(TEN_DECIMALS); // native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18
uint256 _relayerFee = (msg.value).sub(amount);
Expand All @@ -217,11 +221,14 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
function undelegate(address validator, uint256 amount) override external payable noReentrant tenDecimalPrecision(amount) initParams {
require(msg.value >= relayerFee, "not enough relay fee");
if (amount < minDelegation) {
require(amount > bSCRelayerFee, "not enough funds");
require(amount == delegatedOfValidator[msg.sender][validator], "invalid amount");
require(amount > bSCRelayerFee, "not enough funds");
}
require(delegatedOfValidator[msg.sender][validator] >= amount, "invalid amount");
require(block.timestamp >= pendingUndelegateTime[msg.sender][validator], "pending undelegation exist");
uint256 remainBalance = delegatedOfValidator[msg.sender][validator].sub(amount, "not enough funds");
if (remainBalance != 0) {
require(remainBalance > bSCRelayerFee, "insufficient balance after undelegate");
}

uint256 convertedAmount = amount.div(TEN_DECIMALS); // native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18
uint256 _relayerFee = msg.value;
Expand All @@ -248,9 +255,13 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
function redelegate(address validatorSrc, address validatorDst, uint256 amount) override external noReentrant payable tenDecimalPrecision(amount) initParams {
require(validatorSrc != validatorDst, "invalid redelegation");
require(msg.value >= relayerFee, "not enough relay fee");
require(amount >= minDelegation && delegatedOfValidator[msg.sender][validatorSrc] >= amount, "invalid amount");
require(amount >= minDelegation, "invalid amount");
require(block.timestamp >= pendingRedelegateTime[msg.sender][validatorSrc][validatorDst] &&
block.timestamp >= pendingRedelegateTime[msg.sender][validatorDst][validatorSrc], "pending redelegation exist");
uint256 remainBalance = delegatedOfValidator[msg.sender][validatorSrc].sub(amount, "not enough funds");
if (remainBalance != 0) {
require(remainBalance > bSCRelayerFee, "insufficient balance after redelegate");
}

uint256 convertedAmount = amount.div(TEN_DECIMALS);// native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18
uint256 _relayerFee = msg.value;
Expand All @@ -268,7 +279,7 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {

pendingRedelegateTime[msg.sender][validatorDst][validatorSrc] = block.timestamp.add(LOCK_TIME);
pendingRedelegateTime[msg.sender][validatorSrc][validatorDst] = block.timestamp.add(LOCK_TIME);

ICrossChain(CROSS_CHAIN_CONTRACT_ADDR).sendSynPackage(CROSS_STAKE_CHANNELID, msgBytes, oracleRelayerFee.div(TEN_DECIMALS));
payable(TOKEN_HUB_ADDR).transfer(oracleRelayerFee);
payable(SYSTEM_REWARD_ADDR).transfer(bSCRelayerFee);
Expand All @@ -281,7 +292,8 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
require(amount > 0, "no pending reward");

distributedReward[msg.sender] = 0;
payable(msg.sender).transfer(amount);
(bool success,) = msg.sender.call{gas: transferGas, value: amount}("");
require(success, "transfer failed");
emit rewardClaimed(msg.sender, amount);
}

Expand All @@ -290,7 +302,8 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
require(amount > 0, "no undelegated funds");

undelegated[msg.sender] = 0;
payable(msg.sender).transfer(amount);
(bool success,) = msg.sender.call{gas: transferGas, value: amount}("");
require(success, "transfer failed");
emit undelegatedClaimed(msg.sender, amount);
}

Expand Down Expand Up @@ -376,6 +389,7 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
} else if (Memory.compareStrings(key, "bSCRelayerFee")) {
require(value.length == 32, "length of bSCRelayerFee mismatch");
uint256 newBSCRelayerFee = BytesToTypes.bytesToUint256(32, value);
require(newBSCRelayerFee != 0, "the BSCRelayerFee must not be zero");
require(newBSCRelayerFee < relayerFee, "the BSCRelayerFee must be less than relayerFee");
require(newBSCRelayerFee%TEN_DECIMALS==0, "the BSCRelayerFee mod ten decimals must be zero");
bSCRelayerFee = newBSCRelayerFee;
Expand Down
26 changes: 20 additions & 6 deletions contracts/Staking.template
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
uint256 public constant INIT_RELAYER_FEE = 16 * 1e15;
uint256 public constant INIT_BSC_RELAYER_FEE = 1 * 1e16;
uint256 public constant INIT_MIN_DELEGATION = 100 * 1e18;
uint256 public constant INIT_TRANSFER_GAS = 2300;

uint256 public relayerFee;
uint256 public bSCRelayerFee;
uint256 public minDelegation;
uint256 public transferGas;

mapping(address => uint256) delegated; // delegator => totalAmount
mapping(address => mapping(address => uint256)) delegatedOfValidator; // delegator => validator => amount
Expand Down Expand Up @@ -78,6 +80,7 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
relayerFee = INIT_RELAYER_FEE;
bSCRelayerFee = INIT_BSC_RELAYER_FEE;
minDelegation = INIT_MIN_DELEGATION;
transferGas = INIT_TRANSFER_GAS;
alreadyInit = true;
}
_;
Expand Down Expand Up @@ -192,7 +195,8 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
function delegate(address validator, uint256 amount) override external payable noReentrant tenDecimalPrecision(amount) initParams {
require(amount >= minDelegation, "invalid delegate amount");
require(msg.value >= amount.add(relayerFee), "not enough msg value");
require(payable(msg.sender).send(0), "invalid delegator"); // the msg sender must be payable
(bool success,) = msg.sender.call{gas: transferGas}("");
require(success, "invalid delegator"); // the msg sender must be payable

uint256 convertedAmount = amount.div(TEN_DECIMALS); // native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18
uint256 _relayerFee = (msg.value).sub(amount);
Expand All @@ -217,11 +221,14 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
function undelegate(address validator, uint256 amount) override external payable noReentrant tenDecimalPrecision(amount) initParams {
require(msg.value >= relayerFee, "not enough relay fee");
if (amount < minDelegation) {
require(amount > bSCRelayerFee, "not enough funds");
require(amount == delegatedOfValidator[msg.sender][validator], "invalid amount");
require(amount > bSCRelayerFee, "not enough funds");
}
require(delegatedOfValidator[msg.sender][validator] >= amount, "invalid amount");
require(block.timestamp >= pendingUndelegateTime[msg.sender][validator], "pending undelegation exist");
uint256 remainBalance = delegatedOfValidator[msg.sender][validator].sub(amount, "not enough funds");
if (remainBalance != 0) {
require(remainBalance > bSCRelayerFee, "insufficient balance after undelegate");
}

uint256 convertedAmount = amount.div(TEN_DECIMALS); // native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18
uint256 _relayerFee = msg.value;
Expand All @@ -248,9 +255,13 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
function redelegate(address validatorSrc, address validatorDst, uint256 amount) override external noReentrant payable tenDecimalPrecision(amount) initParams {
require(validatorSrc != validatorDst, "invalid redelegation");
require(msg.value >= relayerFee, "not enough relay fee");
require(amount >= minDelegation && delegatedOfValidator[msg.sender][validatorSrc] >= amount, "invalid amount");
require(amount >= minDelegation, "invalid amount");
require(block.timestamp >= pendingRedelegateTime[msg.sender][validatorSrc][validatorDst] &&
block.timestamp >= pendingRedelegateTime[msg.sender][validatorDst][validatorSrc], "pending redelegation exist");
uint256 remainBalance = delegatedOfValidator[msg.sender][validatorSrc].sub(amount, "not enough funds");
if (remainBalance != 0) {
require(remainBalance > bSCRelayerFee, "insufficient balance after redelegate");
}

uint256 convertedAmount = amount.div(TEN_DECIMALS);// native bnb decimals is 8 on BBC, while the native bnb decimals on BSC is 18
uint256 _relayerFee = msg.value;
Expand Down Expand Up @@ -281,7 +292,8 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
require(amount > 0, "no pending reward");

distributedReward[msg.sender] = 0;
payable(msg.sender).transfer(amount);
(bool success,) = msg.sender.call{gas: transferGas, value: amount}("");
require(success, "transfer failed");
emit rewardClaimed(msg.sender, amount);
}

Expand All @@ -290,7 +302,8 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
require(amount > 0, "no undelegated funds");

undelegated[msg.sender] = 0;
payable(msg.sender).transfer(amount);
(bool success,) = msg.sender.call{gas: transferGas, value: amount}("");
require(success, "transfer failed");
emit undelegatedClaimed(msg.sender, amount);
}

Expand Down Expand Up @@ -376,6 +389,7 @@ contract Staking is IStaking, System, IParamSubscriber, IApplication {
} else if (Memory.compareStrings(key, "bSCRelayerFee")) {
require(value.length == 32, "length of bSCRelayerFee mismatch");
uint256 newBSCRelayerFee = BytesToTypes.bytesToUint256(32, value);
require(newBSCRelayerFee != 0, "the BSCRelayerFee must not be zero");
require(newBSCRelayerFee < relayerFee, "the BSCRelayerFee must be less than relayerFee");
require(newBSCRelayerFee%TEN_DECIMALS==0, "the BSCRelayerFee mod ten decimals must be zero");
bSCRelayerFee = newBSCRelayerFee;
Expand Down
4 changes: 3 additions & 1 deletion contracts/TokenHub.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,9 @@ contract TokenHub is ITokenHub, System, IParamSubscriber, IApplication, ISystemR

function withdrawStakingBNB(uint256 amount) external override returns(bool) {
require(msg.sender == STAKING_CONTRACT_ADDR, "only staking system contract can call this function");
payable(STAKING_CONTRACT_ADDR).transfer(amount);
if (amount != 0) {
payable(STAKING_CONTRACT_ADDR).transfer(amount);
}
return true;
}
}
4 changes: 3 additions & 1 deletion contracts/TokenHub.template
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,9 @@ contract TokenHub is ITokenHub, System, IParamSubscriber, IApplication, ISystemR

function withdrawStakingBNB(uint256 amount) external override returns(bool) {
require(msg.sender == STAKING_CONTRACT_ADDR, "only staking system contract can call this function");
payable(STAKING_CONTRACT_ADDR).transfer(amount);
if (amount != 0) {
payable(STAKING_CONTRACT_ADDR).transfer(amount);
}
return true;
}
}
9 changes: 4 additions & 5 deletions contracts/mock/MockTokenHub.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,10 @@ contract MockTokenHub is ITokenHub {

function withdrawStakingBNB(uint256 amount) external override returns(bool) {
address STAKING_CONTRACT_ADDR = address(0x0000000000000000000000000000000000002001);
require(msg.sender == STAKING_CONTRACT_ADDR,
"only staking system contract can call this function");
payable(STAKING_CONTRACT_ADDR).transfer(amount);
require(msg.sender == STAKING_CONTRACT_ADDR, "only staking system contract can call this function");
if (amount != 0) {
payable(STAKING_CONTRACT_ADDR).transfer(amount);
}
return true;
}
}


0 comments on commit eb85ade

Please sign in to comment.