chore: first pass

docs/00-onchain-data.md

@@ -14,7 +14,7 @@ How does this work?
 2. Once the schema was created, you can write data to it by calling one of the
    EAS smart contracts functions, for example
    `attest(AttestationRequest calldata request)`
-   (see [IEAS.sol](https://github.com/ethereum-attestation-service/eas-contracts/blob/master/contracts/IEAS.sol#L148-L169))
+   (see [IEAS.sol](https://github.com/ethereum-attestation-service/eas-contracts/blob/c4c63775094e9104d0b4335ac8428f8d10f6d589/contracts/IEAS.sol#L116-L132))
 3. The following data will be registered in the attestation:
    1. the attester (this will be the`msg.sender`)
    2. the recipient (an ETH address)
@@ -177,10 +177,9 @@ mapping(address => mapping(bytes32 => bytes32)) public userAttestations;
 ```
 
 In order to ensure the integrity of the data that a resolver stores, resolver
-smart contract shall only validate and store date from trusted sources:
-
-- a trusted EAS contract
-- a trusted Attester
+smart contract shall 
+- only allow calls from a trusted EAS smart contract
+- only accept data coming from a trusted attester
 
 
 ## GitcoinPassportDecoder

docs/01-onchain-passport-attestation.md

@@ -61,20 +61,20 @@ new SchemaEncoder(
   hashes field, this is an ordered array, containing th expiration date for each
   stamp.
 
-Considering the list of providers above if a user has the `BrightId`,
-`CommunityStakingSilver` and `Discord` stamps, their attestation will look like:
-
 #### providerMapVersion
 
 - this field will be used to indicate the version of the provider map used to
   encode the `providers` field. This will allow us to version the provider map
   if there are a large amount of stamps that need to be replaced/removed in the
   future. If new providers are only added, the providerMapVersion does not change.
 
+Considering the list of providers above if a user has the `BrightId`,
+`CommunityStakingSilver` and `Discord` stamps, their attestation will look like:
+
 ```json
 {
-  "providers": ["12983612785124"],
-  "hashes": ["0x0000000000000001", "0x0000000000000002", "0x0000000000000003"],
+  "providers": ["0x0000000000000029"], // 01 + 08 + 20
+  "hashes": ["0x0000000000000001", "0x0000000000000008", "0x0000000000000020"], // [BrightId, CommunityStakingSilver, Discord]
   "issuanceDates": ["123456789", "123456789", "123456789"],
   "expirationDates": ["123456789", "123456789", "123456789"]
 }

reivew.md

@@ -0,0 +1,21 @@
+# GitcoinVerifier
+
+**Questions**
+- `L282`: Thoughts on just withdrawing contract balance as opposed to specifying an amount ?
+
+**Low**
+- `L6` : Recommend using `@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol`
+- `L43-L80`: Recommend keeping constants with storage variables
+- `L94`: Recommend adding natspec for `__GitcoinVerifier_initno no`
+- `L34`: can be made constant with value `GitcoinVerifier`. Remove `L100` during initialize
+- `L235`: Consider addressing the TODO via loop. It does increase gas cost but likely worth it unless passport team is confident that the IAM will work as expected
+- `L202`: Recommend storing the length in variable as opposed to computing it at every iteration. Gas Optimisation 
+```javascript
+uint256 attestationsLength = attestationRequest.multiAttestationRequest.length;
+bytes32[] memory multiAttestHashes = new bytes32[](attestationsLength);
+// avoids computing attestationsLength on every iteration 
+for (uint i = 0; i < attestationsLength; ) {
+    ...
+}
+```
+