CORIONplatform / solidity

GNU General Public License v3.0
12 stars 9 forks source link

moduleHandler: dropModule vs disableModule vs replaceModule #162

Closed gundas closed 6 years ago

gundas commented 6 years ago

For some reason the dropModule(..) function in moduleHandler checks the votes for replaceModule action to disable the module:

   function dropModule(string name, bool callCallback) external returns (bool success) {
        /*
            Deleting module from the database. Can be called only by the Publisher contract or while debug mode by owners.

            @name           Name of module to delete.
            @callCallback   Call the replaceable module to confirm replacement or not.

            @success        Was the Function successful?
        */
        var (_success, _found, _id) = getModuleIDByAddress(msg.sender);
        require( _success );
        if ( ! ( _found && modules[_id].name == sha3('Publisher') )) {
            require( block.number < debugModeUntil );
            if ( ! insertAndCheckDo(calcDoHash("replaceModule", sha3(name, callCallback))) ) {
                return true;
            }
        }

Also, the dropModule(..) function name is not consistent with a similar function called callDisableCallback(...) - i think the dropModule(..) function should be renamed to disableModule(...).

iFA88 commented 6 years ago

Yes, you have right. There is a typo in the calcDoHash parameter.

dropModule would delete the module from the moduleHandler database. This is very important, it is not enough disable it.