Rigor Protocol contest - CodingNameKiki's results

Community lending and instant payments for new home construction.

General Information

Platform: Code4rena

Start Date: 01/08/2022

Pot Size: $50,000 USDC

Total HM: 26

Participants: 133

Period: 5 days

Judge: Jack the Pug

Total Solo HM: 6

Id: 151

League: ETH

Rigor Protocol

Findings Distribution

Researcher Performance

Rank: 79/133

Findings: 2

Award: $62.35

🌟 Selected for report: 0

🚀 Solo Findings: 0

  1. Override function that are unused should have the variable name removed or commented out to avoid compiler warnings

Community.sol 122: function createCommunity(bytes calldata _hash, address _currency) 169: function addMember(bytes calldata _data, bytes calldata _signature) 297: function payPublishFee(uint256 _communityID, address _project) 555: function restrictToAdmin() external override onlyHomeFiAdmin { 566: function unrestrictToAdmin() external override onlyHomeFiAdmin {

  1. Public functions not called by the contract should be declared external instead

HomeFi.sol 264: function isTrustedForwarder(address _forwarder)

DebtToken.sol 91: function transferFrom( 100: function transfer(

  1. Constants should be defined rather than using magic numbers

Project.sol 576: uint256 _maxLoop = 50;

  1. Adding a return statement when the function defines a named return variable, is redundant

HomeFi.sol 296: return projectCount;

DebtToken.sol 83: return _decimals;

  1. Multiple address mapping can be combined into a single mapping of an address to a struct, where appropriate

Community.sol 59: mapping(address => uint256) public override projectPublished; 61: mapping(address => mapping(bytes32 => bool)) public override approvedHashes;

HomeFi.sol 64: mapping(address => uint256) public override projectTokenId; 66: mapping(address => address) public override wrappedToken;

#0 - zgorizzo69

2022-08-08T15:54:50Z

thanks for your work

  1. running yarn compile there is no warnings
  2. public is required for isTrustedForwarder
  3. for wrappedToken address are not the same one is a project address and the other a currency address don't reflect the

  1. Replacing > 0 with != 0 in (if statement, require and others..) saves gas

contracts/Community.sol 261: if (projectPublished[_project] > 0) { 426: if ( _communities[_communityID].projectDetails[_project].lentAmount > 0) 764: require(_repayAmount > 0, "Community::!repay"); 840: if (_interestEarned > 0) {

contracts/Project.sol 195: require(_cost > 0, "Project::!value>0"); 380: if (_leftOutTokens > 0) { 601: if (_changeOrderedTask.length > 0) { 691: if (_loopCount > 0) emit TaskAllocated(_tasksAllocated);

contracts/HomeFi.sol 245: return projectTokenId[_project] > 0;

contracts/Disputes.sol 106: require(_actionType > 0 && _actionType <= uint8(ActionType.TaskPay), "Disputes::!ActionType");

  1. Unchecked {++i} instead of i++ and removing the applied zero to i (for some examples below), since the default value is zero anyways. These two things save a lot of gas.

Recommended: Use ++i instead of i++. This is especially useful in for loops but this optimization can be used anywhere in your code. You can also use unchecked{++i;} for even more gas savings but this will not check to see if i overflows. For extra safety if you are worried about this, you can add a require statement after the loop checking if i is equal to the final incremented value. (But since it is for loop, the i can't overflow).

contracts/Community.sol (Before) 624: for (uint256 i = 0; i < _communities[_communityID].memberCount; i++) {

(After) 624: for (uint256 i; i < _communities[_communityID].memberCount;) {
unchecked{ ++i; }

contracts/Project.sol (Before) 248: for (uint256 i = 0; i < _length; i++) {

(After) 248: for (uint256 i; i < _length;) {
unchecked{ ++i; }

contracts/Project.sol (Before) 311: for (uint256 i = 0; i < _length; i++) {

(After) 311: for (uint256 i; i < _length;) {
unchecked{ ++i; }

contracts/Project.sol (Before) 322: for (uint256 i = 0; i < _length; i++) {

(After) 322: for (uint256 i; i < _length;) {
unchecked{ ++i; }

contracts/Project.sol (Before) 368: for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {

(After) 368: for (uint256 _taskID = 1; _taskID <= _length;) {
unchecked{ ++_taskID; }

contracts/Project.sol (Before) 603: for (; i < _changeOrderedTask.length; i++) {

(After) 603: for (; i < _changeOrderedTask.length;) {
unchecked{ ++i; }

contracts/Project.sol (Before) 650: for (++j; j <= taskCount; j++) {

(After) 650: for (++j; j <= taskCount;) {
unchecked{ ++j; }

contracts/Project.sol (Before) 710: for (uint256 _taskID = 1; _taskID <= _length; _taskID++) {

(After) 710: for (uint256 _taskID = 1; _taskID <= _length;) {
unchecked{ ++_taskID; }

contracts/HomeFiProxy.sol (Before) 87: for (uint256 i = 0; i < _length; i++) {

(After) 87: for (uint256 i; i < _length;) {
unchecked{ ++i; }

contracts/HomeFiProxy.sol (Before) 136: for (uint256 i = 0; i < _length; i++) {

(After) 136: for (uint256 i; i < _length;) {
unchecked{ ++i; }

  1. Using bools for storage incurs overhead

contracts/Community.sol 55. bool public override restrictedToAdmin; 61: mapping(address => mapping(bytes32 => bool)) public override approvedHashes;

contracts/Project.sol 68: bool public override contractorConfirmed; 78: bool public override contractorDelegated; 84: mapping(address => mapping(bytes32 => bool)) public override approvedHashes;

contracts/HomeFi.sol 50: bool public override addrSet;

contracts/HomeFiProxy.sol 30: mapping(address => bool) internal contractsActive;

  1. x += y costs more gas than x = x + y for state variables

contracts/Community.sol 423: .totalLent += _amountToProject; 435: .lentAmount += _lendingAmount;

  1. Empty blocks should be removed or emit something

contracts/Project.sol 88: constructor() initializer {}

  1. Internal functions only called ones can be inlined to save gas

contracts/Project.sol 770: function autoWithdraw(uint256 _amount) internal {

contracts/HomeFi.sol 284: function mintNFT(address _to, string memory _tokenURI) 314: function _msgData()

contracts/Disputes.sol 207: function resolveHandler(uint256 _disputeID) internal { 236: function executeTaskAdd(address _project, bytes memory _actionData) 253: function executeTaskChange(address _project, bytes memory _actionData) 268: function executeTaskPay(address _project, bytes memory _actionData)

  1. Functions guaranted to revert when called by normal users can be marked payable

contracts/HomeFi.sol 157: function replaceAdmin(address _newAdmin) 171: function replaceTreasury(address _newTreasury) 185: function replaceLenderFee(uint256 _newLenderFee) 200: function setTrustedForwarder(address _newForwarder)

contracts/Disputes.sol 141: function resolveDispute(

contracts/HomeFiProxy.sol 100: function addNewContract(bytes2 _contractName, address _contractAddress) 125: function upgradeMultipleImplementations( 150: function changeProxyAdminOwner(address _newAdmin)

  1. Changing require statements with revert in modifier function saves gas.

contracts/HomeFiProxy.sol (Before) 41: require(_address != address(0), "Proxy::0 address");

(After) 41: if (_address == address(0)) { revert("Proxy::0 address"); }

#0 - zgorizzo69

2022-08-08T15:46:17Z

the comment could have used markdown features a bit more :stuck_out_tongue_closed_eyes: 5. we need this to initialize

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter