Why is solidity throwing “Note: The constructor should be payable if you send value. ”, when clearly the issue is elsewhere?

Question

I'm writing a contract on Remix that is able to deploy and track child contracts.

Here are the 2 contracts:

pragma solidity ^0.5.0;

Contract Factory

contract ContractFactory{
    ChildContract[] public createdContracts;
    ChildContract public activeChild;

    function createChild() public{
        ChildContract newChild = new ChildContract();
        createdContracts.push(newChild);
        // if (address(activeChild) != address(0)){
            activeChild.activate(false);
        // }
        activeChild = newChild;
    }
}

Child Contract

contract ChildContract{
    bool isActive;

    constructor() public{
        isActive = true;
    }

    function activate(bool status) public{
        isActive = status;
    }
}

I deployed the ContractFactory and it was successful. Then I tried calling the createChild function. Then I got the following error:

transact to ContractFactory.createChild errored: 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. 

As evident from the code, if I un-comment the commented out if condition inside the createChild function, the function will run without issues. My question is - Why is solidity throwing this misleading error, as the issue was with an unset variable?

Answer 1

Solidity throwing implies that it is a compilation error, which is obviously not the case here.

The exception is thrown during runtime, by the web3 client (Remix in your case) which submits the transaction.

It is thrown because you've masked the if (address(activeChild) != address(0)) statement, hence the attempt to call activeChild.activate (when activeChild points to address 0) reverts the transaction.

This information is explicitly provided to you by Remix:

transact to ContractFactory.createChild errored: VM error: revert.
revert  The transaction has been reverted to the initial state.

The note about the constructor... well, it is just a note to keep you aware of the fact noted...

Answer 2

Commenting out the check is the cause of the failure.

activeChild doesn't become a valid instance until later.

activeChild = newChild;

But this line seems to be trying to deactivate a predecessor. How can that work if there is no predecessor?

activeChild.activate(false)

On the first run, there will be no previous activeChild, so it is important to check or it will surely run into trouble.

I would probably be more explicit about the comparison.

if(address(activeChild) != address(0)) { // carry on and deactivate it

As other commentators have mentioned, the possible cause of the runtime error is merely a guess and it is often misleading.

Hope it helps.