Transactions of ether between contracts

Question

I am trying to figure out how transactions between contracts work and what you can and cannot do. I want to have a contract that makes some logical decision about who to pay when poked by another contract.

Here is the contract i've been trying to emulate this behaviour with:

pragma solidity ^0.4.17;

contract DecisionMaker{

    Sender sd;

    function someLogicToDecidePayment(address receiver, address sender) public {
        //Some logic then:
        sd = Sender(sender);
        sd.send_transfer(receiver, 1000);
    }

}

contract Sender {
    DecisionMaker dm;

    constructor() public payable{}

    function send(address _receiver) payable {
        _receiver.call.value(10000000).gas(20317)();
    }

    function send_transfer(address _receiver, uint _amount) payable public {
        _receiver.transfer(_amount);
    }

    function placeBuy(address receiver, address decisionMaker) public {
        dm = DecisionMaker(decisionMaker);
        dm.someLogicToDecidePayment(receiver, address(this));
    }


} 

contract Receiver {
    uint public balance = 0;

    constructor() public payable{}

    function () payable {
      balance += msg.value;
    }
}

When trying to run placeBuy from the Sender Contract. This will create an instance of DecisionMaker so the function can run someLogicToDecidePayment(). Following some logic to decide who to pay i would like for the contract to invoke send_transfer from the Senders Contract to send some ether to Receiver contract. However this fails, with the following error:

VM error: revert.
revert  The transaction has been reverted to the initial state.
Note: The constructor should be payable if you send value.  Debug the transaction to get more information. 

If I am to use the function send within the Sender Contract instead of send_transfer, the transaction goes through, but the balance of Receiver is not increased. It seems that I am not able to send value with a transaction when using contracts. Could anyone help me understand why I can't successfully transfer ether from one contract to another with either of those methods?

Thank you in advance.

EDIT

I have added a payable constructor to both the sender and receiver as suggested by Lượng. Unfortunately, I was still unable to send ether between the contracts using the transfer function - send_transfer() - however was successful to using the send() method. I have attached my test script for anyone that would like to test this out in the future:

const assert = require('assert');
const ganache = require('ganache-cli');
const Web3 = require('web3');
const web3 = new Web3(ganache.provider());

const compiledDecision = require('../ethereum/build/DecisionMaker.json');
const compiledReceiver = require('../ethereum/build/Receiver.json');
const compiledSender = require('../ethereum/build/Sender.json');

let accounts;
let decision;
let sender;
let receiver;
let confirmation;
let post_balance;


beforeEach(async () => {
    accounts = await web3.eth.getAccounts();

    decision = await new web3.eth.Contract(JSON.parse(compiledDecision.interface))
        .deploy({ data: compiledDecision.bytecode })
        .send({ from: accounts[0], gas: '1999999' });

    receiver = await new web3.eth.Contract(JSON.parse(compiledReceiver.interface))
        .deploy({ data: compiledReceiver.bytecode })
        .send({ from: accounts[0], gas: '1999999' });

    sender = await new web3.eth.Contract(JSON.parse(compiledSender.interface))
        .deploy({ data: compiledSender.bytecode })
        .send({ from: accounts[0], gas: '1999999' });

});

describe('Testing Contracts', () => {
    it('Attempts to make a transaction from sender to receiver through DecisionMaker', async () =>{
        let pre_balance = await receiver.methods.balance().call();

        await sender.methods.deposit().send({
            from: accounts[0],
            gas: 1000000,
            value: 1000000
        });

        let senderBalance = await web3.eth.getBalance(sender.options.address);
        console.log('senders contract balance', senderBalance);
        console.log('pre balance of receiver (variable)', pre_balance);

        try{
        let confirmation= await sender.methods.placeBuy(receiver.options.address, decision.options.address).send({
            from: accounts[0],
            gas: 1999999
        })
        post_balance = await receiver.methods.balance().call();
        }catch(err){
        console.log(err);
        }

    let balanceOfAddress = await web3.eth.getBalance(receiver.options.address);
    senderBalance = await web3.eth.getBalance(sender.options.address);
    console.log('confirmation of transaction', confirmation);
    console.log('post balance of receiver (variable)', post_balance);
    console.log('balance of receiver contract address', balanceOfAddress);
    console.log('senders balance post transaction', senderBalance);

    console.log('finished test');
    });
});

I am still curious and would appreciate if anyone could give a suggestion for why the send_transfer() function doesn't work but the send() does :)

Answer 1

The constructor should be payable if you send value.

Create payable constructor if your smart contract use payable function. In this case, your contract Sender has not created the payable constructor

Answer 2

A couple of observations.

I don't think it's good form to rely on the receiver having a custom fallback function unless you're sure that all receivers will be contracts that you control. It's not a realistic expectations for the other participants.

Another issue that you are running into is the 2,300 gas stipend linked to the transfer and send methods. These are to prevent re-entrance attack by starving the attacker. 2,300 is enough accept the funds and emit and event but that's about it.

It's not enough to write to storage (5,000 to overwrite non-zero and 20,000 for a new slot). Consequently, the balances[msg.sender] += ... causes an out of gas situation.

I see two ways to handle this and you can decided which way feels right.

1. The Escrow method: Sender forwards the funds to Decider who either spends them or returns them.
2. The Approval method: Sender gets a signal from the Decider and then proceeds to spend, if approved.

Either way can be coded so it works without exception.

The first way requires a funky call syntax to forward some eth to Decider, and all of the gas. Call contract and send value from Solidity

Decider then goes:

// decide what to do
if(isGo) {
  receiver.transfer(msg.value);
} else :
  msg.sender.transfer(msg.value);
}

The other way: Spender seeks approval with:

require(dm.approveThis(args, ...));
// you don't get here unless approveThis was true
receiver.transfer(amount);

Hope it helps.