Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
4 changes: 4 additions & 0 deletions contracts/interfaces/IScdMcdMigration.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ interface IScdMcdMigration {
interface IDaiAdapter {
function dai() public returns (address);
}

interface ISaiTop {
function caged() public returns (uint256);
}
20 changes: 20 additions & 0 deletions contracts/mocks/ForeignBridgeErcToNativeMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
pragma solidity 0.4.24;

import "../upgradeable_contracts/erc20_to_native/ForeignBridgeErcToNative.sol";

contract ForeignBridgeErcToNativeMock is ForeignBridgeErcToNative {
function migrationContract() internal pure returns (IScdMcdMigration) {
// Address generated in unit test
return IScdMcdMigration(0x44Bf5539DAAc4259f7F11A24280255ED2Fa3F7BF);
}

function halfDuplexErc20token() public pure returns (ERC20) {
// Address generated in unit test
return ERC20(0x872D709De609c391741c7595F0053F6060e59e0D);
}

function saiTopContract() internal pure returns (ISaiTop) {
// Address generated in unit test
return ISaiTop(0x96bc48adACdB60E6536E55a6727919a397F14540);
}
}
9 changes: 9 additions & 0 deletions contracts/mocks/SaiTopMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
pragma solidity 0.4.24;

contract SaiTopMock {
uint256 public caged;

function setCaged(uint256 _value) public {
caged = _value;
}
}
19 changes: 19 additions & 0 deletions contracts/upgradeable_contracts/BaseBridgeValidators.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,23 @@ contract BaseBridgeValidators is InitializableBridge, Ownable {
function setNextValidator(address _prevValidator, address _validator) internal {
addressStorage[keccak256(abi.encodePacked("validatorsList", _prevValidator))] = _validator;
}

function isValidatorDuty(address _validator) external view returns (bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we increase a version in getBridgeValidatorsInterfacesVersion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should. Updated version to 2.3.0 be3cd33

uint256 counter = 0;
address next = getNextValidator(F_ADDR);
require(next != address(0));

while (next != F_ADDR) {
if (next == _validator) {
return (block.number % validatorCount() == counter);
}

next = getNextValidator(next);
counter++;

require(next != address(0));
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import "../../interfaces/IScdMcdMigration.sol";
contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge, OtherSideBridgeStorage {
event TokensSwapped(address indexed from, address indexed to, uint256 value);

bytes32 internal constant MIN_HDTOKEN_BALANCE = 0x48649cf195feb695632309f41e61252b09f537943654bde13eb7bb1bca06964e; // keccak256(abi.encodePacked("minHDTokenBalance"))
bytes4 internal constant SWAP_TOKENS = 0x73d00224; // swapTokens()

function initialize(
address _validatorContract,
address _erc20token,
Expand Down Expand Up @@ -61,6 +64,11 @@ contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge, OtherSideB

function claimTokens(address _token, address _to) public {
require(_token != address(erc20token()));
if (_token == address(halfDuplexErc20token())) {
// SCD is not claimable if the bridge accepts deposits of this token
// solhint-disable-next-line not-rely-on-time
require(!isTokenSwapAllowed(now));
}
super.claimTokens(_token, _to);
}

Expand All @@ -71,7 +79,13 @@ contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge, OtherSideB
) internal returns (bool) {
setTotalExecutedPerDay(getCurrentDay(), totalExecutedPerDay(getCurrentDay()).add(_amount));
uint256 amount = _amount.div(10**decimalShift());
return erc20token().transfer(_recipient, amount);
bool res = erc20token().transfer(_recipient, amount);

if (tokenBalance(halfDuplexErc20token()) > 0) {
address(this).call(abi.encodeWithSelector(SWAP_TOKENS));
}

return res;
}

function onFailedMessage(address, uint256, bytes32) internal {
Expand All @@ -83,23 +97,113 @@ contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge, OtherSideB
super._relayTokens(_sender, _receiver, _amount);
}

function migrateToMCD(address _migrationContract) external onlyOwner {
function migrateToMCD() external onlyOwner {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As part of the security audit we have received a finding against this functionality and here is a recommendation: if the migration address is hardcoded or configurable, the migrateToMCD function does not need the onlyOwner modifier and anyone could invoke migrateToMCD, and thus trigger the migration.

So, let's remove the modifier onlyOwner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the method 251f949

bytes32 storageAddress = 0x3378953eb16363e06fd9ea9701d36ed7285d206d9de7df55b778462d74596a89; // keccak256(abi.encodePacked("migrationToMcdCompleted"))

require(!boolStorage[storageAddress]);
require(AddressUtils.isContract(_migrationContract));

uint256 curBalance = erc20token().balanceOf(address(this));
require(erc20token().approve(_migrationContract, curBalance));
//It is important to note that this action will cause appearing of `Transfer`
//event as part of the tokens minting
IScdMcdMigration(_migrationContract).swapSaiToDai(curBalance);
address saiContract = erc20token();
address mcdContract = IDaiAdapter(IScdMcdMigration(_migrationContract).daiJoin()).dai();

address mcdContract = IDaiAdapter(migrationContract().daiJoin()).dai();
setErc20token(mcdContract);
require(erc20token().balanceOf(address(this)) == curBalance);

emit TokensSwapped(saiContract, erc20token(), curBalance);
uintStorage[MIN_HDTOKEN_BALANCE] = 10 ether;

swapTokens();

boolStorage[storageAddress] = true;
}

function saiTopContract() internal pure returns (ISaiTop) {
return ISaiTop(0x9b0ccf7C8994E19F39b2B4CF708e0A7DF65fA8a3);
}

function isTokenSwapAllowed(uint256 _ts) public view returns (bool) {
uint256 esTs = saiTopContract().caged();
if (esTs > 0 && _ts > esTs) {
return false;
}
return true;
}

function halfDuplexErc20token() public pure returns (ERC20) {
return ERC20(0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359);
}

function setMinHDTokenBalance(uint256 _minBalance) external onlyOwner {
uintStorage[MIN_HDTOKEN_BALANCE] = _minBalance;
}

function minHDTokenBalance() public view returns (uint256) {
return uintStorage[MIN_HDTOKEN_BALANCE];
}

function isHDTokenBalanceAboveMinBalance() public view returns (bool) {
if (tokenBalance(halfDuplexErc20token()) > minHDTokenBalance()) {
return true;
}
return false;
}

function tokenBalance(ERC20 _token) internal view returns (uint256) {
return _token.balanceOf(address(this));
}

function migrationContract() internal pure returns (IScdMcdMigration) {
return IScdMcdMigration(0xc73e0383F3Aff3215E6f04B0331D58CeCf0Ab849);
}

function swapTokens() public {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we increase the version returned by getBridgeInterfacesVersion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it makes sense. Updated to 2.5.0 c2c2c4c

// solhint-disable-next-line not-rely-on-time
require(isTokenSwapAllowed(now));

IScdMcdMigration mcdMigrationContract = migrationContract();
ERC20 hdToken = halfDuplexErc20token();
ERC20 fdToken = erc20token();

uint256 curHDTokenBalance = tokenBalance(hdToken);
require(curHDTokenBalance > 0);

uint256 curFDTokenBalance = tokenBalance(fdToken);

require(hdToken.approve(mcdMigrationContract, curHDTokenBalance));
mcdMigrationContract.swapSaiToDai(curHDTokenBalance);

require(tokenBalance(fdToken).sub(curFDTokenBalance) == curHDTokenBalance);

emit TokensSwapped(hdToken, fdToken, curHDTokenBalance);
}

function relayTokens(address _from, address _receiver, uint256 _amount, address _token) external {
require(_from == msg.sender || _from == _receiver);
_relayTokens(_from, _receiver, _amount, _token);
}

function relayTokens(address _receiver, uint256 _amount, address _token) external {
_relayTokens(msg.sender, _receiver, _amount, _token);
}

function _relayTokens(address _sender, address _receiver, uint256 _amount, address _token) internal {
require(_receiver != bridgeContractOnOtherSide());
require(_receiver != address(0));
require(_receiver != address(this));
require(_amount > 0);
require(withinLimit(_amount));

ERC20 tokenToOperate = ERC20(_token);
ERC20 hdToken = halfDuplexErc20token();
ERC20 fdToken = erc20token();

if (tokenToOperate == ERC20(0x0)) {
tokenToOperate = fdToken;
}

require(tokenToOperate == fdToken || tokenToOperate == hdToken);

setTotalSpentPerDay(getCurrentDay(), totalSpentPerDay(getCurrentDay()).add(_amount));

tokenToOperate.transferFrom(_sender, address(this), _amount);
emit UserRequestForAffirmation(_receiver, _amount);

if (tokenToOperate == hdToken) {
swapTokens();
}
}
}
1 change: 1 addition & 0 deletions scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ start_ganache() {
--account="0x2bdd21761a483f71054e14f5b827213567971c676928d9a1808cbfa4b7501207,1000000000000000000000000"
--account="0x2bdd21761a483f71054e14f5b827213567971c676928d9a1808cbfa4b7501208,1000000000000000000000000"
--account="0x2bdd21761a483f71054e14f5b827213567971c676928d9a1808cbfa4b7501209,1000000000000000000000000"
--account="0x19fba401d77e4113b15095e9aa7117bcd25adcfac7f6111f8298894eef443600,1000000000000000000000000"
)

if [ "$SOLIDITY_COVERAGE" != true ]; then
Expand Down
Loading