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.

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

Arbitrary Message Bridging in the subsidized mode #73

Closed
akolotov opened this issue Aug 20, 2018 · 36 comments
Closed

Arbitrary Message Bridging in the subsidized mode #73

akolotov opened this issue Aug 20, 2018 · 36 comments
Assignees

Comments

@akolotov
Copy link
Collaborator

akolotov commented Aug 20, 2018

Based on feature description available here: https://hackmd.io/61fw0kf8T2C9bvDTOpzyVQ

POA network projects provides a set of bridges working in EVM-based
networks which allows to transfer assets from one blockchain to another.
At this moment the POA bridge supports two modes:

  • native-to-erc20 (used in production)
  • erc20-to-erc20 (in development stage)

Intorduction of every new bridge mode requires changes as on both the contract side and server side. It increase efforts needed for development and testing. That's why it is good idea to think of generic bridging approach which would allow transfer arbitrary data between two blockchains as so any depending on receiver the data could be interpreted as transfer of digital assets or an arbitrary contract method invocation.

Basic bridge functionality

Let's consider that such kind of bridge has several components:

  1. A contract in Home network which sends a message to another blockchain - HomeSender;
  2. A contract in Foreign to receive the message from Home blockchain - ForeignReceiver;
  3. A bridge contract in Home network which is responsible for transferring the message from Home- HomeBridge;
  4. A bridge contract in Foreign network which is responsible for delivering the message to ForeignReceiver.
  5. Bridge process (server) connected to both Home and Foreign blockchains which is responsible for listening events on Home side and creating transactions in Foreign side.

Having this we could describe the process of transferring the message as the set of the following steps:

  1. A user performs an action to interact with HomeSender by invoking directly or indirectly its method as so HomeSender needs to call the method Action1 from ForeignReceiver contract with some integer parameter P.
function Action1(uint256 P);
  1. HomeSender invokes the method requireToPassMessage from HomeBridge and pass the following parameters:
    • contractAddress - address of ForeignReceiver;
    • data - set of bytes where the selector of Action1 and value of parameter P encoded;
    • gas - estimated value of gas which is expected to be spent for Action1 execution.
function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas);

Example of way to encode data:

bytes4 methodSelector = ForeignReceiver(0).Action1.selector;
bytes memory data = abi.encodeWithSelector(methodSelector, P);

which equals to

bytes4 methodSelector = keccak256("Action1(uint256)");
bytes memory data = abi.encodeWithSelector(methodSelector, P);
  1. The method requireToTransfer raises the event RequestForSignature with the following parameters:
RequestForSignature(msg.sender, contractAddress, data, gas)

where msg.sender is an address of the contract that calls requireToTransfer.

In order to secure the bridge operation it also could be worth to set a maximum value of estimated gas can be requested in one transaction. A daily limit for gas could be introduced as well.

  1. The bridge process catches RequestForSignature event and initiates procedure to sign the request to transfer data.
  2. As soon as enough signatures are collected in the bridge contract, it emits CollectedSignatures event which contains a hash of the transaction raised initial RequestForSignature:
CollectedSignatures(msg.sender, txhash, numberOfSignaturesCollected)
  1. The bridge catches CollectedSignatures event and calls executeSignatures of the bridge contract ForeignBridge on Foreign side with the list of collected signatures.
executeSignatures(vs, rs, ss, contractSender, contractAddress, 
                  data, gas, txhash)

A validator generates her signature combining the set of parameters:

abi.encodePacked(contractSender, contractAddress, data, gas, txhash)

Gas value specified for the transaction that invokes executeSignatures is calculated as amount of gas needed for execution of executeSignatures itself plus gas specified by HomeSender.

  1. The method executeSignatures after signatures verification invokes a method specified in data from contractAddress:
contractAddress.call.gas(gas)(data);

A simple example demostrating minimal functionality of the bridge:

contract ContractOnHome {
    HomeBridge bridge;
    ContractOnForeign darkSide;
    
    constructor(HomeBridge _bridge, ContractOnForeign _contract) public {
        bridge = _bridge;
        darkSide = _contract;
    }
    
    function setValueOnForeign(uint256 _i) public {
        bytes4 methodSelector = ContractOnForeign(0).setValue.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _i);
        bridge.requireToPassMessage(darkSide, encodedData, 100000);
    }
}

contract HomeBridge {
    event RequestForSignature(address contractReceiver, bytes encodedData, uint256 gas);
    
    function requireToPassMessage(address _contract, bytes _data, uint256 _gas) public {
        emit RequestForSignature(_contract, _data, _gas);
    }
}

contract ForeignBridge {
    function executeSignatures(address _contract, bytes _data, uint256 _gas) public {
        require(_contract.call.gas(_gas)(_data));
    }
}

contract ContractOnForeign {
    uint256 public i;
    
    function setValue(uint256 _i) public {
        i = _i;
    }
}

Bridge operations compensation

In cases when a method that is going to be invoked on Foreign network side could consume lots of gas or the bridge is going to be used intensively it makes sense to introduce gaining fees to compensate bridge operations.

In the approach described below it is assumed that there is a registration of HomeSender contract address in ForeignBridge. Funds to cover bridge expenses must be deposited as part of the contract registration.

In this case HomeBridge could provide interface to call requireToPassMessage with different set of arguments:

function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas);
function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas, uint256 gasPrice);
function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas, bytes1 oracleGasPriceSpeed);

Where

  • gasPrice - fee to cover expenses of the bridge to execute the method Action1 for every gas unit.
  • oracleGasPriceSpeed - an integer which defines the gas price speed used by the gas price oracle (0x01 - instant, 0x02 - fast, 0x03 - standard, 0x04 - slow).

Therefore if gasPrice is specified as part of the method requireToPassMessage invocation it is necessary to check that it is greater than some minimum value.

RequestForSignature also allows to be emitted in several ways:

RequestForSignature(msg.sender, contractAddress, data, gas)
RequestForSignature(msg.sender, contractAddress, data, gas, gasPrice)
RequestForSignature(msg.sender, contractAddress, data, gas, oracleGasPriceSpeed)

executeSignatures of the bridge contract ForeignBridge will have three implementation:

executeSignatures(vs, rs, ss, contractSender, contractAddress, data, gas, txhash)

executeSignatures(vs, rs, ss, contractSender, contractAddress, data, gas,
                  gasPrice, txhash)

executeSignatures(vs, rs, ss, contractSender, contractAddress, data, gas,
                  oracleGasPriceSpeed, txhash)

Depending on the form of the method the following combination of parameters is used to verify the validators signatures:

abi.encodePacked(contractSender, contractAddress, data, gas, txhash)

abi.encodePacked(contractSender, contractAddress, data, gas, gasPrice, txhash)

abi.encodePacked(contractSender, contractAddress, data, gas,
                 oracleGasPriceSpeed, txhash)

If the signatures vaildation is correct the deposit of contractSender account is reduced by (executeSignaturesGas + gas) * gasPrice value, where executeSignaturesGas is a constant that present amount of gas needed to execute executeSignatures method. If oracleGasPriceSpeed is specified instead of gasPrice as per the method call, gasPrice should be equal tx.gasprice.

The contract must also force the transaction sender use gasPrice initially specified by the HomeSender through the check that gasPrice is equal tx.gasprice.

