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

Uses of address types can be more specific #290

Open
fulldecent opened this issue Nov 18, 2019 · 0 comments
Open

Uses of address types can be more specific #290

fulldecent opened this issue Nov 18, 2019 · 0 comments

Comments

@fulldecent
Copy link
Contributor

In the implementation, many uses of the type address could be made more specific.

Example:

contract CountdownGriefing_Factory is Factory {
    constructor(address instanceRegistry, address templateContract) public {
        CountdownGriefing template;
        // set instance type
        bytes4 instanceType = bytes4(keccak256(bytes('Agreement')));
        // set initSelector
        bytes4 initSelector = template.initialize.selector;
        // initialize factory params
        Factory._initialize(instanceRegistry, templateContract, instanceType, initSelector);
    }
}

could become:

contract CountdownGriefing_Factory is Factory {
    constructor(iRegistry instanceRegistry, CountdownGriefing templateContract) public {
        // set instance type
        bytes4 instanceType = bytes4(keccak256(bytes('Agreement')));
        // set initSelector
        bytes4 initSelector = templateContract.initialize.selector;
        // initialize factory params
        Factory._initialize(instanceRegistry, templateContract, instanceType, initSelector);
    }
}

Type safety improves the ability of static analyzers to help you code, removes the necessity of type casting, and are a best practice.

Recommendation: review every parameter and variable of type address and use a more specific class if it represents a contract address.

Recommendation: update Factory to inherit from iFactory.

Recommendation: update iNMR to inherit from IERC20.

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

2 participants