[Ethereum] Transfer ether from one payable contract to another not working

etherremixsolidity

Scenario:

I have users who will initiate booking for one product with x price. This contract will create a wallet contract between that user and the product so that he can return/cancel the booking at any time and get the refund or succeed.

Code:

User talks via contract EthBooking

contract EthBooking {
    function addBooking(uint listingId) public payable returns(uint bookingId) {
    bookingId =  BookingLibrary.book(ethDB, listingId, msg.sender, msg.value);

    //Send money to the contract
    return bookingId;
}
}

Booking Library code:

library BookingLibrary {

function book(address db, uint listingId, address sender, uint value) 
        internal returns (uint bookingId) 
{
    var cost = ListingLibrary.getCost(db,listingId);
        //Create Contract wallet
        EthWallet wallet = new EthWallet();
        //Send money to the contract
        wallet.receiveFunds.value(cost);
    return bookingId;
}

EthWallet Code:

contract EthWallet is Mortal {

function ethWallet() {
}

function receiveFunds() payable {

}

function sendFunds(address receiver, uint amount) {
    receiver.transfer(amount);
}

}

PROBLEM

The ethers sent is held in contract EthBooking and is not transferred to EthWallet as expected with code wallet.receiveFunds.value(cost); .

Please guide me about if the chosen approach is right and also why the ethers are not transferred to the destination.
PS: The code is shortened with removal of irrelevant parts to avoid confusion

Best Answer

I mocked this up in Truffle.

Using your way:

wallet.receiveFunds.value(cost);

Resulted in failure:

[ Instantiate Contract ]
    √ should instantiate the pact correctly
Wallet address: 0xa830ec357b950fde26d220b0f256b6d47529bdbd
Amount sent in wei: 3000000000000000000
Balance in eth wallet: 0
    (failed) 1) should transfer funds using library

Using this method:

wallet.receiveFunds.value(cost)();

Resulted in success:

[ Instantiate Contract ]
    √ should instantiate the pact correctly
Wallet address: 0xcc9fa65469197707e06eb16c2b01b9a62ddea75f
Amount sent in wei: 3000000000000000000
Balance in eth wallet: 3000000000000000000
    √ should transfer funds using library (410ms)

Others that worked:

wallet.send(cost);  // made a fallback function in EthWallet for this
wallet.transfer(cost);    // same as above, fallback function
address(wallet).call.value(cost)();  // fallback again

See Sending and Receiving Ether @ http://solidity.readthedocs.io/en/develop/security-considerations.html?highlight=send%20transfer

Other Notes: Your constructor should be exactly the same. Not ethWallet but EthWallet.

Related Solutions

Solidity – VM Exception: Invalid JUMP When Calling a Second Function

You need to add the constant keyword to the test() function like:

pragma solidity ^0.4.0;
contract Test {
    mapping (address => uint256) weismap;

    function bet(uint vote) payable returns (uint256 weis) {
        if (msg.value==0) 
            throw;
        weismap[msg.sender]= msg.value;
        return  weismap[msg.sender];
    }

    function test() constant returns (uint myVote)  {
        return 1;
    }
}

Calling bet(...) with the value 2. Note that I'm sending 2 ethers as well with the transaction:

Now I call test() AND I've left the value 2. This call to test() fails as the constant function is being called as a transaction, not a read of the values off the blockchain:

If I call test() AND I've set value to blank, this call to test() succeeds as the constant function is being called as a read of the values off the blockchain:

solidity – Calling One Contract Method from Another Contract

pragma solidity 0.4.21;

contract Base {

    uint public dataA;
    bytes32 public dataB;

    function setAB(uint a, bytes32 b) public {
        dataA = a;
        dataB = b;
    }

    function getA() public view returns(uint) {
        return dataA ;
    }

    function getB() public view returns(bytes32) {
        return dataB ;
    }

}

contract Extra {

   Base base;

   function Extra() public {
       base = new Base();
   }

   function getBaseAddres() public view returns(address) {
       return address(base);
   }

   function baseGetA() public view returns(uint) {
        return base.getA();
   }

   function baseGetB() public view returns(bytes32) {
       return base.getB();
   }

   function baseSetAB(uint a, bytes32 b) public returns(bool success) {
       base.setAB(a,b);
       return true;
   }
}

Hope it helps.

UPDATE

The above example just creates a new Base with each deployment of Extra to show them talking in the simplest manner I can think of. Another way to go is to deploy a Base separately. It's location can then be relayed to Extra.

You could pass it into the constructor:

Base base;

function Extra(address baseAddress) public {
  base = Base(baseAddress);
}

It's not checking anything, merely assuming the deployer knows what they're doing. You can also make the address of the base something the owner can change.

Add an owner to the state and the constructor:

address public owner;

function Extra(address baseAddress) public {
  base = Base(baseAddress);
  owner = msg.sender;
}

Add a modifier (after state and before constructor, by convention) to restrict access to the sensitive function.

modifier onlyOwner {
  require(msg.sender == owner);
  _;
}

Add a restricted function to change the base.

function changeBase(address newBase) public onlyOwner returns(bool success) {
  base = Base(newBase);
  return true;
}

You might want to know the address of the Base:

return address(base);

It would be good practice to add event emitters to the state-changing functions in both contracts.

Best Answer

Related Solutions

Solidity – VM Exception: Invalid JUMP When Calling a Second Function

solidity – Calling One Contract Method from Another Contract

Related Topic