The corresponding amount of coins is sent to the address that invokes executeSignatures method.

The main idea of the bridge contracts with compensation of bridge operations demostrated below:

pragma solidity ^0.4.24;

contract ContractOnHome {
    HomeBridge homeBridge;
    ForeignBridge foreignBridge;
    ContractOnForeign darkSide;
    
    uint256 constant WITHDRAW_GAS_USAGE = 100000; // must be provided by the bridge operator
    uint256 constant SETVALUE_GAS_USAGE = 90000; // must be provided by the application contract
    uint256 constant FOREIGN_GAS_PRICE = 1; //1 wei - gasprice in JS VM
    
    constructor(HomeBridge _bridgeAtHome, ForeignBridge _bridgeAtForeign, ContractOnForeign _contract) public {
        homeBridge = _bridgeAtHome;
        foreignBridge = _bridgeAtForeign;
        darkSide = _contract;
    }
    
    function depositClearanceOnForeign(address _depostRecipient) public {
        require(_depostRecipient != address(0));
        bytes4 methodSelector = ForeignBridge(0).withdrawFromDeposit.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _depostRecipient);
        homeBridge.requireToPassMessage(foreignBridge, 
                                        encodedData, 
                                        WITHDRAW_GAS_USAGE, 
                                        FOREIGN_GAS_PRICE);
    } 
    
    function setValueOnForeign(uint256 _i) public {
        bytes4 methodSelector = ContractOnForeign(0).setValue.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _i);
        homeBridge.requireToPassMessage(darkSide, 
                                        encodedData, 
                                        SETVALUE_GAS_USAGE, 
                                        FOREIGN_GAS_PRICE);
    }
}

contract HomeBridge {
    event RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas, uint256 gasPrice);
    
    function requireToPassMessage(address _contract, bytes _data, uint256 _gas, uint256 _gasPrice) public {
        emit RequestForSignature(msg.sender, _contract, _data, _gas, _gasPrice);
    }
}

contract ForeignBridge {
    uint256 constant PASS_MESSAGE_GAS = 100000;
    mapping (address => uint256) public balanceOf;
    address accountToWithdraw = address(0);
    
    function withdrawFromDeposit(address _recipient) public {
        require(msg.sender == address(this));
        require(accountToWithdraw != address(0));
        require(balanceOf[accountToWithdraw] > 0);
        uint256 withdrawValue = balanceOf[accountToWithdraw];
        balanceOf[accountToWithdraw] = 0;
        _recipient.transfer(withdrawValue);
        accountToWithdraw = address(0);
    }
    
    function depositForContractSender(address _contract) public payable {
        require(_contract != address(0));
        balanceOf[_contract] = balanceOf[_contract] + msg.value;
    }
    
    function executeSignatures(address _sender, address _contract, bytes _data, uint256 _gas, uint256 _gasPrice) public {
        require(_gasPrice == tx.gasprice);
        uint256 fee = (PASS_MESSAGE_GAS + _gas) * tx.gasprice;
        require(balanceOf[_sender] >= fee);
        require(address(this).balance >= fee);
        
        balanceOf[_sender] = balanceOf[_sender] - fee;

        //Special check to handle invocation of withdrawFromDeposit
        if (_contract == address(this)) { 
            bytes4 withdrawFromDepositSelector = this.withdrawFromDeposit.selector;
            if ((_data[0] == withdrawFromDepositSelector[0]) && 
                (_data[1] == withdrawFromDepositSelector[1]) &&
                (_data[2] == withdrawFromDepositSelector[2]) && 
                (_data[3] == withdrawFromDepositSelector[3])) {
                accountToWithdraw = _sender;
            }
        }

        require(_contract.call.gas(_gas)(_data));
        
        msg.sender.transfer(fee);
    }
}

contract ContractOnForeign {
    uint256 public i;
    
    function setValue(uint256 _i) public {
        i = _i;
    }
}

Configurable mode

Two approaches to work with expenses were presented above. Both of them could be used depending on validator abilities and owner of the bridge. It makes sense to provide a configuration parameter to define the mode of the bridge as so the same source code could be used for different types of bridge:

pragma solidity ^0.4.24;

contract ContractOnHome {
    HomeBridge homeBridge;
    ForeignBridge foreignBridge;
    ContractOnForeign darkSide;
    
    uint256 constant WITHDRAW_GAS_USAGE = 100000; // must be provided by the bridge operator
    uint256 constant SETVALUE_GAS_USAGE = 90000; // must be provided by the application contract
    uint256 constant FOREIGN_GAS_PRICE = 1; //1 wei - gasprice in JS VM
    
    constructor(HomeBridge _bridgeAtHome, ForeignBridge _bridgeAtForeign, ContractOnForeign _contract) public {
        homeBridge = _bridgeAtHome;
        foreignBridge = _bridgeAtForeign;
        darkSide = _contract;
    }
    
    function depositClearanceOnForeign(address _depostRecipient) public {
        require(_depostRecipient != address(0));
        bytes4 methodSelector = ForeignBridge(0).withdrawFromDeposit.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _depostRecipient);
        homeBridge.requireToPassMessage(foreignBridge, 
                                        encodedData, 
                                        WITHDRAW_GAS_USAGE, 
                                        FOREIGN_GAS_PRICE);
    } 
    
    function setValueOnForeign(uint256 _i) public {
        bytes4 methodSelector = ContractOnForeign(0).setValue.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _i);
        homeBridge.requireToPassMessage(darkSide, 
                                        encodedData, 
                                        SETVALUE_GAS_USAGE, 
                                        FOREIGN_GAS_PRICE);
    }
}

contract HomeBridge {
    bytes4 constant SUBSIDIZED_MODE = bytes4(keccak256("AMB-subsidized-mode"));
    bytes4 constant DEFRAYAL_MODE = bytes4(keccak256("AMB-defrayal-mode"));
    bytes4 public foreignBridgeMode = DEFRAYAL_MODE;
    
    function setSubsidizedModeForForeign() public {
        foreignBridgeMode = SUBSIDIZED_MODE;
    }

    function setDefrayalModeForForeign() public {
        foreignBridgeMode = DEFRAYAL_MODE;
    }
    
    event RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas);
    event RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas, uint256 gasPrice);
    event RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas, bytes1 oracleGasPriceSpeed);

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas) public {
        require(foreignBridgeMode == SUBSIDIZED_MODE);
        emit RequestForSignature(msg.sender, _contract, _data, _gas);
    }
    
    function requireToPassMessage(address _contract, bytes _data, uint256 _gas, uint256 _gasPrice) public {
        if (foreignBridgeMode == SUBSIDIZED_MODE)
            emit RequestForSignature(msg.sender, _contract, _data, _gas);
        else 
            emit RequestForSignature(msg.sender, _contract, _data, _gas, _gasPrice);
    }

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas, bytes1 _oracleGasPriceSpeed) public {
        if (foreignBridgeMode == SUBSIDIZED_MODE)
            emit RequestForSignature(msg.sender, _contract, _data, _gas);
        else 
            emit RequestForSignature(msg.sender, _contract, _data, _gas, _oracleGasPriceSpeed);
    }
}

