[CHORE] Smart Contract Improvements

[tim-schultz]

Testing

• [x] Make unit tests as easy to understand as possible. Remove duplicate mocks and comments where necessary
• [x] Ensure final contracts have 100% test coverage. As of 5/18 we are at 91.3%
• [ ] Write fuzz tests? - It looks like https://github.com/crytic/echidna is a popular choice

Both Contracts

• [x] Lock the solidity version

GitcoinAttester

• [ ] ~remove setEASAddress and set address in constructor~
• [x] eas address is redundant if we have EAS contract we can get address

GitcoinVerifier

• [x] revert within the _verify function. With this method you could remove the if statement all together and revert for the independent reasons(nonce/signature)
• [x] remove _hashArray
• [x] DOMAIN_SEPARATOR (contracts/GitcoinVerifier.sol#22) should be immutable
• [x] attester (contracts/GitcoinVerifier.sol#16) should be immutable
• [x] issuer (contracts/GitcoinVerifier.sol#17) should be immutable
• [x] bytes32 private constant EIP712DOMAIN_TYPEHASH, bytes32 private constant STAMP_TYPEHASH, bytes32 private constant PASSPORT_TYPEHASH can be prehashed and the values can be assigned to the bytes32 value - will minimize gas in deployment
• [x] Add an onlyOwner withdraw function to contract. Funds would currently be locked forever. This would also require the contract to be updated to ownable. Another option could be an allowed address set in the constructor/deployment
• [x] Convert loops to unchecked loop to reduce gas usage https://hackmd.io/@totomanov/gas-optimization-loops#2-Increment-the-variable-in-an-unchecked-block

General

• [x] remove console.logs
• [x] Adjust hardhat compiler version to match solidity version that we lock to
• [x] Move to yarn? We use this for all other project
• [x] Ensure all contracts are fully commented
• [x] Improve error handling - revert should be specific to case that causes revert
• [ ] Deployment Checklist(set env variables, contract owners, verifiers, etc..)

[tim-schultz]

It would be great to be able to run tests against any environment. Currently they will only pass you have your rpc set to point at Sepolia.

This can be changed by deploying a mock version of EAS instead of referencing the version that is deployed on Sepolia https://github.com/gitcoinco/eas-proxy/blob/7f6700b981b8ae77a84a77f51d1a80dc8b5d051e/test/GitcoinAttester.ts#L89

[dmrazzy]

Change