diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index d8e0fec4d1d..c9571ddedbf 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -12,8 +12,6 @@ import "../common/interfaces/ICeloVersionedContract.sol"; import "../common/Initializable.sol"; import "../common/UsingRegistry.sol"; import "../common/Signatures.sol"; -import "../common/UsingPrecompiles.sol"; -import "../common/libraries/ReentrancyGuard.sol"; /** * @title Contract mapping identifiers to accounts @@ -23,9 +21,7 @@ contract FederatedAttestations is ICeloVersionedContract, Ownable, Initializable, - UsingRegistry, - ReentrancyGuard, - UsingPrecompiles + UsingRegistry { using SafeMath for uint256; using SafeCast for uint256; @@ -39,7 +35,7 @@ contract FederatedAttestations is // TODO ASv2 revisit linting issues & all solhint-disable-next-line max-line-length // identifier -> issuer -> attestations - mapping(bytes32 => mapping(address => OwnershipAttestation[])) public identifierToAddresses; + mapping(bytes32 => mapping(address => OwnershipAttestation[])) public identifierToAttestations; // account -> issuer -> identifiers mapping(address => mapping(address => bytes32[])) public addressToIdentifiers; // signer => isRevoked @@ -78,7 +74,6 @@ contract FederatedAttestations is _transferOwnership(msg.sender); setRegistry(registryAddress); setEip712DomainSeparator(); - // TODO ASv2 initialize any other variables here } /** @@ -113,12 +108,70 @@ contract FederatedAttestations is } /** - * @notice Returns info about attestations (with unrevoked signers) - * for an identifier produced by a list of issuers + * @notice Helper function for _lookupAttestations to calculate the + total number of attestations completed for an identifier + by each trusted issuer, from unrevoked signers only + * @param identifier Hash of the identifier + * @param trustedIssuers Array of n issuers whose attestations will be included + * @return [0] Sum total of attestations found + * [1] Array of number of attestations found per issuer + */ + function getNumUnrevokedAttestations(bytes32 identifier, address[] memory trustedIssuers) + internal + view + returns (uint256, uint256[] memory) + { + uint256 totalAttestations = 0; + uint256[] memory countsPerIssuer = new uint256[](trustedIssuers.length); + + for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { + // solhint-disable-next-line max-line-length + OwnershipAttestation[] storage attestationsPerIssuer = identifierToAttestations[identifier][trustedIssuers[i]]; + for (uint256 j = 0; j < attestationsPerIssuer.length; j = j.add(1)) { + if (revokedSigners[attestationsPerIssuer[j].signer]) { + continue; + } + totalAttestations = totalAttestations.add(1); + countsPerIssuer[i] = countsPerIssuer[i].add(1); + } + } + return (totalAttestations, countsPerIssuer); + } + + /** + * @notice Helper function for _lookupAttestations to calculate the + total number of attestations completed for an identifier + by each trusted issuer + * @param identifier Hash of the identifier + * @param trustedIssuers Array of n issuers whose attestations will be included + * @return [0] Sum total of attestations found + * [1] Array of number of attestations found per issuer + */ + function getNumAttestations(bytes32 identifier, address[] memory trustedIssuers) + internal + view + returns (uint256, uint256[] memory) + { + uint256 totalAttestations = 0; + uint256 numAttestationsForIssuer; + uint256[] memory countsPerIssuer = new uint256[](trustedIssuers.length); + + for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { + numAttestationsForIssuer = identifierToAttestations[identifier][trustedIssuers[i]].length; + totalAttestations = totalAttestations.add(numAttestationsForIssuer); + countsPerIssuer[i] = numAttestationsForIssuer; + } + return (totalAttestations, countsPerIssuer); + } + + /** + * @notice Returns info about up to `maxAttestations` attestations for + * `identifier` produced by unrevoked signers of `trustedIssuers` * @param identifier Hash of the identifier * @param trustedIssuers Array of n issuers whose attestations will be included * @param maxAttestations Limit the number of attestations that will be returned - * @return for m found attestations, m <= maxAttestations: + * @return [0] Array of number of attestations returned per issuer + * @return [1 - 3] for m (== sum([0])) found attestations, m <= maxAttestations: * [ * Array of m accounts, * Array of m issuedOns, @@ -127,115 +180,357 @@ contract FederatedAttestations is * @dev Adds attestation info to the arrays in order of provided trustedIssuers * @dev Expectation that only one attestation exists per (identifier, issuer, account) */ + // TODO reviewers: is it preferable to return an array of `trustedIssuer` indices + // (indicating issuer per attestation) instead of counts per attestation? + function lookupUnrevokedAttestations( + bytes32 identifier, + address[] calldata trustedIssuers, + uint256 maxAttestations + ) external view returns (uint256[] memory, address[] memory, uint256[] memory, address[] memory) { + // TODO reviewers: this is to get around a stack too deep error; + // are there better ways of dealing with this? + return _lookupUnrevokedAttestations(identifier, trustedIssuers, maxAttestations); + } - // TODO ASv2 consider also returning an array with counts of attestations per issuer - function lookupAttestations( + /** + * @notice Helper function for lookupUnrevokedAttestations to get around stack too deep + * @param identifier Hash of the identifier + * @param trustedIssuers Array of n issuers whose attestations will be included + * @param maxAttestations Limit the number of attestations that will be returned + * @return [0] Array of number of attestations returned per issuer + * @return [1 - 3] for m (== sum([0])) found attestations, m <= maxAttestations: + * [ + * Array of m accounts, + * Array of m issuedOns, + * Array of m signers + * ]; index corresponds to the same attestation + * @dev Adds attestation info to the arrays in order of provided trustedIssuers + * @dev Expectation that only one attestation exists per (identifier, issuer, account) + */ + function _lookupUnrevokedAttestations( bytes32 identifier, address[] memory trustedIssuers, uint256 maxAttestations - ) public view returns (address[] memory, uint256[] memory, address[] memory) { - // Cannot dynamically allocate an in-memory array - // For now require a max returned parameter to pre-allocate and then shrink - // TODO ASv2 is it a risk to allocate an array of size maxAttestations? + ) internal view returns (uint256[] memory, address[] memory, uint256[] memory, address[] memory) { + uint256[] memory countsPerIssuer = new uint256[](trustedIssuers.length); + + // Pre-computing length of unrevoked attestations requires many storage lookups. + // Allow users to call that first and pass this in as maxAttestations. // Same index corresponds to same attestation address[] memory accounts = new address[](maxAttestations); uint256[] memory issuedOns = new uint256[](maxAttestations); address[] memory signers = new address[](maxAttestations); uint256 currIndex = 0; + OwnershipAttestation[] memory attestationsPerIssuer; + + for (uint256 i = 0; i < trustedIssuers.length && currIndex < maxAttestations; i = i.add(1)) { + attestationsPerIssuer = identifierToAttestations[identifier][trustedIssuers[i]]; + for ( + uint256 j = 0; + j < attestationsPerIssuer.length && currIndex < maxAttestations; + j = j.add(1) + ) { + if (revokedSigners[attestationsPerIssuer[j].signer]) { + continue; + } + accounts[currIndex] = attestationsPerIssuer[j].account; + issuedOns[currIndex] = attestationsPerIssuer[j].issuedOn; + signers[currIndex] = attestationsPerIssuer[j].signer; + currIndex = currIndex.add(1); + countsPerIssuer[i] = countsPerIssuer[i].add(1); + } + } + + if (currIndex >= maxAttestations) { + return (countsPerIssuer, accounts, issuedOns, signers); + } + + // Trim returned structs if necessary + address[] memory trimmedAccounts = new address[](currIndex); + uint256[] memory trimmedIssuedOns = new uint256[](currIndex); + address[] memory trimmedSigners = new address[](currIndex); + + for (uint256 i = 0; i < currIndex; i = i.add(1)) { + trimmedAccounts[i] = accounts[i]; + trimmedIssuedOns[i] = issuedOns[i]; + trimmedSigners[i] = signers[i]; + } + return (countsPerIssuer, trimmedAccounts, trimmedIssuedOns, trimmedSigners); + } + + /** + * @notice Similar to lookupUnrevokedAttestations but returns all attestations + * for `identifier` produced by `trustedIssuers`, + * either including or excluding attestations from revoked signers + * @param identifier Hash of the identifier + * @param trustedIssuers Array of n issuers whose attestations will be included + * @param includeRevoked Whether to include attestations produced by revoked signers + * @return [0] Array of number of attestations returned per issuer + * @return [1 - 3] for m (== sum([0])) found attestations: + * [ + * Array of m accounts, + * Array of m issuedOns, + * Array of m signers + * ]; index corresponds to the same attestation + * @dev Adds attestation info to the arrays in order of provided trustedIssuers + * @dev Expectation that only one attestation exists per (identifier, issuer, account) + */ + function lookupAttestations( + bytes32 identifier, + address[] calldata trustedIssuers, + bool includeRevoked + ) external view returns (uint256[] memory, address[] memory, uint256[] memory, address[] memory) { + // TODO reviewers: this is to get around a stack too deep error; + // are there better ways of dealing with this? + return _lookupAttestations(identifier, trustedIssuers, includeRevoked); + } + + /** + * @notice Helper function for lookupAttestations to get around stack too deep + * @param identifier Hash of the identifier + * @param trustedIssuers Array of n issuers whose attestations will be included + * @param includeRevoked Whether to include attestations produced by revoked signers + * @return [0] Array of number of attestations returned per issuer + * @return [1 - 3] for m (== sum([0])) found attestations: + * [ + * Array of m accounts, + * Array of m issuedOns, + * Array of m signers + * ]; index corresponds to the same attestation + * @dev Adds attestation info to the arrays in order of provided trustedIssuers + * @dev Expectation that only one attestation exists per (identifier, issuer, account) + */ + function _lookupAttestations( + bytes32 identifier, + address[] memory trustedIssuers, + bool includeRevoked + ) internal view returns (uint256[] memory, address[] memory, uint256[] memory, address[] memory) { + uint256 totalAttestations; + uint256[] memory countsPerIssuer; + + (totalAttestations, countsPerIssuer) = includeRevoked + ? getNumAttestations(identifier, trustedIssuers) + : getNumUnrevokedAttestations(identifier, trustedIssuers); + + address[] memory accounts = new address[](totalAttestations); + uint256[] memory issuedOns = new uint256[](totalAttestations); + address[] memory signers = new address[](totalAttestations); + + OwnershipAttestation[] memory attestationsPerIssuer; + // Reset this and use as current index to get around stack-too-deep + // TODO reviewers: is it preferable to pack two uint256 counters into a struct + // and use one for total (above) & one for currIndex (below)? + totalAttestations = 0; for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { - uint256 numTrustedIssuers = identifierToAddresses[identifier][trustedIssuers[i]].length; - for (uint256 j = 0; j < numTrustedIssuers; j = j.add(1)) { - // Only create and push new attestation if we haven't hit max - if (currIndex < maxAttestations) { - // solhint-disable-next-line max-line-length - OwnershipAttestation memory attestation = identifierToAddresses[identifier][trustedIssuers[i]][j]; - if (!revokedSigners[attestation.signer]) { - accounts[currIndex] = attestation.account; - issuedOns[currIndex] = attestation.issuedOn; - signers[currIndex] = attestation.signer; - currIndex = currIndex.add(1); + attestationsPerIssuer = identifierToAttestations[identifier][trustedIssuers[i]]; + for (uint256 j = 0; j < attestationsPerIssuer.length; j = j.add(1)) { + if (!includeRevoked && revokedSigners[attestationsPerIssuer[j].signer]) { + continue; + } + accounts[totalAttestations] = attestationsPerIssuer[j].account; + issuedOns[totalAttestations] = attestationsPerIssuer[j].issuedOn; + signers[totalAttestations] = attestationsPerIssuer[j].signer; + totalAttestations = totalAttestations.add(1); + } + } + return (countsPerIssuer, accounts, issuedOns, signers); + } + + /** + * @notice Helper function for lookupIdentifiers to calculate the + total number of identifiers completed for an identifier + by each trusted issuer, from unrevoked signers only + * @param account Address of the account + * @param trustedIssuers Array of n issuers whose identifiers will be included + * @return [0] Sum total of identifiers found + * [1] Array of number of identifiers found per issuer + */ + function getNumUnrevokedIdentifiers(address account, address[] memory trustedIssuers) + internal + view + returns (uint256, uint256[] memory) + { + uint256 totalIdentifiers = 0; + uint256[] memory countsPerIssuer = new uint256[](trustedIssuers.length); + + OwnershipAttestation[] memory attestationsPerIssuer; + bytes32[] memory identifiersPerIssuer; + + for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { + identifiersPerIssuer = addressToIdentifiers[account][trustedIssuers[i]]; + for (uint256 j = 0; j < identifiersPerIssuer.length; j = j.add(1)) { + bytes32 identifier = identifiersPerIssuer[j]; + // Check if the mapping was produced by a revoked signer + attestationsPerIssuer = identifierToAttestations[identifier][trustedIssuers[i]]; + for (uint256 k = 0; k < attestationsPerIssuer.length; k = k.add(1)) { + OwnershipAttestation memory attestation = attestationsPerIssuer[k]; + // (identifier, account, issuer) tuples are checked for uniqueness on registration + if (!(attestation.account == account) || revokedSigners[attestation.signer]) { + continue; } - } else { + totalIdentifiers = totalIdentifiers.add(1); + countsPerIssuer[i] = countsPerIssuer[i].add(1); break; } } } + return (totalIdentifiers, countsPerIssuer); + } - // Trim returned structs if necessary - if (currIndex < maxAttestations) { - address[] memory trimmedAccounts = new address[](currIndex); - uint256[] memory trimmedIssuedOns = new uint256[](currIndex); - address[] memory trimmedSigners = new address[](currIndex); - - for (uint256 i = 0; i < currIndex; i = i.add(1)) { - trimmedAccounts[i] = accounts[i]; - trimmedIssuedOns[i] = issuedOns[i]; - trimmedSigners[i] = signers[i]; - } - return (trimmedAccounts, trimmedIssuedOns, trimmedSigners); - } else { - return (accounts, issuedOns, signers); + /** + * @notice Helper function for lookupIdentifiers to calculate the + total number of identifiers completed for an identifier + by each trusted issuer + * @param account Address of the account + * @param trustedIssuers Array of n issuers whose identifiers will be included + * @return [0] Sum total of identifiers found + * [1] Array of number of identifiers found per issuer + */ + function getNumIdentifiers(address account, address[] memory trustedIssuers) + internal + view + returns (uint256, uint256[] memory) + { + uint256 totalIdentifiers = 0; + uint256 numIdentifiersForIssuer; + uint256[] memory countsPerIssuer = new uint256[](trustedIssuers.length); + + for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { + numIdentifiersForIssuer = addressToIdentifiers[account][trustedIssuers[i]].length; + totalIdentifiers = totalIdentifiers.add(numIdentifiersForIssuer); + countsPerIssuer[i] = numIdentifiersForIssuer; } + return (totalIdentifiers, countsPerIssuer); } /** - * @notice Returns identifiers mapped (by unrevoked signers) to a - * given account address by a list of trusted issuers + * @notice Returns up to `maxIdentifiers` identifiers mapped to `account` + * by unrevoked signers of `trustedIssuers` * @param account Address of the account * @param trustedIssuers Array of n issuers whose identifier mappings will be used * @param maxIdentifiers Limit the number of identifiers that will be returned - * @return Array (length <= maxIdentifiers) of identifiers + * @return [0] Array of number of identifiers returned per issuer + * @return [1] Array (length == sum([0]) <= maxIdentifiers) of identifiers * @dev Adds identifier info to the arrays in order of provided trustedIssuers * @dev Expectation that only one attestation exists per (identifier, issuer, account) */ - - // TODO ASv2 consider also returning an array with counts of identifiers per issuer - - function lookupIdentifiersByAddress( + function lookupUnrevokedIdentifiers( address account, - address[] memory trustedIssuers, + address[] calldata trustedIssuers, uint256 maxIdentifiers - ) public view returns (bytes32[] memory) { + ) external view returns (uint256[] memory, bytes32[] memory) { + uint256[] memory countsPerIssuer = new uint256[](trustedIssuers.length); // Same as for the other lookup, preallocate and then trim for now uint256 currIndex = 0; bytes32[] memory identifiers = new bytes32[](maxIdentifiers); - for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { - address trustedIssuer = trustedIssuers[i]; - uint256 numIssuersForAddress = addressToIdentifiers[account][trustedIssuer].length; - for (uint256 j = 0; j < numIssuersForAddress; j = j.add(1)) { - // Iterate through the list of identifiers - if (currIndex < maxIdentifiers) { - bytes32 identifier = addressToIdentifiers[account][trustedIssuer][j]; - // Check if the mapping was produced by a revoked signer - // solhint-disable-next-line max-line-length - OwnershipAttestation[] memory attestationsForIssuer = identifierToAddresses[identifier][trustedIssuer]; - for (uint256 k = 0; k < attestationsForIssuer.length; k = k.add(1)) { - OwnershipAttestation memory attestation = attestationsForIssuer[k]; - // (identifier, account, issuer) tuples should be unique - if (attestation.account == account && !revokedSigners[attestation.signer]) { - identifiers[currIndex] = identifier; - currIndex = currIndex.add(1); - break; - } + OwnershipAttestation[] memory attestationsPerIssuer; + bytes32[] memory identifiersPerIssuer; + + for (uint256 i = 0; i < trustedIssuers.length && currIndex < maxIdentifiers; i = i.add(1)) { + identifiersPerIssuer = addressToIdentifiers[account][trustedIssuers[i]]; + for ( + uint256 j = 0; + j < identifiersPerIssuer.length && currIndex < maxIdentifiers; + j = j.add(1) + ) { + bytes32 identifier = identifiersPerIssuer[j]; + // Check if the mapping was produced by a revoked signer + attestationsPerIssuer = identifierToAttestations[identifier][trustedIssuers[i]]; + for (uint256 k = 0; k < attestationsPerIssuer.length; k = k.add(1)) { + // (identifier, account, issuer) tuples are checked for uniqueness on registration + if ( + !(attestationsPerIssuer[k].account == account) || + revokedSigners[attestationsPerIssuer[k].signer] + ) { + continue; } - } else { + identifiers[currIndex] = identifier; + currIndex = currIndex.add(1); + countsPerIssuer[i] = countsPerIssuer[i].add(1); break; } } } - if (currIndex < maxIdentifiers) { - // Allocate and fill properly-sized array - bytes32[] memory trimmedIdentifiers = new bytes32[](currIndex); - for (uint256 i = 0; i < currIndex; i = i.add(1)) { - trimmedIdentifiers[i] = identifiers[i]; + if (currIndex >= maxIdentifiers) { + return (countsPerIssuer, identifiers); + } + // Allocate and fill properly-sized array + bytes32[] memory trimmedIdentifiers = new bytes32[](currIndex); + for (uint256 i = 0; i < currIndex; i = i.add(1)) { + trimmedIdentifiers[i] = identifiers[i]; + } + return (countsPerIssuer, trimmedIdentifiers); + } + + /** + * @notice Similar to lookupUnrevokedIdentifiers but returns all identifiers + * mapped to an address with attestations from a list of issuers, + * either including or excluding attestations from revoked signers + * @param account Address of the account + * @param trustedIssuers Array of n issuers whose identifier mappings will be used + * @param includeRevoked Whether to include identifiers attested by revoked signers + * @return [0] Array of number of identifiers returned per issuer + * @return [1] Array (length == sum([0])) of identifiers + * @dev Adds identifier info to the arrays in order of provided trustedIssuers + * @dev Expectation that only one attestation exists per (identifier, issuer, account) + */ + function lookupIdentifiers( + address account, + address[] calldata trustedIssuers, + bool includeRevoked + ) external view returns (uint256[] memory, bytes32[] memory) { + uint256 totalIdentifiers; + uint256[] memory countsPerIssuer; + + (totalIdentifiers, countsPerIssuer) = includeRevoked + ? getNumIdentifiers(account, trustedIssuers) + : getNumUnrevokedIdentifiers(account, trustedIssuers); + + bytes32[] memory identifiers = new bytes32[](totalIdentifiers); + bytes32[] memory identifiersPerIssuer; + + uint256 currIndex = 0; + + for (uint256 i = 0; i < trustedIssuers.length; i = i.add(1)) { + identifiersPerIssuer = addressToIdentifiers[account][trustedIssuers[i]]; + for (uint256 j = 0; j < identifiersPerIssuer.length; j = j.add(1)) { + if ( + !includeRevoked && + !foundUnrevokedAttestation(account, identifiersPerIssuer[j], trustedIssuers[i]) + ) { + continue; + } + identifiers[currIndex] = identifiersPerIssuer[j]; + currIndex = currIndex.add(1); + } + } + return (countsPerIssuer, identifiers); + } + + /** + * @notice Helper function for lookupIdentifiers to search through the + * attestations from `issuer` for one with an unrevoked signer + * that maps `account` -> `identifier + * @param account Address of the account + * @param identifier Hash of the identifier + * @param issuer Issuer whose attestations to search + * @return Whether or not an unrevoked attestation is found establishing the mapping + */ + function foundUnrevokedAttestation(address account, bytes32 identifier, address issuer) + internal + view + returns (bool) + { + OwnershipAttestation[] memory attestations = identifierToAttestations[identifier][issuer]; + for (uint256 i = 0; i < attestations.length; i = i.add(1)) { + if (attestations[i].account == account && !revokedSigners[attestations[i].signer]) { + return true; } - return trimmedIdentifiers; - } else { - return identifiers; } + return false; } // TODO do we want to restrict permissions, or should anyone @@ -320,17 +615,17 @@ contract FederatedAttestations is isValidAttestation(identifier, issuer, account, issuedOn, signer, v, r, s), "Signature is invalid" ); - for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i = i.add(1)) { + for (uint256 i = 0; i < identifierToAttestations[identifier][issuer].length; i = i.add(1)) { // This enforces only one attestation to be uploaded // for a given set of (identifier, issuer, account) // Editing/upgrading an attestation requires that it be deleted before a new one is registered require( - identifierToAddresses[identifier][issuer][i].account != account, + identifierToAttestations[identifier][issuer][i].account != account, "Attestation for this account already exists" ); } OwnershipAttestation memory attestation = OwnershipAttestation(account, issuedOn, signer); - identifierToAddresses[identifier][issuer].push(attestation); + identifierToAttestations[identifier][issuer].push(attestation); addressToIdentifiers[account][issuer].push(identifier); emit AttestationRegistered(identifier, issuer, account, issuedOn, signer); } @@ -346,7 +641,7 @@ contract FederatedAttestations is public isValidUser(issuer, account) { - OwnershipAttestation[] memory attestations = identifierToAddresses[identifier][issuer]; + OwnershipAttestation[] memory attestations = identifierToAttestations[identifier][issuer]; for (uint256 i = 0; i < attestations.length; i = i.add(1)) { OwnershipAttestation memory attestation = attestations[i]; if (attestation.account == account) { @@ -354,8 +649,8 @@ contract FederatedAttestations is // and then move the last element in the array to that empty spot, // to avoid having empty elements in the array // TODO reviewers: is there a better way of doing this? - identifierToAddresses[identifier][issuer][i] = attestations[attestations.length - 1]; - identifierToAddresses[identifier][issuer].pop(); + identifierToAttestations[identifier][issuer][i] = attestations[attestations.length - 1]; + identifierToAttestations[identifier][issuer].pop(); bool deletedIdentifier = false; @@ -379,7 +674,7 @@ contract FederatedAttestations is function revokeSigner(address signer) public { // TODO ASv2 add constraints on who has permissions to revoke a signer - // TODO ASv2 consider whether we want to check if the signer is an authorized signer + // TODO ASv2 consider whether to check if the signer is an authorized signer // or to allow any address to be revoked revokedSigners[signer] = true; } diff --git a/packages/protocol/test/identity/federatedattestations.ts b/packages/protocol/test/identity/federatedattestations.ts index 7201c87d5ce..478bea69178 100644 --- a/packages/protocol/test/identity/federatedattestations.ts +++ b/packages/protocol/test/identity/federatedattestations.ts @@ -153,7 +153,7 @@ contract('FederatedAttestations', (accounts: string[]) => { }) }) - describe('#lookupAttestations', () => { + describe('looking up attestations', () => { interface AttestationTestCase { account: string issuedOn: number @@ -161,33 +161,61 @@ contract('FederatedAttestations', (accounts: string[]) => { } const checkAgainstExpectedAttestations = ( + expectedCountsPerIssuer: number[], expectedAttestations: AttestationTestCase[], + actualCountsPerIssuer: BigNumber[], actualAddresses: string[], actualIssuedOns: BigNumber[], actualSigners: string[] ) => { + expect(actualCountsPerIssuer.map((count) => count.toNumber())).to.eql(expectedCountsPerIssuer) + assert.lengthOf(actualAddresses, expectedAttestations.length) assert.lengthOf(actualIssuedOns, expectedAttestations.length) assert.lengthOf(actualSigners, expectedAttestations.length) expectedAttestations.forEach((expectedAttestation, index) => { - assert.equal(expectedAttestation.account, actualAddresses[index]) - assert.equal(expectedAttestation.issuedOn, actualIssuedOns[index].toNumber()) - assert.equal(expectedAttestation.signer, actualSigners[index]) + assert.equal(actualAddresses[index], expectedAttestation.account) + assert.equal(actualIssuedOns[index].toNumber(), expectedAttestation.issuedOn) + assert.equal(actualSigners[index], expectedAttestation.signer) }) } describe('when identifier has not been registered', () => { - it('should return empty list', async () => { - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer1], - 1 - ) - checkAgainstExpectedAttestations([], addresses, issuedOns, signers) + describe('#lookupUnrevokedAttestations', () => { + it('should return empty list', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations(identifier1, [issuer1], 1) + checkAgainstExpectedAttestations([0], [], countsPerIssuer, addresses, issuedOns, signers) + }) + }) + describe('#lookupAttestations', () => { + ;[true, false].forEach((includeRevoked) => { + describe(`includeRevoked = ${includeRevoked}`, () => { + it('should return empty list', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations(identifier1, [issuer1], true) + checkAgainstExpectedAttestations( + [0], + [], + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + }) + }) }) }) - describe('when identifier has been registered', () => { const account2 = accounts[3] @@ -243,94 +271,325 @@ contract('FederatedAttestations', (accounts: string[]) => { } }) - it('should return all attestations from one issuer', async () => { - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer1], - // Do not allow for maxAttestations to coincidentally limit incorrect output - issuer1Attestations.length + 1 - ) - checkAgainstExpectedAttestations(issuer1Attestations, addresses, issuedOns, signers) - }) + describe('#lookupUnrevokedAttestations', () => { + it('should return empty count if no issuers specified', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations(identifier1, [], 1) + checkAgainstExpectedAttestations([], [], countsPerIssuer, addresses, issuedOns, signers) + }) - it('should return empty list if no attestations exist for an issuer', async () => { - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer3], - 1 - ) - checkAgainstExpectedAttestations([], addresses, issuedOns, signers) - }) + it('should return all attestations from one issuer', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations( + identifier1, + [issuer1], + // Do not allow for maxAttestations to coincidentally limit incorrect output + issuer1Attestations.length + 1 + ) + checkAgainstExpectedAttestations( + [issuer1Attestations.length], + issuer1Attestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) - it('should return attestations from multiple issuers in correct order', async () => { - const expectedAttestations = issuer2Attestations.concat(issuer1Attestations) - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer3, issuer2, issuer1], - expectedAttestations.length + 1 - ) - checkAgainstExpectedAttestations(expectedAttestations, addresses, issuedOns, signers) - }) + it('should return empty list if no attestations exist for an issuer', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations(identifier1, [issuer3], 1) + checkAgainstExpectedAttestations([0], [], countsPerIssuer, addresses, issuedOns, signers) + }) - it('should return empty list if maxAttestations == 0', async () => { - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer1], - 0 - ) - checkAgainstExpectedAttestations([], addresses, issuedOns, signers) - }) + it('should return attestations from multiple issuers in correct order', async () => { + const expectedAttestations = issuer2Attestations.concat(issuer1Attestations) + const expectedCountsPerIssuer = [ + 0, + issuer2Attestations.length, + issuer1Attestations.length, + ] + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations( + identifier1, + [issuer3, issuer2, issuer1], + expectedAttestations.length + 1 + ) + checkAgainstExpectedAttestations( + expectedCountsPerIssuer, + expectedAttestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) - it('should only return maxAttestations attestations when more are present', async () => { - const expectedAttestations = issuer1Attestations.slice(0, -1) - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer1], - expectedAttestations.length - ) - checkAgainstExpectedAttestations(expectedAttestations, addresses, issuedOns, signers) - }) + it('should return empty list if maxAttestations == 0', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations(identifier1, [issuer1], 0) + checkAgainstExpectedAttestations([0], [], countsPerIssuer, addresses, issuedOns, signers) + }) - it('should not return attestations from revoked signers', async () => { - const attestationToRevoke = issuer2Attestations[0] - await federatedAttestations.revokeSigner(attestationToRevoke.signer) - const expectedAttestations = issuer2Attestations.slice(1) + it('should only return maxAttestations attestations when more are present', async () => { + const expectedAttestations = issuer1Attestations.slice(0, -1) + const expectedCountsPerIssuer = [expectedAttestations.length] + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations( + identifier1, + [issuer1], + expectedAttestations.length + ) + checkAgainstExpectedAttestations( + expectedCountsPerIssuer, + expectedAttestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) - const [addresses, issuedOns, signers] = await federatedAttestations.lookupAttestations( - identifier1, - [issuer2], - issuer2Attestations.length - ) - checkAgainstExpectedAttestations(expectedAttestations, addresses, issuedOns, signers) + it('should not return attestations from revoked signers', async () => { + const attestationToRevoke = issuer2Attestations[0] + await federatedAttestations.revokeSigner(attestationToRevoke.signer) + const expectedAttestations = issuer2Attestations.slice(1) + const expectedCountsPerIssuer = [expectedAttestations.length] + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupUnrevokedAttestations( + identifier1, + [issuer2], + issuer2Attestations.length + ) + checkAgainstExpectedAttestations( + expectedCountsPerIssuer, + expectedAttestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + }) + + describe('#lookupAttestations', () => { + ;[true, false].forEach((includeRevoked) => { + describe(`includeRevoked = ${includeRevoked}`, () => { + it('should return empty count and list if no issuers specified', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations(identifier1, [], includeRevoked) + checkAgainstExpectedAttestations( + [], + [], + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + + it('should return all attestations from one issuer', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations( + identifier1, + [issuer1], + includeRevoked + ) + checkAgainstExpectedAttestations( + [issuer1Attestations.length], + issuer1Attestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + + it('should return empty list if no attestations exist for an issuer', async () => { + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations( + identifier1, + [issuer3], + includeRevoked + ) + checkAgainstExpectedAttestations( + [0], + [], + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + + it('should return attestations from multiple issuers in correct order', async () => { + const expectedAttestations = issuer2Attestations.concat(issuer1Attestations) + const expectedCountsPerIssuer = [ + 0, + issuer2Attestations.length, + issuer1Attestations.length, + ] + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations( + identifier1, + [issuer3, issuer2, issuer1], + includeRevoked + ) + checkAgainstExpectedAttestations( + expectedCountsPerIssuer, + expectedAttestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + + if (includeRevoked) { + it('should return attestations from revoked and unrevoked signers', async () => { + await federatedAttestations.revokeSigner(issuer1Attestations[0].signer) + const expectedAttestations = issuer1Attestations.concat(issuer2Attestations) + const expectedCountsPerIssuer = [ + issuer1Attestations.length, + issuer2Attestations.length, + ] + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations( + identifier1, + [issuer1, issuer2], + includeRevoked + ) + checkAgainstExpectedAttestations( + expectedCountsPerIssuer, + expectedAttestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + } else { + it('should not return attestations from revoked signers', async () => { + const attestationToRevoke = issuer2Attestations[0] + await federatedAttestations.revokeSigner(attestationToRevoke.signer) + const expectedAttestations = issuer2Attestations.slice(1) + const expectedCountsPerIssuer = [expectedAttestations.length] + const [ + countsPerIssuer, + addresses, + issuedOns, + signers, + ] = await federatedAttestations.lookupAttestations( + identifier1, + [issuer2], + includeRevoked + ) + checkAgainstExpectedAttestations( + expectedCountsPerIssuer, + expectedAttestations, + countsPerIssuer, + addresses, + issuedOns, + signers + ) + }) + } + }) + }) }) }) }) - describe('#lookupIdentifiersByAddress', () => { + describe('looking up identifiers', () => { + interface IdentifierTestCase { + identifier: string + signer: string + } + + const checkAgainstExpectedIdCases = ( + expectedCountsPerIssuer: number[], + expectedIdentifiers: IdentifierTestCase[], + actualCountsPerIssuer: BigNumber[], + actualIdentifiers: string[] + ) => { + expect(actualCountsPerIssuer.map((count) => count.toNumber())).to.eql(expectedCountsPerIssuer) + expect(actualIdentifiers).to.eql(expectedIdentifiers.map((idCase) => idCase.identifier)) + } + describe('when address has not been registered', () => { - it('should return empty list', async () => { - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer1], - 1 - ) - assert.equal(actualIdentifiers.length, 0) + describe('#lookupUnrevokedIdentifiers', () => { + it('should return empty list', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers(account1, [issuer1], 1) + checkAgainstExpectedIdCases([0], [], actualCountsPerIssuer, actualIdentifiers) + }) + }) + describe('#lookupIdentifiers', () => { + ;[true, false].forEach((includeRevoked) => { + describe(`includeRevoked = ${includeRevoked}`, () => { + it('should return empty list', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers(account1, [issuer1], true) + checkAgainstExpectedIdCases([0], [], actualCountsPerIssuer, actualIdentifiers) + }) + }) + }) }) }) describe('when address has been registered', () => { - interface IdentifierTestCase { - identifier: string - signer: string - } - - const checkAgainstExpectedIdCases = ( - expectedIdentifiers: IdentifierTestCase[], - actualIdentifiers: string[] - ) => { - expect(expectedIdentifiers.map((idCase) => idCase.identifier)).to.eql(actualIdentifiers) - } - const issuer2 = accounts[3] const issuer2Signer = accounts[4] const issuer2Signer2 = accounts[5] @@ -376,62 +635,203 @@ contract('FederatedAttestations', (accounts: string[]) => { } }) - it('should return all identifiers from one issuer', async () => { - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer1], - issuer1IdCases.length + 1 - ) - checkAgainstExpectedIdCases(issuer1IdCases, actualIdentifiers) - }) + describe('#lookupUnrevokedIdentifiers', () => { + it('should return empty count if no issuers specified', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers(account1, [], 1) + checkAgainstExpectedIdCases([], [], actualCountsPerIssuer, actualIdentifiers) + }) - it('should return empty list if no identifiers exist for an (issuer,address)', async () => { - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer3], - 1 - ) - assert.equal(actualIdentifiers.length, 0) - }) + it('should return all identifiers from one issuer', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers( + account1, + [issuer1], + issuer1IdCases.length + 1 + ) + checkAgainstExpectedIdCases( + [issuer1IdCases.length], + issuer1IdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) - it('should return identifiers from multiple issuers in correct order', async () => { - const expectedIdCases = issuer2IdCases.concat(issuer1IdCases) - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer3, issuer2, issuer1], - expectedIdCases.length + 1 - ) - checkAgainstExpectedIdCases(expectedIdCases, actualIdentifiers) - }) + it('should return empty list if no identifiers exist for an (issuer,address)', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers(account1, [issuer3], 1) + checkAgainstExpectedIdCases([0], [], actualCountsPerIssuer, actualIdentifiers) + }) - it('should return empty list if maxIdentifiers == 0', async () => { - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer1], - 0 - ) - assert.equal(actualIdentifiers.length, 0) - }) + it('should return identifiers from multiple issuers in correct order', async () => { + const expectedIdCases = issuer2IdCases.concat(issuer1IdCases) + const expectedCountsPerIssuer = [0, issuer2IdCases.length, issuer1IdCases.length] + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers( + account1, + [issuer3, issuer2, issuer1], + expectedIdCases.length + 1 + ) + checkAgainstExpectedIdCases( + expectedCountsPerIssuer, + expectedIdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) - it('should only return maxIdentifiers identifiers when more are present', async () => { - const expectedIdCases = issuer2IdCases.concat(issuer1IdCases).slice(0, -1) - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer2, issuer1], - expectedIdCases.length - ) - checkAgainstExpectedIdCases(expectedIdCases, actualIdentifiers) + it('should return empty list if maxIdentifiers == 0', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers(account1, [issuer1], 0) + checkAgainstExpectedIdCases([0], [], actualCountsPerIssuer, actualIdentifiers) + }) + + it('should only return maxIdentifiers identifiers when more are present', async () => { + const expectedIdCases = issuer2IdCases.concat(issuer1IdCases).slice(0, -1) + const expectedCountsPerIssuer = [ + issuer2IdCases.length, + expectedIdCases.length - issuer2IdCases.length, + ] + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers( + account1, + [issuer2, issuer1], + expectedIdCases.length + ) + checkAgainstExpectedIdCases( + expectedCountsPerIssuer, + expectedIdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) + + it('should not return identifiers from revoked signers', async () => { + await federatedAttestations.revokeSigner(issuer2IdCases[0].signer) + const expectedIdCases = issuer2IdCases.slice(1) + const expectedCountsPerIssuer = [expectedIdCases.length] + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupUnrevokedIdentifiers( + account1, + [issuer2], + expectedIdCases.length + 1 + ) + checkAgainstExpectedIdCases( + expectedCountsPerIssuer, + expectedIdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) }) - it('should not return identifiers from revoked signers', async () => { - await federatedAttestations.revokeSigner(issuer2IdCases[0].signer) - const expectedIdCases = issuer2IdCases.slice(1) - const actualIdentifiers = await federatedAttestations.lookupIdentifiersByAddress( - account1, - [issuer2], - expectedIdCases.length + 1 - ) - checkAgainstExpectedIdCases(expectedIdCases, actualIdentifiers) + describe('#lookupIdentifiers', () => { + ;[true, false].forEach((includeRevoked) => { + describe(`includeRevoked = ${includeRevoked}`, () => { + it('should return empty count if no issuers specified', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers(account1, [], includeRevoked) + checkAgainstExpectedIdCases([], [], actualCountsPerIssuer, actualIdentifiers) + }) + + it('should return all identifiers from one issuer', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers(account1, [issuer1], includeRevoked) + checkAgainstExpectedIdCases( + [issuer1IdCases.length], + issuer1IdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) + + it('should return empty list if no identifiers exist for an (issuer,address)', async () => { + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers(account1, [issuer3], includeRevoked) + checkAgainstExpectedIdCases([0], [], actualCountsPerIssuer, actualIdentifiers) + }) + + it('should return identifiers from multiple issuers in correct order', async () => { + const expectedIdCases = issuer2IdCases.concat(issuer1IdCases) + const expectedCountsPerIssuer = [0, issuer2IdCases.length, issuer1IdCases.length] + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers( + account1, + [issuer3, issuer2, issuer1], + includeRevoked + ) + checkAgainstExpectedIdCases( + expectedCountsPerIssuer, + expectedIdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) + + if (includeRevoked) { + it('should return identifiers from revoked and unrevoked signers', async () => { + await federatedAttestations.revokeSigner(issuer2IdCases[0].signer) + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers( + account1, + [issuer2], + includeRevoked + ) + checkAgainstExpectedIdCases( + [issuer2IdCases.length], + issuer2IdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) + } else { + it('should not return identifiers from revoked signers', async () => { + await federatedAttestations.revokeSigner(issuer2IdCases[0].signer) + const expectedIdCases = issuer2IdCases.slice(1) + const expectedCountsPerIssuer = [expectedIdCases.length] + + const [ + actualCountsPerIssuer, + actualIdentifiers, + ] = await federatedAttestations.lookupIdentifiers( + account1, + [issuer2], + includeRevoked + ) + checkAgainstExpectedIdCases( + expectedCountsPerIssuer, + expectedIdCases, + actualCountsPerIssuer, + actualIdentifiers + ) + }) + } + }) + }) }) }) })