contract ForeignBridge {
    bytes4 constant SUBSIDIZED_MODE = bytes4(keccak256("AMB-subsidized-mode"));
    bytes4 constant DEFRAYAL_MODE = bytes4(keccak256("AMB-defrayal-mode"));
    uint256 constant PASS_MESSAGE_GAS = 100000;
    mapping (address => uint256) public balanceOf;
    bytes4 public foreignBridgeMode = DEFRAYAL_MODE;
    address accountToWithdraw = address(0);

    function setSubsidizedModeForForeign() public {
        foreignBridgeMode = SUBSIDIZED_MODE;
    }

    function setDefrayalModeForForeign() public {
        foreignBridgeMode = DEFRAYAL_MODE;
    }

    function withdrawFromDeposit(address _recipient) public {
        require(msg.sender == address(this));
        require(accountToWithdraw != address(0));
        require(balanceOf[accountToWithdraw] > 0);
        balanceOf[accountToWithdraw] = 0;
        _recipient.transfer(balanceOf[accountToWithdraw]);
        accountToWithdraw = address(0);
    }
    
    function depositForContractSender(address _contract) public payable {
        require(_contract != address(0));
        balanceOf[_contract] = balanceOf[_contract] + msg.value;
    }

    function _passMessage(address _sender, address _contract, bytes _data, uint256 _gas) internal {
        //Special check to handle invocation of withdrawFromDeposit
        if (_contract == address(this)) { 
            bytes4 withdrawFromDepositSelector = this.withdrawFromDeposit.selector;
            if ((_data[0] == withdrawFromDepositSelector[0]) && 
                (_data[1] == withdrawFromDepositSelector[1]) &&
                (_data[2] == withdrawFromDepositSelector[2]) && 
                (_data[3] == withdrawFromDepositSelector[3])) {
                accountToWithdraw = _sender;
            }
        }

        require(_contract.call.gas(_gas)(_data));
    }
    
    function _defrayAndPassMessage(address _sender, address _contract, bytes _data, uint256 _gas) internal {
        uint256 fee = (PASS_MESSAGE_GAS + _gas) * tx.gasprice;
        require(balanceOf[_sender] >= fee);
        require(address(this).balance >= fee);
        
        balanceOf[_sender] = balanceOf[_sender] - fee;
        
        _passMessage(_sender, _contract, _data, _gas);

        msg.sender.transfer(fee);
    }

    function executeSignatures(address _sender, address _contract, bytes _data, uint256 _gas) public {
        require(foreignBridgeMode == SUBSIDIZED_MODE);
        _passMessage(_sender, _contract, _data, _gas);
    }
    
    function executeSignatures(address _sender, address _contract, bytes _data, uint256 _gas, uint256 _gasPrice) public {
        require(foreignBridgeMode == DEFRAYAL_MODE);
        require(_gasPrice == tx.gasprice);
        _defrayAndPassMessage(_sender, _contract, _data, _gas);
    }

    function executeSignatures(address _sender, address _contract, bytes _data, uint256 _gas, bytes1 _oracleGasPriceSpeed) public {
        require(foreignBridgeMode == DEFRAYAL_MODE);
        _defrayAndPassMessage(_sender, _contract, _data, _gas);
    }
}

contract ContractOnForeign {
    uint256 public i;
    
    function setValue(uint256 _i) public {
        i = _i;
    }
}

This will allow to configure different sides of the bridge differently. For example, the bridge on the side of a chain with low gas price could be configured to not require cover gas used by the validator, at the same time another side of the bridge could still expect payment for handling transactions.

@patitonar
Copy link
Contributor

patitonar commented Aug 28, 2018

Great job @akolotov , the approach looks really good. I only want to mention one trade-off regarding the name of functions and events.

On Javascript, normally to call a method you would do something like myContract.methods.myMethod(123)

Given:

function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas);
function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas, uint256 gasPrice);
function requireToPassMessage(address contractAddress, bytes data,
                           uint256 gas, bytes1 oracleGasPriceSpeed);

The second and third functions have the same amount of parameters but with different types.
Javascript has no way to match the argument to the correct expected type of the function, so for calling those functions you will need to do something like this:

myContract.methods['requireToPassMessage(address, bytes, uint256, uint256)'](args)

myContract.methods['requireToPassMessage(address, bytes, uint256, bytes1)'](args)

which is not very straightforward.

Something similar happens to Events names. Given:

RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas);
RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas, uint256 gasPrice);
RequestForSignature(address contractSender, address contractReceiver, bytes encodedData, uint256 gas, bytes1 oracleGasPriceSpeed);

To listen to the events on the contracts you won't be able to use the name of the events, you would have to get the signature hash and use it to filter by topic which also is not very straightforward.

I think it would be a good idea to consider adding specific names for each event and method to avoid this complexity.

@pablofullana pablofullana changed the title Arbitrary message bridging Arbitrary Message Bridging Aug 29, 2018
@fvictorio
Copy link
Contributor

With respect to compensation, would it make sense to use gasleft() to see how much gas was actually used?

Example: Let's say I use AMB to send a transaction to the foreign network. I set some gas limit but for some reason the method ends up using a lot less than that. With this implementation, the balance will be reduced by the whole amount, which may be unfair to the user.

The problem is that gasleft() would be called before the method ended, but this remaining cost could be included in PASS_MESSAGE_GAS.

In code, this would be something like:

uint256 fee = (PASS_MESSAGE_GAS + _gas) * tx.gasprice;
require(balanceOf[_sender] >= fee);
require(address(this).balance >= fee);
        
require(_contract.call.gas(_gas)(_data));

fee = fee - (gasleft() * tx.gasprice)

balanceOf[_sender] = balanceOf[_sender] - fee;

msg.sender.transfer(fee);

Does this make sense?

@akolotov
Copy link
Collaborator Author

@patitonar thanks for comment.
I don't it is any issue at all since the contracts above is a sketch of what is actually needed to be implemented, so in real contracts things could be a bit different but a bit complicated.

The main reason to use the same name for different events is to simplify watcher on the bridge application side. I would avoid listening bunch of events event if they are for the same action - to relay data to another side of the bridge.
In order to achieve my goal and address you valid comment we could use packed set of bytes and pass them in the raised event like in the code example below. In this case the bridge application will unpack the bytes set in order to extract data for additional actions (e.g. gasPrice or oracleGasPriceSpeed). Good news here is that the bridge application could pass this bytes set to sign method to get signature without a need to pack them.

contract TestContract0 {
    event TestEvent(bytes data);
    function EmitEvent() public {
        address someAddress = msg.sender;
        bytes32 someArbitraryData = keccak256(abi.encodePacked(someAddress));
        uint256 someInt = block.timestamp;
        bytes1 someByte = bytes1(keccak256(abi.encodePacked(someArbitraryData)));
        
        //data: 0x000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03
        emit TestEvent(abi.encode(someAddress, someArbitraryData));
        //data: 0x000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03000000000000000000000000000000000000000000000000000000005b877705
        emit TestEvent(abi.encode(someAddress, someArbitraryData, someInt));
        //data: 0x000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03cd00000000000000000000000000000000000000000000000000000000000000
        emit TestEvent(abi.encode(someAddress, someArbitraryData, someByte));

        //data: 0xca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03
        emit TestEvent(abi.encodePacked(someAddress, someArbitraryData));
        //data: 0xca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03000000000000000000000000000000000000000000000000000000005b877705
        emit TestEvent(abi.encodePacked(someAddress, someArbitraryData, someInt));
        //data: 0xca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03cd
        emit TestEvent(abi.encodePacked(someAddress, someArbitraryData, someByte));
    }
}

