BasicToken Code Style Findings

BTN-01C: Inaccurate Evaluation

Description:

The second require check of the transfer function is meant to ensure the user has a sufficient balance to transfer funds with, however, it inaccurately and redundantly does so.

Example:

Copy
30require(_to != address(0),"invalid address");
31require(_value <= balanceOf(msg.sender));
32require(transferAllowance,"Not allowed to transfer");
33
34// SafeMath.sub will throw if there is not enough balance.
35uint256 _value1 = (_value.mul(1 ether)).div(demandFactor);
36balances[msg.sender] = balances[msg.sender].sub(_value1);

Recommendation:

Firstly, the multiplication and division of L35 can truncate, meaning that even if _value <= balanceOf(msg.sender) is true, _value.mul(1 ether).div(demandFactor) > _balances[msg.sender] can also be causing the check to be futile as it would incorrectly validate a transfer. Lastly, the SafeMath invocation of sub on the balances mappping directly would throw if the user had insufficient balance and as such the check in its entirety is unnecessary. We advise it to be safely removed from the codebase.The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

BTN-02C: Nonstandard Naming Convention

Description:

The _totalSupply variable of the contract is declared as public generating a getter function for it along with the totalSupply function which is also public.

Example:

Copy
13uint256 public _totalSupply;
14uint256 public demandFactor;
15bool public transferAllowance;
16
17event Transfer(address indexed from, address indexed to, uint256 value);
18 
19function totalSupply() public view returns (uint256) {
20    return (_totalSupply.mul(demandFactor)).div(1 ether);
21}

Recommendation:

We advise the former to be set as internal or private given that _ prefixed variables are meant to be available solely internally and to avoid confusion between the _totalSupply and totalSupply functions.The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

BTN-03C: Redundant SafeMath Invocation

Description:

The divisions with the unit literal 1 ether redundantly utilize the div operation of SafeMath.

Example:

Copy
49return (balances[_owner].mul(demandFactor)).div(1 ether);

Recommendation:

We advise the operation to be safely replaced by its literal operation (/) given that the div function internally evaluates the divisor to be non-zero, a trait guaranteed by the utilization of the 1 ether literal.The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

BTN-04C: Visibility Specifiers Missing

Description:

The balances mapping has no explicitly set visibility specifier.

Example:

Copy
11mapping(address => uint256) balances;

Recommendation:

We advise one to be set so to avoid compilation discrepancies as in its current state a visibility specifier is assigned automnatically by the compiler which can change in a future version.The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.