diff --git a/docs/00-onchain-data.md b/docs/00-onchain-data.md index 118df75..5f61505 100644 --- a/docs/00-onchain-data.md +++ b/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 diff --git a/docs/01-onchain-passport-attestation.md b/docs/01-onchain-passport-attestation.md index 4f59f99..9e053ac 100644 --- a/docs/01-onchain-passport-attestation.md +++ b/docs/01-onchain-passport-attestation.md @@ -61,9 +61,6 @@ 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 @@ -71,10 +68,13 @@ Considering the list of providers above if a user has the `BrightId`, 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"] } diff --git a/reivew.md b/reivew.md new file mode 100644 index 0000000..242481b --- /dev/null +++ b/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; ) { + ... +} +``` +