This approach will increase gas usage of the corresponding methods but not significantly: look at three methods below and corresponding comments how much they consume. We see there that usage abi.encodePacked could save some gas since less bytes is stored in the blockchain database in the transaction receipt.

contract TestContract1 {
    event TestEvent01(address, bytes32, uint256, bytes1);
    event TestEvent02(bytes data);

    //Consumes 24281 gas on Sokol-testnet
    function EmitEvent01() public {
        address someAddress = msg.sender;
        bytes32 someArbitraryData = keccak256(abi.encodePacked(someAddress));
        uint256 someInt = block.timestamp;
        bytes1 someByte = bytes1(keccak256(abi.encodePacked(someArbitraryData)));
        
        emit TestEvent01(someAddress, someArbitraryData, someInt, someByte);
    }
    
    //Consumes 25293 gas on Sokol-testnet
    function EmitEvent02() public {
        address someAddress = msg.sender;
        bytes32 someArbitraryData = keccak256(abi.encodePacked(someAddress));
        uint256 someInt = block.timestamp;
        bytes1 someByte = bytes1(keccak256(abi.encodePacked(someArbitraryData)));
        
        emit TestEvent02(abi.encode(someAddress, someArbitraryData, someInt, someByte));
    }

    //Consumes 25102 gas on Sokol-testnet
    function EmitEvent02WithPackedData() public {
        address someAddress = msg.sender;
        bytes32 someArbitraryData = keccak256(abi.encodePacked(someAddress));
        uint256 someInt = block.timestamp;
        bytes1 someByte = bytes1(keccak256(abi.encodePacked(someArbitraryData)));
        
        emit TestEvent02(abi.encodePacked(someAddress, someArbitraryData, someInt, someByte));
    }
}

The main reason to have the same names for methods is similar: it should simplify the logic of the bridge:

  1. the application contracts don't need to chose which method they need to call in different scenarios. Instead they will always call requireToPassMessage. Since Solidity compiler knows about type of input parameters it will generate correct selector for the method. It means that there is no limitation to have three requireToPassMessage with different set of parameters.
  2. a network participant must don't care which method to call on the contracts side to relay data. Especially in the case when an user would like to relay a message together with collected signatures from Home to Foreign instead of the bridge (such ability provided by the bridge contracts - anyone could extract the message and collected signatures from the contract on Home side and send the transaction to the contract on Foreign). In order to achieve the goal 'one method name for different type of data' it is suggested to have only one executeSignatures method and pass packed data to it. It will increase gas consumption insignificantly since requires to store two additional uint256 within the set of ABI encoded data passed the method and extra instructions to unpack the data. This approach is being demonstrated below.
contract TestContract2 {
    event TestEvent(address, bytes32, uint256, bytes1);
    
    function parseMessage(bytes _data)
        internal
        pure
        returns(address someAddress, bytes32 someArbitraryData, uint256 someInt, bytes1 someByte)
    {
        assembly {
            someAddress := and(mload(add(_data, 20)), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
            someArbitraryData := mload(add(_data, 52))
            someInt := mload(add(_data, 84))
            someByte := and(mload(add(_data, 116)), 0xFF00000000000000000000000000000000000000000000000000000000000000)
        }
    }
    
    //Passed: "0xCA35b7d915458EF540aDe6068dFe2F44E8fa733c", "0xb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03", 1535604485, "0xcd"
    //Consumes 27686 gas on Sokol-testnet
    function getParameters(address someAddress, bytes32 someArbitraryData, uint256 someInt, bytes1 someByte) public {
        emit TestEvent(someAddress, someArbitraryData, someInt, someByte);    
    }

    //Passed: "0xca35b7d915458ef540ade6068dfe2f44e8fa733cb1591967aed668a4b27645ff40c444892d91bf5951b382995d4d4f6ee3a2ce03000000000000000000000000000000000000000000000000000000005b877705cd"
    //Consumes 28224 gas on Sokol-testnet
    function getParametersFromData(bytes _data) public {
        address someAddress;
        bytes32 someArbitraryData;
        uint256 someInt;
        bytes1 someByte;
        (someAddress, someArbitraryData, someInt, someByte) = parseMessage(_data);
        emit TestEvent(someAddress, someArbitraryData, someInt, someByte);    
    }
}

@akolotov
Copy link
Collaborator Author

akolotov commented Aug 30, 2018

Bearing in mind described in the previous comment here is the next version for Home and Foreign bridge contracts:

pragma solidity ^0.4.24;

contract ContractOnHome {
    HomeBridge homeBridge;
    ForeignBridge foreignBridge;
    ContractOnForeign darkSide;
    
    uint256 constant WITHDRAW_GAS_USAGE = 100000; // must be provided by the bridge operator
    uint256 constant SETVALUE_GAS_USAGE = 90000; // must be provided by the application contract
    uint256 constant FOREIGN_GAS_PRICE = 1; //1 wei - gasprice in JS VM
    
    constructor(HomeBridge _bridgeAtHome, ForeignBridge _bridgeAtForeign, ContractOnForeign _contract) public {
        homeBridge = _bridgeAtHome;
        foreignBridge = _bridgeAtForeign;
        darkSide = _contract;
    }
    
    function depositClearanceOnForeign(address _depostRecipient) public {
        require(_depostRecipient != address(0));
        bytes4 methodSelector = ForeignBridge(0).withdrawFromDeposit.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _depostRecipient);
        homeBridge.requireToPassMessage(foreignBridge, 
                                        encodedData, 
                                        WITHDRAW_GAS_USAGE, 
                                        FOREIGN_GAS_PRICE);
    } 
    
    function setValueOnForeign(uint256 _i) public {
        bytes4 methodSelector = ContractOnForeign(0).setValue.selector;
        bytes memory encodedData = abi.encodeWithSelector(methodSelector, _i);
        homeBridge.requireToPassMessage(darkSide, 
                                        encodedData, 
                                        SETVALUE_GAS_USAGE, 
                                        FOREIGN_GAS_PRICE);
    }
}

contract HomeBridge {
    bytes4 constant SUBSIDIZED_MODE = bytes4(keccak256("AMB-subsidized-mode"));
    bytes4 constant DEFRAYAL_MODE = bytes4(keccak256("AMB-defrayal-mode"));
    bytes4 public foreignBridgeMode = DEFRAYAL_MODE;
    
    function setSubsidizedModeForForeign() public {
        foreignBridgeMode = SUBSIDIZED_MODE;
    }

    function setDefrayalModeForForeign() public {
        foreignBridgeMode = DEFRAYAL_MODE;
    }
    
    event RequestForSignature(bytes encodedData);

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas) public {
        require(foreignBridgeMode == SUBSIDIZED_MODE);
        emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x00), _data));
    }
    
    function requireToPassMessage(address _contract, bytes _data, uint256 _gas, uint256 _gasPrice) public {
        if (foreignBridgeMode == SUBSIDIZED_MODE)
            emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x00), _data));
        else 
            emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x01), _gasPrice, _data));
    }

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas, bytes1 _oracleGasPriceSpeed) public {
        if (foreignBridgeMode == SUBSIDIZED_MODE)
            emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x00), _data));
        else 
            emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x02), _oracleGasPriceSpeed, _data));
    }
}

