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

Separate fee into home fee and foreign fee #151

Merged
merged 7 commits into from
Feb 5, 2019
Merged

Separate fee into home fee and foreign fee #151

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e1787e6
Refactor fee into homeFee and foreignFee
patitonar Feb 1, 2019
34a4960
Update deploy script to use home and foreign fee on rewardableInitialize
patitonar Feb 1, 2019
b209667
Update gas consumption
patitonar Feb 4, 2019
b54cd53
Update fee parameter docs
patitonar Feb 4, 2019
78d6c3b
Refactor RewardableBridge
patitonar Feb 4, 2019
5a69e97
Fix rewardableInitialize methods
patitonar Feb 4, 2019
f9e3193
Update gas consumption docs
patitonar Feb 4, 2019
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
29 changes: 20 additions & 9 deletions contracts/upgradeable_contracts/BaseFeeManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,44 @@ pragma solidity 0.4.24;
import "../upgradeability/EternalStorage.sol";
import "../libraries/SafeMath.sol";
import "../IRewardableValidators.sol";
import "./FeeTypes.sol";


contract BaseFeeManager is EternalStorage {
contract BaseFeeManager is EternalStorage, FeeTypes {
using SafeMath for uint256;

bytes32 public constant REWARD_FOR_TRANSFERRING_FROM_HOME = keccak256(abi.encodePacked("reward-transferring-from-home"));

bytes32 public constant REWARD_FOR_TRANSFERRING_FROM_FOREIGN = keccak256(abi.encodePacked("reward-transferring-from-foreign"));

event FeeUpdated(uint256 fee);
event HomeFeeUpdated(uint256 fee);
event ForeignFeeUpdated(uint256 fee);

function calculateFee(uint256 _value, bool _recover) external view returns(uint256) {
uint256 fee = getFee();
function calculateFee(uint256 _value, bool _recover, bytes32 _feeType) external view returns(uint256) {
uint256 fee = _feeType == HOME_FEE ? getHomeFee() : getForeignFee();
uint256 eth = 1 ether;
if (!_recover) {
return _value.mul(fee).div(eth);
}
return _value.mul(fee).div(eth.sub(fee));
}

function setFee(uint256 _fee) external {
uintStorage[keccak256(abi.encodePacked("fee"))] = _fee;
emit FeeUpdated(_fee);
function setHomeFee(uint256 _fee) external {
uintStorage[keccak256(abi.encodePacked("homeFee"))] = _fee;
emit HomeFeeUpdated(_fee);
}

function getFee() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("fee"))];
function getHomeFee() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("homeFee"))];
}

function setForeignFee(uint256 _fee) external {
uintStorage[keccak256(abi.encodePacked("foreignFee"))] = _fee;
emit ForeignFeeUpdated(_fee);
}

function getForeignFee() public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked("foreignFee"))];
}

