Merge branch 'audit'

audit-files/commit-ba4b2663761d1809a0e16b5b716ce376aabc531c/aderyn-report-5-9-2024-7:56pm.md

@@ -0,0 +1,79 @@
+# Aderyn Analysis Report
+
+This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a static analysis tool built by [Cyfrin](https://cyfrin.io), a blockchain security company. This report is not a substitute for manual audit or security review. It should not be relied upon for any purpose other than to assist in the identification of potential security vulnerabilities.
+# Table of Contents
+
+- [Summary](#summary)
+  - [Files Summary](#files-summary)
+  - [Files Details](#files-details)
+  - [Issue Summary](#issue-summary)
+- [Low Issues](#low-issues)
+  - [L-1: Solidity pragma should be specific, not wide](#l-1-solidity-pragma-should-be-specific-not-wide)
+  - [L-2: `public` functions not used internally could be marked `external`](#l-2-public-functions-not-used-internally-could-be-marked-external)
+  - [L-3: PUSH0 is not supported by all chains](#l-3-push0-is-not-supported-by-all-chains)
+
+
+# Summary
+
+## Files Summary
+
+| Key | Value |
+| --- | --- |
+| .sol Files | 1 |
+| Total nSLOC | 37 |
+
+
+## Files Details
+
+| Filepath | nSLOC |
+| --- | --- |
+| contracts/GitcoinPassportEligibility.sol | 37 |
+| **Total** | **37** |
+
+
+## Issue Summary
+
+| Category | No. of Issues |
+| --- | --- |
+| High | 0 |
+| Low | 3 |
+
+
+# Low Issues
+
+## L-1: Solidity pragma should be specific, not wide
+
+Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of `pragma solidity ^0.8.0;`, use `pragma solidity 0.8.0;`
+
+- Found in contracts/GitcoinPassportEligibility.sol [Line: 2](contracts/GitcoinPassportEligibility.sol#L2)
+
+	```solidity
+	pragma solidity ^0.8.19;
+	```
+
+
+
+## L-2: `public` functions not used internally could be marked `external`
+
+Instead of marking a function as `public`, consider marking it as `external` if it is not used internally.
+
+- Found in contracts/GitcoinPassportEligibility.sol [Line: 63](contracts/GitcoinPassportEligibility.sol#L63)
+
+	```solidity
+	  function getWearerStatus(address _wearer, uint256 /*_hatId*/ )
+	```
+
+
+
+## L-3: PUSH0 is not supported by all chains
+
+Solc compiler version 0.8.20 switches the default target EVM version to Shanghai, which means that the generated bytecode will include PUSH0 opcodes. Be sure to select the appropriate EVM version in case you intend to deploy on a chain other than mainnet like L2 chains that may not support PUSH0, otherwise deployment of your contracts will fail.
+
+- Found in contracts/GitcoinPassportEligibility.sol [Line: 2](contracts/GitcoinPassportEligibility.sol#L2)
+
+	```solidity
+	pragma solidity ^0.8.19;
+	```
+
+
+

audit-files/commit-ba4b2663761d1809a0e16b5b716ce376aabc531c/findings/finding-1.md

@@ -0,0 +1,9 @@
+### [I-1] GitcoinPassportEligibility::GITCOIN_PASSPORT_DECODER function does not follow the mixedCase naming convention, resulting in potential confusion from code reviewers
+
+**Description:** All caps naming convention is reserved for constant variables. Although `GitcoinPassportEligibility::GITCOIN_PASSPORT_DECODER` returns an immutable constant value, it is still a function. Thus it should follow the mixedCase naming convention.
+
+**Impact:** Reduces the understanding and potential interactibility of the protocol, and muddies up automated tool's results..
+
+**Proof of Concept:** Patrick Collins, a leader security smart contract auditor and educator follows the mixedCase naming convention. Alongside automated tools like Slither and Aderyn to report instances of functions not being correctly in mixedCase. Newcomers and the majority of developers, auditors, and researchers will follow these conventions. Alongside muddying up the information that is returned from the automated tools.
+
+**Recommended Mitigation:** Rename `GitcoinPassportEligibility::GITCOIN_PASSPORT_DECODER` to `GitcoinPassportEligibility::gitcoinPassportDecoder` to satisfy the requirement of functions being in mixedCase.

audit-files/commit-ba4b2663761d1809a0e16b5b716ce376aabc531c/findings/finding-2.md

@@ -0,0 +1,9 @@
+### [I-2] GitcoinPassportEligibility::SCORE_CRITERION function does not follow the mixedCase naming convention, resulting in potential confusion from code reviewers
+
+**Description:** All caps naming convention is reserved for constant variables. Although `GitcoinPassportEligibility::SCORE_CRITERION` returns an immutable constant value, it is still a function. Thus it should follow the mixedCase naming convention.
+
+**Impact:** Reduces the understanding and potential interactibility of the protocol, and muddies up automated tool's results..
+
+**Proof of Concept:** Patrick Collins, a leader security smart contract auditor and educator follows the mixedCase naming convention. Alongside automated tools like Slither and Aderyn to report instances of functions not being correctly in mixedCase. Newcomers and the majority of developers, auditors, and researchers will follow these conventions. Alongside muddying up the information that is returned from the automated tools.
+
+**Recommended Mitigation:** Rename `GitcoinPassportEligibility::SCORE_CRITERION` to `GitcoinPassportEligibility::scoreCriterion` to satisfy the requirement of functions being in mixedCase.

audit-files/commit-ba4b2663761d1809a0e16b5b716ce376aabc531c/findings/finding-3.md

@@ -0,0 +1,9 @@
+### [I-3] GitcoinPassportEligibility::getWearerStatus' first parameter, _wearer, does not follow the mixedCase naming convention, resulting in potential confusion from code reviewers
+
+**Description:** The underscore naming convention is an outdated practice for function parameters. 
+
+**Impact:** Reduces the understanding and potential interactibility of the protocol, and muddies up automated tool's results.
+
+**Proof of Concept:** Patrick Collins, a leader security smart contract auditor and educator follows the mixedCase naming convention. Alongside automated tools like Slither and Aderyn to report instances of functions not being correctly in mixedCase. Newcomers and the majority of developers, auditors, and researchers will follow these conventions. Alongside muddying up the information that is returned from the automated tools.
+
+**Recommended Mitigation:** Rename `GitcoinPassportEligibility::getWearerStatus`' first parameter, `_wearer`, to `wearer` to satisfy the requirement of functions being in mixedCase.

audit-files/commit-ba4b2663761d1809a0e16b5b716ce376aabc531c/findings/finding-4.md

@@ -0,0 +1,13 @@
+### [G-1] `GitcoinPassportEligibility::getWearerStatus` does not have the most efficient visibility type.
+
+**Description:** `GitcoinPassportEligibility::getWearerStatus` is not called within `GitcoinPassportEligibility`, however its visibility is `public`.
+
+**Impact:** Increases the gas cost of calling the function.
+
+**Proof of Concept:** We can see that through fuzz testing public and external functions with the same parameters and operations, the external function resulted in costing less gas to call.
+
+`test_externalFunction(uint256[20]) (runs: 257, μ: 255839, ~: 255839)`
+
+`test_publicFunction(uint256[20]) (runs: 257, μ: 257286, ~: 257286)`
+
+**Recommended Mitigation:** Change `GitcoinPassportEligibility::getWearerStatus`'s visibility from `public` to `external`.