contract ForeignBridge {
    bytes4 constant SUBSIDIZED_MODE = bytes4(keccak256("AMB-subsidized-mode"));
    bytes4 constant DEFRAYAL_MODE = bytes4(keccak256("AMB-defrayal-mode"));
    uint256 constant PASS_MESSAGE_GAS = 100000;
    mapping (address => uint256) public balanceOf;
    bytes4 public foreignBridgeMode = DEFRAYAL_MODE;
    address accountForAction = address(0);

    function setSubsidizedModeForForeign() public {
        foreignBridgeMode = SUBSIDIZED_MODE;
    }

    function setDefrayalModeForForeign() public {
        foreignBridgeMode = DEFRAYAL_MODE;
    }

    function withdrawFromDeposit(address _recipient) public {
        require(msg.sender == address(this));
        require(accountForAction != address(0));
        require(balanceOf[accountForAction] > 0);
        uint256 withdrawValue = balanceOf[accountForAction];
        balanceOf[accountForAction] = 0;
        _recipient.transfer(withdrawValue);
        accountForAction = address(0);
    }
    
    function depositForContractSender(address _contract) public payable {
        require(_contract != address(0));
        balanceOf[_contract] = balanceOf[_contract] + msg.value;
    }
    
    function isWithdrawFromDepositSelector(bytes _data) internal pure returns(bool _retval) {
        _retval = false;
        bytes4 withdrawFromDepositSelector = this.withdrawFromDeposit.selector;
        if ((_data[0] == withdrawFromDepositSelector[0]) && 
            (_data[1] == withdrawFromDepositSelector[1]) &&
            (_data[2] == withdrawFromDepositSelector[2]) && 
            (_data[3] == withdrawFromDepositSelector[3])) {
            _retval = true;    
        }        
    }

    function _passMessage(address _sender, address _contract, bytes _data, uint256 _gas) internal {
        if (_contract == address(this)) { 
            //Special check to handle invocation of withdrawFromDeposit
            if (isWithdrawFromDepositSelector(_data)) {
                accountForAction = _sender;
            }
        }

        require(_contract.call.gas(_gas)(_data));
    }
    
    function _defrayAndPassMessage(address _sender, address _contract, bytes _data, uint256 _gas) internal {
        uint256 fee = (PASS_MESSAGE_GAS + _gas) * tx.gasprice;
        require(balanceOf[_sender] >= fee);
        require(address(this).balance >= fee);
        
        balanceOf[_sender] = balanceOf[_sender] - fee;
        
        _passMessage(_sender, _contract, _data, _gas);

        msg.sender.transfer(fee);
    }

    function unpackData(bytes _data)
        internal
        pure
        returns(address sender, address executor, uint256 gasLimit, bytes1 dataType, uint256 gasPrice, bytes1 oracleGasPriceSpeed, bytes memory data)
    {
        uint256 datasize;
        uint256 srcdataptr = 20 + 20 + 32 + 1; //20 (sender)  + 20 (executor) + 32 (gasPrice) + 1 (dataType)
        assembly {
            sender := and(mload(add(_data, 20)), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
            executor := and(mload(add(_data, 40)), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
            gasLimit := mload(add(_data, 72))
            dataType := and(mload(add(_data, 104)), 0xFF00000000000000000000000000000000000000000000000000000000000000)
            switch dataType
            case 0x0000000000000000000000000000000000000000000000000000000000000000 {
                gasPrice := 0
                oracleGasPriceSpeed := 0x00
            }
            case 0x0100000000000000000000000000000000000000000000000000000000000000 {
                gasPrice := mload(add(_data, 105))
                oracleGasPriceSpeed := 0x00
                srcdataptr := add(srcdataptr, 0x20)
            }
            case 0x0200000000000000000000000000000000000000000000000000000000000000 {
                gasPrice := 0
                oracleGasPriceSpeed := and(mload(add(_data, 105)), 0xFF00000000000000000000000000000000000000000000000000000000000000)
                srcdataptr := add(srcdataptr, 0x01)
            }
            default {
                revert(0, 1)
            }
            datasize := sub(mload(_data), srcdataptr)
        }
        data = new bytes(datasize);
        assembly {
            let dataptr := add(data, /*BYTES_HEADER_SIZE*/32)
            calldatacopy(dataptr, add(68, srcdataptr), datasize) //68 = 4 (selector) + 32 (bytes header) + 32 (bytes length)
        }
    }

    function executeSignatures(bytes _data) public {
        address sender; 
        address executor; 
        uint256 gasLimit; 
        bytes1 dataType; 
        uint256 gasPrice;
        bytes1 oracleGasPriceSpeed;
        bytes memory data;
        (sender, executor, gasLimit, dataType, gasPrice, oracleGasPriceSpeed, data) = unpackData(_data);
        if (dataType == 0x00) {
            require(foreignBridgeMode == SUBSIDIZED_MODE);
            _passMessage(sender, executor, data, gasLimit);
        } else if (dataType == 0x01) {
            require(foreignBridgeMode == DEFRAYAL_MODE);
            require(gasPrice == tx.gasprice);
            _defrayAndPassMessage(sender, executor, data, gasLimit);
            
        } else if (dataType == 0x02) {
            require(foreignBridgeMode == DEFRAYAL_MODE);
            _defrayAndPassMessage(sender, executor, data, gasLimit);
        } else {
            revert();
        }
    }
}

contract ContractOnForeign {
    uint256 public i;
    
    function setValue(uint256 _i) public {
        i = _i;
    }
}

@akolotov
Copy link
Collaborator Author

@fvictorio thanks for the suggestion.

Yes, it could be implemented. But there are two things we need keep in mind here:

  1. Gas usage compensation could also be used in case of deposit withdrawal:
    executeSignatures
    .._defrayAndPassMessage
    ...._passMessage
    ......withdrawFromDeposit
    
    The changes as you suggested to implement could cause a situation when the particular deposit will be withdrawn and no funds will be available to compensate gas used for this operation. I though about another approach to pass a limit which can not be exceed withdrawFromDeposit and used further for compensation but such approach will not allow to clear deposit completely - some funds will be locked forever.
  2. It was assumed that gas unutilized by the application contract could be used as reward for a validator to relay the message and signatures from Home side to Foreign. I would avoid to include this reward in PASS_MESSAGE_GAS in order to allow applications to define how much they would like to pay for service. I would leave PASS_MESSAGE_GAS as close as it is possible to adequate value how much the invocation of executeSignatures could consume if the called contract's method performs no operations - it should increase trust to the bridge owners.

But if you could suggest some other ideas how validators could get reward and it will consistent with another bridge functionality, I am open for any suggestions!

@ghost ghost added in progress and removed backlog labels Sep 4, 2018
@patitonar
Copy link
Contributor

patitonar commented Sep 4, 2018

@akolotov Here it is mentioned that the logic to send/receive messages is going to be implemented on both sides of the bridge.
Is sending a message from Foreign to Home going to work on the same way as Home to Foreign? Example:

  • Call requireToPassMessage on Foreign Bridge, which emits RequestForSignature
  • The bridge process catches RequestForSignature event and sign the request (by calling submitSignature on Foreign Bridge?)
  • Emit CollectedSignatures when number of signatures reached
  • The bridge process catches CollectedSignatures event and calls executeSignatures on Home bridge

An other question about this, Are both sides going to work with the same mode? Or for example is there a possibility that passing messages from Home to Foreign could work on DEFRAYAL_MODE and passing messages from Foreign to Home on SUBSIDIZED_MODE?

@akolotov
Copy link
Collaborator Author

akolotov commented Sep 4, 2018

@patitonar thanks for asking
Actually, I would like to see a suggestion from you how the current architecture of the bridge contracts could be re-used to transfer messages from Foreign to Home. E.g. in the current architecture the names of event are different on opposite sides of the bridge: UserRequestForSignature vs UserRequestForAffirmation. The same is for methods which are being called by the bridge: executeAffirmation vs executeSignatures. Please also note that we don't need to collect signatures on Foreign side, every bridge instance sends executeAffirmation to confirm the user request.

@patitonar
Copy link
Contributor

@akolotov here are some comments that came out:

  1. Regarding sending messages from Foreign to Home I think there are two options:
    A. Make the contracts work on only one direction as they are now and then deploy them again but inverted. So one couple of contracts would be used to send messages from Home to Foreign and the other to send messages from Foreign to Home. The issue with this approach is that we would have more deployed contracts to keep track of (on bridge, ui, monitor) and on Foreign-to-Home side the validators signatures will be performed on foreign network which is more expensive.

    B. Re-use the current architecture of the bridge contracts and implement a similar approach. In this case some methods would be repeated for both contracts and it will be worth extracting them into base contracts.
    - On foreign bridge we will have the 3 requireToPassMessage methods similar as in home bridge, that will emit UserRequestForAffirmation
    - The bridge catches that event and call executeAffirmation on Home bridge

  2. A compensation mechanism is provided only for the last validator that submits their signature, who then makes the call to executeSignatures on foreign bridge. The validators that submitted the first signatures are not compensated. Should we analyze a compensation for them too?

  3. Right now we allow to execute a contract method, but we have a limitation that only methods that don't require value to be sent can be executed.
    We were thinking on supporting that use case too. An option could be that requireToPassMessage can receive the value argument that is going to be sent with the method call. Then on Foreign when executeSignatures is called, besides using the contract balance to pay the fee, it will use the balance to transfer the specified value with the method call.

  4. On the other bridge implementations when a validator calls submitSignature the function Message.isMessageValid(message) is called, checking the length of the message to ensure the message is valid.
    On AMB implementation, the length of the message will not be fixed because of the data field and the differents arguments it can receive (gasPrice or oracleGasPriceSpeed).
    What kind of checks should we perform in order to validate that an AMB message is considered valid?

  5. Are both sides going to work with the same SUBSIDIZED/DEFRAYAL mode? Or is there a possibility that passing messages from Home to Foreign works on DEFRAYAL_MODE and passing messages from Foreign to Home does it on SUBSIDIZED_MODE?

@akolotov
Copy link
Collaborator Author

@patitonar thanks for questions.

  1. I would recommend to re-use the current architecture. We cannot use approach you suggesting in A, since logic of contracts is not the same - they work with validators signatures differently: when you send a message from Home to Foreign, signatures are being collected on Home side and then being sent on Foreign in one transaction; when you send a message form Foreign to Home, signatures are being sent to Home by validators directly. I don't mind if methods to initiate a message relaying could be named similarly on both sides. The same is for events. But I have a question for you: is it good idea to have different events on different sides in order we could differentiate them easily in the bridge code (e.g. Home side of the bridge always listens for events RequestForSignature whereas Foreign side - RequestForAffirmation events)?

  2. The validators that submitted the first signatures are not compensated.

    It is assumed that validators always submit signatures on Home side where transactions are very cheap. So there is no need to compensate this.

  3. we have a limitation that only methods that don't require value to be sent can be executed.

    It was done intentionally since there is no automatic possibility to convert value sent to Home contract to value needed to be pay by validator to execute a method. That's why we will not support this feature at this moment.

  4. What kind of checks should we perform in order to validate that an AMB message is considered valid?

    We will not check length of the message at all.

  5. Or is there a possibility that passing messages from Home to Foreign works on DEFRAYAL_MODE and passing messages from Foreign to Home does it on SUBSIDIZED_MODE?

    Yes, this is the intention.

@akolotov
Copy link
Collaborator Author

It is suggested to enhance check for data requested to be sent to another side of the bridge.

In the sketches described previously the simple attack is possible - a contract could call requireToPassMessage by passing less gas than it is required to even send transaction with requested parameters.

For example, the method below will be executed successfully if _data is 0xFFFFFFFF whereas _gas is 100.

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas) public {
        require(foreignBridgeMode == SUBSIDIZED_MODE);
        emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x00), _data));
    }