function distributeFeeFromAffirmation(uint256 _fee) external {
Expand Down
7 changes: 7 additions & 0 deletions contracts/upgradeable_contracts/FeeTypes.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
pragma solidity 0.4.24;


contract FeeTypes {
bytes32 internal constant HOME_FEE = keccak256(abi.encodePacked("home-fee"));
bytes32 internal constant FOREIGN_FEE = keccak256(abi.encodePacked("foreign-fee"));
}
34 changes: 25 additions & 9 deletions contracts/upgradeable_contracts/RewardableBridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
pragma solidity 0.4.24;

import "./Ownable.sol";
import "./FeeTypes.sol";


contract RewardableBridge is Ownable {
contract RewardableBridge is Ownable, FeeTypes {

function setFee(uint256 _fee) external onlyOwner {
_setFee(feeManagerContract(), _fee);
function setHomeFee(uint256 _fee) 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.

Does it make sense to move these methods to the corresponding implementations of the bridges:

  • erc20_to_native/RewardableHomeBridgeErcToNative.sol:
    • setHomeFee
    • setForeignFee
    • getHomeFee
    • getForeignFee
  • native_to_erc20/RewardableHomeBridgeNativeToErc.sol
    • setForeignFee
    • getForeignFee
  • native_to_erc20/RewardableForeignBridgeNativeToErc.sol
    • setHomeFee
    • getHomeFee

By doing this we would:

  1. slightly reduce the code deployed.
  2. get rid of mess up especially in rhe case of native-to-erc mode.

What do you think?

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 agree on the changes, this would help on making code easier to follow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 78d6c3b

_setFee(feeManagerContract(), _fee, HOME_FEE);
}

function getFee() public view returns(uint256) {
function setForeignFee(uint256 _fee) external onlyOwner {
_setFee(feeManagerContract(), _fee, FOREIGN_FEE);
}

function getHomeFee() public view returns(uint256) {
return _getFee(HOME_FEE);
}

function getForeignFee() public view returns(uint256) {
return _getFee(FOREIGN_FEE);
}

function _getFee(bytes32 _feeType) public view returns(uint256) {
uint256 fee;
bytes memory callData = abi.encodeWithSignature("getFee()");
address feeManager = feeManagerContract();
string memory method = _feeType == HOME_FEE ? "getHomeFee()" : "getForeignFee()";
bytes memory callData = abi.encodeWithSignature(method);

assembly {
let result := callcode(gas, feeManager, 0x0, add(callData, 0x20), mload(callData), 0, 32)
fee := mload(0)
Expand Down Expand Up @@ -46,8 +61,9 @@ contract RewardableBridge is Ownable {
addressStorage[keccak256(abi.encodePacked("feeManagerContract"))] = _feeManager;
}

function _setFee(address _feeManager, uint256 _fee) internal {
require(_feeManager.delegatecall(abi.encodeWithSignature("setFee(uint256)", _fee)));
function _setFee(address _feeManager, uint256 _fee, bytes32 _feeType) internal {
string memory method = _feeType == HOME_FEE ? "setHomeFee(uint256)" : "setForeignFee(uint256)";
require(_feeManager.delegatecall(abi.encodeWithSignature(method, _fee)));
}

function isContract(address _addr) internal view returns (bool)
Expand All @@ -57,9 +73,9 @@ contract RewardableBridge is Ownable {
return length > 0;
}

function calculateFee(uint256 _value, bool _recover, address _impl) internal view returns(uint256) {
function calculateFee(uint256 _value, bool _recover, address _impl, bytes32 _feeType) internal view returns(uint256) {
uint256 fee;
bytes memory callData = abi.encodeWithSignature("calculateFee(uint256,bool)", _value, _recover);
bytes memory callData = abi.encodeWithSignature("calculateFee(uint256,bool,bytes32)", _value, _recover, _feeType);
assembly {
let result := callcode(gas, _impl, 0x0, add(callData, 0x20), mload(callData), 0, 32)
fee := mload(0)
Expand Down
12 changes: 7 additions & 5 deletions contracts/upgradeable_contracts/erc20_to_native/HomeBridgeErcToNative.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ contract HomeBridgeErcToNative is EternalStorage, BasicBridge, BasicHomeBridge,
uint256 valueToTransfer = msg.value;
address feeManager = feeManagerContract();
if (feeManager != address(0)) {
uint256 fee = calculateFee(valueToTransfer, false, feeManager);
uint256 fee = calculateFee(valueToTransfer, false, feeManager, HOME_FEE);
valueToTransfer = valueToTransfer.sub(fee);
}
setTotalBurntCoins(totalBurntCoins().add(valueToTransfer));
Expand Down Expand Up @@ -76,7 +76,8 @@ contract HomeBridgeErcToNative is EternalStorage, BasicBridge, BasicHomeBridge,
uint256 _foreignMaxPerTx,
address _owner,
address _feeManager,
uint256 _fee
uint256 _homeFee,
uint256 _foreignFee
) public returns(bool)
{
_initialize(
Expand All @@ -93,7 +94,8 @@ contract HomeBridgeErcToNative is EternalStorage, BasicBridge, BasicHomeBridge,
);
require(isContract(_feeManager));
addressStorage[keccak256(abi.encodePacked("feeManagerContract"))] = _feeManager;
_setFee(_feeManager, _fee);
_setFee(_feeManager, _homeFee, HOME_FEE);
_setFee(_feeManager, _foreignFee, FOREIGN_FEE);
setInitialize(true);

return isInitialized();
Expand Down Expand Up @@ -157,7 +159,7 @@ contract HomeBridgeErcToNative is EternalStorage, BasicBridge, BasicHomeBridge,

address feeManager = feeManagerContract();
if (feeManager != address(0)) {
uint256 fee = calculateFee(valueToMint, false, feeManager);
uint256 fee = calculateFee(valueToMint, false, feeManager, FOREIGN_FEE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a bit messed up. Let's clarify this together one more time:
Currently the examples of the .env file says that;

# Fee to be charged for each transfer on Home network
# Makes sense only when HOME_REWARDABLE=true
# e.g. 0.1% fee
HOME_TRANSACTIONS_FEE=0.001
# Fee to be charged for each transfer on Foreign network
# Makes sense only when FOREIGN_REWARDABLE=true
# e.g. 0.1% fee
FOREIGN_TRANSACTIONS_FEE=0.001

What does Fee to be charged for each transfer on Foreign network mean?
FOREIGN_FEE means fee that we take for every transaction that directed form the Foreign network to the Home network.
So, HOME_FEE means fee that we take for every transaction that directed form the Home network to the Foreign network
Is it your understanding also? If so, does it make sense to re-phrase the statements in the examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarification. Yes, that's my understanding too. I'll update documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed on b54cd53

distributeFeeFromAffirmation(fee, feeManager);
valueToMint = valueToMint.sub(fee);
}
Expand All @@ -173,7 +175,7 @@ contract HomeBridgeErcToNative is EternalStorage, BasicBridge, BasicHomeBridge,
bytes32 txHash;
address contractAddress;
(recipient, amount, txHash, contractAddress) = Message.parseMessage(_message);
uint256 fee = calculateFee(amount, true, feeManager);
uint256 fee = calculateFee(amount, true, feeManager, HOME_FEE);
distributeFeeFromSignatures(fee, feeManager);
}
}
Expand Down
8 changes: 5 additions & 3 deletions contracts/upgradeable_contracts/native_to_erc20/ForeignBridgeNativeToErc.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ contract ForeignBridgeNativeToErc is ERC677Receiver, BasicBridge, BasicForeignBr
uint256 _homeMaxPerTx,
address _owner,
address _feeManager,
uint256 _fee
uint256 _homeFee,
uint256 _foreignFee
) public returns(bool) {
_initialize(
_validatorContract,
Expand All @@ -70,7 +71,8 @@ contract ForeignBridgeNativeToErc is ERC677Receiver, BasicBridge, BasicForeignBr
);
require(isContract(_feeManager));
addressStorage[keccak256(abi.encodePacked("feeManagerContract"))] = _feeManager;
_setFee(_feeManager, _fee);
_setFee(_feeManager, _homeFee, HOME_FEE);
_setFee(_feeManager, _foreignFee, FOREIGN_FEE);
setInitialize(true);
return isInitialized();
}
Expand Down Expand Up @@ -119,7 +121,7 @@ contract ForeignBridgeNativeToErc is ERC677Receiver, BasicBridge, BasicForeignBr
uint256 valueToMint = _amount;
address feeManager = feeManagerContract();
if (feeManager != address(0)) {
uint256 fee = calculateFee(valueToMint, false, feeManager);
uint256 fee = calculateFee(valueToMint, false, feeManager, HOME_FEE);
distributeFeeFromSignatures(fee, feeManager);
valueToMint = valueToMint.sub(fee);
}
Expand Down
8 changes: 5 additions & 3 deletions contracts/upgradeable_contracts/native_to_erc20/HomeBridgeNativeToErc.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicBridge, BasicHomeBridge,
uint256 _foreignMaxPerTx,
address _owner,
address _feeManager,
uint256 _fee
uint256 _homeFee,
uint256 _foreignFee
) public returns(bool)
{
_initialize(
Expand All @@ -72,7 +73,8 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicBridge, BasicHomeBridge,
);
require(isContract(_feeManager));
addressStorage[keccak256(abi.encodePacked("feeManagerContract"))] = _feeManager;
_setFee(_feeManager, _fee);
_setFee(_feeManager, _homeFee, HOME_FEE);
_setFee(_feeManager, _foreignFee, FOREIGN_FEE);
setInitialize(true);
return isInitialized();
}
Expand Down Expand Up @@ -118,7 +120,7 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicBridge, BasicHomeBridge,

address feeManager = feeManagerContract();
if (feeManager != address(0)) {
uint256 fee = calculateFee(valueToTransfer, false, feeManager);
uint256 fee = calculateFee(valueToTransfer, false, feeManager, FOREIGN_FEE);
distributeFeeFromAffirmation(fee, feeManager);
valueToTransfer = valueToTransfer.sub(fee);
}
Expand Down
12 changes: 8 additions & 4 deletions deploy/src/erc_to_native/home.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const {
FOREIGN_DAILY_LIMIT,
FOREIGN_MAX_AMOUNT_PER_TX,
HOME_REWARDABLE,
HOME_TRANSACTIONS_FEE
HOME_TRANSACTIONS_FEE,
FOREIGN_TRANSACTIONS_FEE
} = env

const DEPLOYMENT_ACCOUNT_ADDRESS = privateKeyToAddress(DEPLOYMENT_ACCOUNT_PRIVATE_KEY)
Expand Down Expand Up @@ -167,7 +168,8 @@ async function deployHome() {
console.log('[Home] feeManager Implementation: ', feeManager.options.address)
homeNonce++

const feeInWei = Web3Utils.toWei(HOME_TRANSACTIONS_FEE.toString(), 'ether')
const homeFeeInWei = Web3Utils.toWei(HOME_TRANSACTIONS_FEE.toString(), 'ether')
const foreignFeeInWei = Web3Utils.toWei(FOREIGN_TRANSACTIONS_FEE.toString(), 'ether')
console.log('\ninitializing Home Bridge with fee contract:\n')
console.log(`Home Validators: ${storageValidatorsHome.options.address},
HOME_DAILY_LIMIT : ${HOME_DAILY_LIMIT} which is ${Web3Utils.fromWei(HOME_DAILY_LIMIT)} in eth,
Expand All @@ -187,7 +189,8 @@ async function deployHome() {
)} in eth,
HOME_BRIDGE_OWNER: ${HOME_BRIDGE_OWNER},
Fee Manager: ${feeManager.options.address},
Fee: ${feeInWei} which is ${HOME_TRANSACTIONS_FEE * 100}%`)
Home Fee: ${homeFeeInWei} which is ${HOME_TRANSACTIONS_FEE * 100}%
Foreign Fee: ${homeFeeInWei} which is ${FOREIGN_TRANSACTIONS_FEE * 100}%`)
initializeHomeBridgeData = await homeBridgeImplementation.methods
.rewardableInitialize(
storageValidatorsHome.options.address,
Expand All @@ -201,7 +204,8 @@ async function deployHome() {
FOREIGN_MAX_AMOUNT_PER_TX,
HOME_BRIDGE_OWNER,
feeManager.options.address,
feeInWei
homeFeeInWei,
foreignFeeInWei
)
.encodeABI({ from: DEPLOYMENT_ACCOUNT_ADDRESS })
} else {
Expand Down
12 changes: 2 additions & 10 deletions deploy/src/loadEnv.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,20 +114,12 @@ if (BRIDGE_MODE === 'ERC_TO_NATIVE') {
}
}

if (HOME_REWARDABLE === 'true') {
validateRewardableAddresses(VALIDATORS, VALIDATORS_REWARD_ACCOUNTS)
validations = {
...validations,
VALIDATORS_REWARD_ACCOUNTS: addressesValidator(),
HOME_TRANSACTIONS_FEE: envalid.num()
}
}

if (FOREIGN_REWARDABLE === 'true') {
if (HOME_REWARDABLE === 'true' || FOREIGN_REWARDABLE === 'true') {
validateRewardableAddresses(VALIDATORS, VALIDATORS_REWARD_ACCOUNTS)
validations = {
...validations,
VALIDATORS_REWARD_ACCOUNTS: addressesValidator(),
HOME_TRANSACTIONS_FEE: envalid.num(),
FOREIGN_TRANSACTIONS_FEE: envalid.num()
}
}
Expand Down
10 changes: 7 additions & 3 deletions deploy/src/native_to_erc/foreign.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const {
BLOCK_REWARD_ADDRESS,
DPOS_VALIDATOR_SET_ADDRESS,
FOREIGN_REWARDABLE,
HOME_TRANSACTIONS_FEE,
FOREIGN_TRANSACTIONS_FEE
} = env

Expand Down Expand Up @@ -210,7 +211,8 @@ async function deployForeign() {
console.log('[Foreign] feeManager Implementation: ', feeManager.options.address)
foreignNonce++

const feeInWei = Web3Utils.toWei(FOREIGN_TRANSACTIONS_FEE.toString(), 'ether')
const homeFeeInWei = Web3Utils.toWei(HOME_TRANSACTIONS_FEE.toString(), 'ether')
const foreignFeeInWei = Web3Utils.toWei(FOREIGN_TRANSACTIONS_FEE.toString(), 'ether')

console.log('\ninitializing Foreign Bridge with fee contract:\n')
console.log(`Foreign Validators: ${storageValidatorsForeign.options.address},
Expand All @@ -230,7 +232,8 @@ async function deployForeign() {
)} in eth,
FOREIGN_BRIDGE_OWNER: ${FOREIGN_BRIDGE_OWNER},
Fee Manager: ${feeManager.options.address},
Fee: ${feeInWei} which is ${FOREIGN_TRANSACTIONS_FEE * 100}%`)
Home Fee: ${homeFeeInWei} which is ${HOME_TRANSACTIONS_FEE * 100}%
Foreign Fee: ${homeFeeInWei} which is ${FOREIGN_TRANSACTIONS_FEE * 100}%`)

initializeFBridgeData = await foreignBridgeImplementation.methods
.rewardableInitialize(
Expand All @@ -245,7 +248,8 @@ async function deployForeign() {
HOME_MAX_AMOUNT_PER_TX,
FOREIGN_BRIDGE_OWNER,
feeManager.options.address,
feeInWei
homeFeeInWei,
foreignFeeInWei
)
.encodeABI({ from: DEPLOYMENT_ACCOUNT_ADDRESS })
} else {
Expand Down
12 changes: 8 additions & 4 deletions deploy/src/native_to_erc/home.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const {
FOREIGN_DAILY_LIMIT,
FOREIGN_MAX_AMOUNT_PER_TX,
HOME_REWARDABLE,
HOME_TRANSACTIONS_FEE
HOME_TRANSACTIONS_FEE,
FOREIGN_TRANSACTIONS_FEE
} = env

const DEPLOYMENT_ACCOUNT_ADDRESS = privateKeyToAddress(DEPLOYMENT_ACCOUNT_PRIVATE_KEY)
Expand Down Expand Up @@ -166,7 +167,8 @@ async function deployHome() {
console.log('[Home] feeManager Implementation: ', feeManager.options.address)
homeNonce++

const feeInWei = Web3Utils.toWei(HOME_TRANSACTIONS_FEE.toString(), 'ether')
const homeFeeInWei = Web3Utils.toWei(HOME_TRANSACTIONS_FEE.toString(), 'ether')
const foreignFeeInWei = Web3Utils.toWei(FOREIGN_TRANSACTIONS_FEE.toString(), 'ether')

console.log('\ninitializing Home Bridge with fee contract:\n')
console.log(`Home Validators: ${storageValidatorsHome.options.address},
Expand All @@ -186,7 +188,8 @@ async function deployHome() {
)} in eth,
HOME_BRIDGE_OWNER: ${HOME_BRIDGE_OWNER},
Fee Manager: ${feeManager.options.address},
Fee: ${feeInWei} which is ${HOME_TRANSACTIONS_FEE * 100}%`)
Home Fee: ${homeFeeInWei} which is ${HOME_TRANSACTIONS_FEE * 100}%
Foreign Fee: ${homeFeeInWei} which is ${FOREIGN_TRANSACTIONS_FEE * 100}%`)

homeBridgeImplementation.options.address = homeBridgeStorage.options.address
initializeHomeBridgeData = await homeBridgeImplementation.methods
Expand All @@ -201,7 +204,8 @@ async function deployHome() {
FOREIGN_MAX_AMOUNT_PER_TX,
HOME_BRIDGE_OWNER,
feeManager.options.address,
feeInWei
homeFeeInWei,
foreignFeeInWei
)
.encodeABI({ from: DEPLOYMENT_ACCOUNT_ADDRESS })
} else {
Expand Down
8 changes: 4 additions & 4 deletions docs/ERC-TO-NATIVE-WITH-REWARD.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ EternalStorageProxy|upgradeTo|35871|30924|30913
RewardableValidators|initialize|202711|423292|318008
EternalStorageProxy|transferProxyOwnership|30653|30653|30653
EternalStorageProxy|deployment|378510|378510|378510
HomeBridgeErcToNative|deployment|5088498|5088498|5088498
HomeBridgeErcToNative|deployment|5653534|5653534|5653534
EternalStorageProxy|upgradeTo|35871|30924|30913
FeeManagerErcToNative|deployment|849694|849694|849694
HomeBridgeErcToNative|rewardableInitialize|327113|327177|327161
FeeManagerErcToNative|deployment|1068197|1068197|1068197
HomeBridgeErcToNative|rewardableInitialize|353084|353148|353132
EternalStorageProxy|transferProxyOwnership|30653|30653|30653
Total| |8973874|9184625|9079303
Total| |9783384|9994135|9888813

##### Foreign
Contract | Method | Min | Max | Avg
Expand Down
Loading