Increase coverage #342
Increase coverage #342
Conversation
contracts/SecurityTokenRegistry.sol
Outdated
| @@ -669,6 +669,7 @@ contract SecurityTokenRegistry is ISecurityTokenRegistry, EternalStorage { | |||
| * @param _patch Patch version of the proxy | |||
| */ | |||
| function setProtocolVersion(address _STFactoryAddress, uint8 _major, uint8 _minor, uint8 _patch) external onlyOwner { | |||
| require(_STFactoryAddress != address(0)); | |||
| function withdrawERC20(address _tokenContract, uint256 _value) external onlyOwner { | ||
| require(_tokenContract != address(0)); | ||
| IERC20 token = IERC20(_tokenContract); | ||
| require(token.transfer(owner, _value)); |
There was a problem hiding this comment.
It is intentional to reduce the size of the contract
There was a problem hiding this comment.
I think we should add at least 1 character reason so that we can know if our require statement is reverting or if it's something generic.
I'll have a deep look at the security token contract and to see if I can save some bits at some other places.
test/k_module_registry.js
Outdated
| describe("Test the initialize the function", async () => { | ||
| it("Should successfully update the implementation address -- fail because polymathRegistry address is 0x", async () => { | ||
| let bytesProxy = encodeProxyCall(MRProxyParameters, [ | ||
| "0x0000000000000000000000000000000000000000", |
There was a problem hiding this comment.
For cleaner code, maybe let's define const nullAddress = "0x0000000000000000000000000000000000000000" and use it everywhere. It is used a lot in all the test cases.
contracts/mocks/MockBurnFactory.sol
Outdated
| * @title Mock Contract Not fit for production environment | ||
| */ | ||
|
|
||
| contract MockBurnFactory is ModuleFactory { |
There was a problem hiding this comment.
I think we should inherit the main contract in the mock contract and overwrite the functions we need to. This will help the reader understand what exactly we are mocking and why.
contract MockBurnFactory is ModuleFactory
can be
contract MockBurnFactory is BurnFactory
Same goes for all mock contracts.
Fixes during increasing the tests
withdrawPolyfunction withwithdrawERC20.