And executeSignatures called with applying gas provided by the contract will fails since 100 gas is not enough to keep the transaction with 0xFFFFFFFF.

It does not make sense to allow such kind of requests.

It is worth to check that gas provided is enough at least to send transaction.

As per Ethereum Yellow Paper 68 gas paid for every non-zero byte of data or code for a transaction. It means that the contracts should declare gas value which is greater or equal 68 multiplied by the length of data:

    function getMinimumGasUsage(bytes _data) pure public returns(uint256 gas) {
        //From Ethereum Yellow Paper
        // 68 gas is paid for every non-zero byte of data or code for a transaction
        return _data.length * 68;
    }

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas) public {
        require(foreignBridgeMode == SUBSIDIZED_MODE);
        bytes memory messageToRelay=abi.encodePacked(msg.sender, _contract, _gas, uint8(0x00), _data);
        require(_gas >= getMinimumGasUsage(messageToRelay));
        emit RequestForSignature(messageToRelay);
    }

@fvictorio
Copy link
Contributor

fvictorio commented Sep 10, 2018

It was done intentionally since there is no automatic possibility to convert value sent to Home contract to value needed to be pay by validator to execute a method. That's why we will not support this feature at this moment.

The idea wouldn't be to convert value in home to value in foreign, but to send part of the value that is already deposited in the foreign contract.

For example, let's say I have a contract in foreign that checks that msg.value >= 0.1. I could deposit enough value to perform transactions and to send 0.1 eth in each call. If that feature is not present, I can only use the bridge to call methods that don't require value to be sent.

@fvictorio
Copy link
Contributor

fvictorio commented Sep 10, 2018

As per Ethereum Yellow Paper 68 gas paid for every non-zero byte of data or code for a transaction. It means that the contracts should declare gas value which is greater or equal 68 multiplied by the length of data:

Is this added to the minimum gas that is used for a transaction (21000, I think)? If that's the case, would it make sense to do return 21000 + _data.length * 68;?

@akolotov
Copy link
Collaborator Author

Is this added to the minimum gas that is used for a transaction (21000, I think)?

No, it is assumed that 21000 is included in PASS_MESSAGE_GAS. We need to differentiate gas requested to be spent as part of a target method invocation by call() and gas used by the bridge to support this.

In this case I think the approach could be refined to:

    function getMinimumGasUsage(bytes _data) pure public returns(uint256 gas) {
        //From Ethereum Yellow Paper
        // 68 gas is paid for every non-zero byte of data or code for a transaction
        return _data.length * 68;
    }

    function requireToPassMessage(address _contract, bytes _data, uint256 _gas) public {
        require(foreignBridgeMode == SUBSIDIZED_MODE);
        require(_gas >= getMinimumGasUsage(_data));
        emit RequestForSignature(abi.encodePacked(msg.sender, _contract, _gas, uint8(0x00), _data));
    }

where gas to handle msg.sender, _contract, _gas and uint8(0x00) is included into PASS_MESSAGE_GAS as well.

@akolotov
Copy link
Collaborator Author

akolotov commented Sep 10, 2018

If that feature is not present, I can only use the bridge to call methods that don't require value to be sent.

Right. This is the intention for the first version of the bridge. The behavior you are describing could be implemented if the contract we invoke through the bridge (ContractOnForeign in the example above) keep funds and implement transferring values:

contract ContractOnForeign {
    TargetContract someAddress;
    
    constructor (TargetContract _addr) public {
        someAddress = _addr;
    }
    
    function setValue(uint256 _i) public {
        require(address(this).balance >= 1 finney);
        someAddress.setValue.value(1 finney)(_i);
    }
}

contract TargetContract {
    uint256 public i;

    function setValue(uint256 _i) payable public {
        require(msg.value >= 1 finney);
        i = _i;
    }
}

@patitonar
Copy link
Contributor

is it good idea to have different events on different sides in order we could differentiate them easily in the bridge code (e.g. Home side of the bridge always listens for events RequestForSignature whereas Foreign side - RequestForAffirmation events)?

I think it's a good idea to keep the different names because they are processed in a different way and they are easily identified inside the bridge code

@ewingrj
Copy link

ewingrj commented Oct 9, 2018

hey guys, how's the progress going on this?

@akolotov
Copy link
Collaborator Author

akolotov commented Oct 9, 2018

@perissology so far we are busy with the launch of the erc-to-native mode for bridge (a stable token to a side-chain coins), that's why AMB was postponed for one month. As soon as we finish the new bridge, we will continue with AMB.

@ewingrj
Copy link

ewingrj commented Oct 10, 2018

thanks for the update 👍

patitonar added a commit that referenced this issue Oct 29, 2018
@ewingrj
Copy link

ewingrj commented Jan 3, 2019

Hey all! is this still in the roadmap?

@akolotov
Copy link
Collaborator Author

akolotov commented Jan 3, 2019

@perissology it was postponed for a while. We are currently implementing distributing rewards between validators for erc20-to-native and native-to-erc20 bridge modes. Then we will shift to support sharing the same validator list between dPoS consensus and the bridge confirmation mechanism. It is our current short term plans.

@adamskrodzki
Copy link

adamskrodzki commented Jan 23, 2019

@akolotov @patitonar

nice Work

Have You consider little different mechanics, where there is HomeBridge and ForeignBridge

and only keccak256(msg.data,_validSender, foreignContractAddress,_declaredValue) is send from one contract to another by validator

when enaught confirmations is made by validators

anyone can call method to ForeignContract

//this is in ForeignBridge
callFromTheOtherSide(uint256 _requestId,bytes data, address _foreignContract) payable{
 // check if hash under _requestId matches keccak256(data,msg.sender,_foreignContract, msg.value)
 // call  _foreignContract  with data and msg.value
}

I believe this approach has seeveral advantages

  1. Validators do not pay for execution of actual method on foreign Network, they just pay for storing single hash in ForeignBridge
  2. _foreignContract with use of msg.sener and tx.origin can easily validate that method was called by person they want
  3. You can easily send value since actuall caller is an actor on other network that wants to send that value

I was thinking about Replay attacks, but they can be easily imtigated on foreign Bridge by invalidation of _requestId after single use.

@akolotov
Copy link
Collaborator Author

akolotov commented Jan 23, 2019

@adamskrodzki please note that the code you are referring is still on the code review
BUT, yes

I want functionality where by making call on POA from SC to bridge (transaction X), after some period of time (when validators provide their signatures), it is possible to call (transaction Y) specific smart contract on ETH with payload determined during transaction X.

this is exactly how it is going to work.

Also I want achieve situation when size of payload of Transaction Y do not affect gas costs that need to be paid by Validator on ETH chain.
In ideal situation validator do not need to do any transaction on ETH at all to authorise transaction Y

Since in a special case when the collected-signatures is not run on the bridge instances, the validators will not relay signed message (transaction Y), so they will not pay for this transaction. Instead the user must monitor the event and invoke "executeSignatures" on the Foreign Bridge contract with data and signatures picked from the Home contract (parts of signatures vs, rs, ss provided by the validators to confirm exact _data to relay):
https://github.com/poanetwork/poa-bridge-contracts/blob/88755a957b7f129f3cb6fb84f0f9e9e04af751cc/contracts/upgradeable_contracts/arbitrary_message/BasicForeignAMB.sol#L11

@adamskrodzki
Copy link

@akolotov
Is this code currently running on testnets? (Sokol - Rinkeby/Ropsten) ?

@akolotov
Copy link
Collaborator Author

It is not. The code review was postponed due to other high priority development activities

@adamskrodzki
Copy link

ok,

I will then take a look and try to deploy it myself on testnets and try to implement some example contracts using it

patitonar added a commit that referenced this issue May 17, 2019
# Conflicts:
#	deploy/.env.example
#	deploy/src/loadEnv.js
#	package-lock.json
@patitonar
Copy link
Contributor

Here is a situation that we need to think how to handle in order to avoid getting stuck on processing the events from the validator oracle perspective.

On an example of collectedSignatures event, after processing the event the watcher will try to call executeSignatures(message, v, r, s).estimateGas(). It could be possible that the message that is going to be executed on ForeignReceiver could fail for some logic on that contract, or in the case of defrayal mode, it could also fail because of not having enough balance.

We need to properly handle those kind of failures on the contracts and/or on the oracle side so if call to estimateGas() fails, we can ignore it or take some specific action, or maybe emit some special event instead of reverting the transaction and so we can continue processing the other events without issues.

The same should be applied for executing the message on Home side.

patitonar added a commit that referenced this issue May 24, 2019
# Conflicts:
#	contracts/upgradeable_contracts/erc20_to_erc20/HomeBridgeErcToErc.sol
#	contracts/upgradeable_contracts/erc20_to_native/HomeBridgeErcToNative.sol
#	contracts/upgradeable_contracts/native_to_erc20/ForeignBridgeNativeToErc.sol
#	contracts/upgradeable_contracts/native_to_erc20/HomeBridgeNativeToErc.sol
#	deploy/src/loadEnv.js
#	flatten.sh
patitonar added a commit that referenced this issue May 27, 2019
# Conflicts:
#	deploy/package-lock.json
@akolotov
Copy link
Collaborator Author

@patitonar thanks for question!

So far I have no generic answer on your question. So, let's consider solutions case by case:

  1. Should we call estimateGas? I don't think so since we have all necessary data already: PASS_MESSAGE_GAS and gasLimit. Sum of these two values is exactly what we need to pass as gas for the executeSignatures transaction.

  2. It could be possible that the message that is going to be executed on ForeignReceiver could fail for some logic on that contract

    It is a valid point. We should consider this operation as failed and do not allow to execute the call again after reset of the block by the bridge node. So the function _passMessage could look like:

     event MessageRelayed(bool);
    
     function _passMessage(address _sender, address _contract, bytes _data, uint256 _gas) internal {
         if (_contract == address(this)) { 
             //Special check to handle invocation of withdrawFromDeposit
             if (isWithdrawFromDepositSelector(_data)) {
                 accountForAction = _sender;
             }
         }
    
         bool status = _contract.call.gas(_gas)(_data);
         emit MessageRelayed(status);
     }

    Does it look safe enough? if the call is interrupted due to out-of-gas, the value of status will be false, if the call is reverted, the value of status will be false. In both cases the new state introduced by the operations happened due to the call will be reverted. Is this your understanding too?

  3. or in the case of defrayal mode, it could also fail because of not having enough balance.

    First of all, it could be checked on the oracle side. Does it make sense to not send such kind of transactions at all in order to prevent dying out of the validator's account? The transaction will not be registered in the contract as handled so the block restart will re-send it. From another point of view this behavior is not what we would like to achieve since every time when someone forgets to fill the balance on the bridge contract we will get a request to restart the validators. That's why we always need to send a transaction even if it is failed and someone should pay for it. Or at least we need to make it expensive to dry out the validator's account. It leads us to the necessity to take fees at the moment when transaction just appeared on one of the bridge sides. I need to think more for the process how it should work. If you have any idea - please share.

@patitonar
Copy link
Contributor

@akolotov thanks for the answers!

  1. Sum of PASS_MESSAGE_GAS and gasLimit is the gas needed for executeSignatures on foreign, but on the case for executeAffirmation also gas for submitting the signature should be added. To avoid any manual calculation I think we should continue calling estimateGas and also it is useful to protect the validator sending transactions that are going to fail because of they were already signed or processed in the case of re-processing previous blocks.

  2. The solution you mentioned looks good to me, it is exactly of what I had in mind for a possible solution. We should think which parameter should be on the new Event beside of the status so it will be easier to identify if the call failed from the user perspective. Maybe txHash would be useful. This solution will also make estimateGas call work OK since it won't fail in case the contract call fails.

  3. Good job on describing the scenarios. I cannot see a clear way to solve this right now, I'll try to think of possibles approaches including the one you mention about taking fees at the moment when transaction just appeared on one of the bridge sides.

@akolotov
Copy link
Collaborator Author

Sum of PASS_MESSAGE_GAS and gasLimit is the gas needed for executeSignatures on foreign, but on the case for executeAffirmation also gas for submitting the signature should be added. To avoid any manual calculation I think we should continue calling estimateGas and also it is useful to protect the validator sending transactions that are going to fail because of they were already signed or processed in the case of re-processing previous blocks.

OK. It could indeed reduce the number of RPC calls in the good case. But we will need to change PASS_MESSAGE_GAS every time when change complexity of these calls.

Maybe txHash would be useful.

Sure. I based on the sketch presented above and txHash` was not introduced there for simplification of the concept demonstration.

@akolotov
Copy link
Collaborator Author

@patitonar during the discussion on the POA Network forum
It was found that a target contract that gets a message through the bridge must know the information about the message sender in some cases:

So, it was suggested to implement in AMB a functionality similar to accountForAction described above:

contract AMB {
    address messageSender;

    function getMessageSender() external view returns (address) {
         return messageSender;
    }

    function _passMessage(address _sender, address _contract, bytes _data, uint256 _gas) internal {
         if (_contract == address(this)) { 
             //Special check to handle invocation of withdrawFromDeposit
             if (isWithdrawFromDepositSelector(_data)) {
                 accountForAction = _sender;
            }
         }

         messageSender = _sender;
         bool status = _contract.call.gas(_gas)(_data);
         messageSender = address(0);
         emit MessageRelayed(status);
    }
}

contract targetContract  {
    address public pairedContract;
    AMB public bridge;
    
    bytes32 data;
    
    function targetMethod(bytes32 _data) external returns (bool) {
        require(msg.sender == address(bridge));
        require(bridge.getMessageSender() == pairedContract);
        data = _data;
        return true;
    }
}

What do you think?

@patitonar
Copy link
Contributor

@akolotov Looks good to me, I'll update the implementation with this feature

patitonar added a commit that referenced this issue Jul 16, 2019
# Conflicts:
#	contracts/upgradeable_contracts/BasicBridge.sol
#	contracts/upgradeable_contracts/ERC677Bridge.sol
#	contracts/upgradeable_contracts/erc20_to_erc20/BasicForeignBridgeErcToErc.sol
#	contracts/upgradeable_contracts/erc20_to_erc20/HomeBridgeErcToErc.sol
#	contracts/upgradeable_contracts/erc20_to_native/ForeignBridgeErcToNative.sol
#	contracts/upgradeable_contracts/erc20_to_native/HomeBridgeErcToNative.sol
#	contracts/upgradeable_contracts/native_to_erc20/ForeignBridgeNativeToErc.sol
#	contracts/upgradeable_contracts/native_to_erc20/HomeBridgeNativeToErc.sol
#	package-lock.json
#	package.json
patitonar added a commit that referenced this issue Jul 17, 2019
# Conflicts:
#	deploy/src/loadEnv.js
#	package-lock.json
patitonar added a commit that referenced this issue Jul 17, 2019
# Conflicts:
#	contracts/upgradeable_contracts/BasicBridge.sol
patitonar added a commit that referenced this issue Jul 22, 2019
# Conflicts:
#	contracts/upgradeable_contracts/BasicBridge.sol
#	contracts/upgradeable_contracts/BasicHomeBridge.sol
#	contracts/upgradeable_contracts/ERC677Bridge.sol
#	package-lock.json
patitonar added a commit that referenced this issue Aug 8, 2019
# Conflicts:
#	contracts/upgradeable_contracts/BasicBridge.sol
#	contracts/upgradeable_contracts/BasicForeignBridge.sol
#	contracts/upgradeable_contracts/ERC677Bridge.sol
#	package-lock.json
#	package.json
patitonar added a commit that referenced this issue Aug 22, 2019
patitonar added a commit that referenced this issue Aug 23, 2019
# Conflicts:
#	contracts/interfaces/IAMB.sol
@igorbarinov igorbarinov pinned this issue Aug 28, 2019
patitonar added a commit that referenced this issue Sep 4, 2019
# Conflicts:
#	contracts/upgradeable_contracts/BasicBridge.sol
@akolotov akolotov unpinned this issue Sep 6, 2019
@akolotov akolotov changed the title Arbitrary Message Bridging Arbitrary Message Bridging in the subsidized mode Sep 7, 2019
@akolotov
Copy link
Collaborator Author

akolotov commented Sep 9, 2019

The issue is being closed since the feature was implemented for the release 3.0.0

@akolotov akolotov closed this as completed Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants