From b0f2d718a2bd2fdb2c6620a845c7c26cbdc6dec5 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Wed, 30 Mar 2022 22:26:23 +0200 Subject: [PATCH 01/12] Add barebones FederatedAttestations contract, proxy, and interface --- .../identity/FederatedAttestations.sol | 60 +++++++++++++++++++ .../interfaces/IFederatedAttestations.sol | 5 ++ .../proxies/FederatedAttestationsProxy.sol | 6 ++ 3 files changed, 71 insertions(+) create mode 100644 packages/protocol/contracts/identity/FederatedAttestations.sol create mode 100644 packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol create mode 100644 packages/protocol/contracts/identity/proxies/FederatedAttestationsProxy.sol diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol new file mode 100644 index 00000000000..eb25bc3d5c4 --- /dev/null +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -0,0 +1,60 @@ +pragma solidity ^0.5.13; + +import "openzeppelin-solidity/contracts/math/SafeMath.sol"; +import "openzeppelin-solidity/contracts/ownership/Ownable.sol"; +import "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol"; +import "openzeppelin-solidity/contracts/utils/SafeCast.sol"; + +import "./interfaces/IFederatedAttestations.sol"; +import "../common/interfaces/IAccounts.sol"; +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 + */ +contract FederatedAttestations is + IFederatedAttestations, + ICeloVersionedContract, + Ownable, + Initializable, + UsingRegistry, + ReentrancyGuard, + UsingPrecompiles +{ + using SafeMath for uint256; + using SafeCast for uint256; + + // TODO State var declarations + + // TODO Event declarations + + /** + * @notice Sets initialized == true on implementation contracts + * @param test Set to true to skip implementation initialization + */ + constructor(bool test) public Initializable(test) {} + + /** + * @notice Used in place of the constructor to allow the contract to be upgradable via proxy. + * @param registryAddress The address of the registry core smart contract. + */ + function initialize(address registryAddress) external initializer { + _transferOwnership(msg.sender); + setRegistry(registryAddress); + // TODO initialize any other variables here + } + + /** + * @notice Returns the storage, major, minor, and patch version of the contract. + * @return The storage, major, minor, and patch version of the contract. + */ + function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { + return (1, 1, 0, 0); + } +} diff --git a/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol b/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol new file mode 100644 index 00000000000..de010be772e --- /dev/null +++ b/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol @@ -0,0 +1,5 @@ +pragma solidity ^0.5.13; + +// TODO add external, view, and only owner function sigs +// separated into these three groups for clarity +interface IFederatedAttestations {} diff --git a/packages/protocol/contracts/identity/proxies/FederatedAttestationsProxy.sol b/packages/protocol/contracts/identity/proxies/FederatedAttestationsProxy.sol new file mode 100644 index 00000000000..6edd4884f1a --- /dev/null +++ b/packages/protocol/contracts/identity/proxies/FederatedAttestationsProxy.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.5.13; + +import "../../common/Proxy.sol"; + +/* solhint-disable no-empty-blocks */ +contract FederatedAttestationsProxy is Proxy {} From 66b25f451907f96b50f0adb0d69c03b5deb05b15 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Thu, 31 Mar 2022 14:23:50 +0200 Subject: [PATCH 02/12] Make TODOs ASv2 specific --- .../protocol/contracts/identity/FederatedAttestations.sol | 6 +++--- .../identity/interfaces/IFederatedAttestations.sol | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index eb25bc3d5c4..3483cf0b12f 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -30,9 +30,9 @@ contract FederatedAttestations is using SafeMath for uint256; using SafeCast for uint256; - // TODO State var declarations + // TODO ASv2 State var declarations - // TODO Event declarations + // TODO ASv2 Event declarations /** * @notice Sets initialized == true on implementation contracts @@ -47,7 +47,7 @@ contract FederatedAttestations is function initialize(address registryAddress) external initializer { _transferOwnership(msg.sender); setRegistry(registryAddress); - // TODO initialize any other variables here + // TODO ASv2 initialize any other variables here } /** diff --git a/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol b/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol index de010be772e..d72986621d7 100644 --- a/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol +++ b/packages/protocol/contracts/identity/interfaces/IFederatedAttestations.sol @@ -1,5 +1,5 @@ pragma solidity ^0.5.13; -// TODO add external, view, and only owner function sigs +// TODO ASv2 add external, view, and only owner function sigs // separated into these three groups for clarity interface IFederatedAttestations {} From 18e682e362d8d9b75f77c44fd7cd4aa9555c1d18 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Thu, 31 Mar 2022 15:58:47 +0200 Subject: [PATCH 03/12] Add FA usages to existing files --- packages/protocol/lib/registry-utils.ts | 3 +++ packages/protocol/lib/test-utils.ts | 2 ++ packages/protocol/migrations/25_governance.ts | 1 + packages/protocol/scripts/build.ts | 2 ++ 4 files changed, 8 insertions(+) diff --git a/packages/protocol/lib/registry-utils.ts b/packages/protocol/lib/registry-utils.ts index d6dae2db095..64e541ba57b 100644 --- a/packages/protocol/lib/registry-utils.ts +++ b/packages/protocol/lib/registry-utils.ts @@ -20,6 +20,7 @@ export enum CeloContractName { Exchange = 'Exchange', ExchangeEUR = 'ExchangeEUR', ExchangeBRL = 'ExchangeBRL', + FederatedAttestations = 'FederatedAttestations', FeeCurrencyWhitelist = 'FeeCurrencyWhitelist', Freezer = 'Freezer', GasPriceMinimum = 'GasPriceMinimum', @@ -55,6 +56,8 @@ export const hasEntryInRegistry: string[] = [ CeloContractName.Election, CeloContractName.Escrow, CeloContractName.Exchange, + // TODO ASv2 revisit this + // CeloContractName.FederatedAttestations, CeloContractName.FeeCurrencyWhitelist, CeloContractName.Freezer, CeloContractName.GasPriceMinimum, diff --git a/packages/protocol/lib/test-utils.ts b/packages/protocol/lib/test-utils.ts index fa1d341bbc6..ee77db99670 100644 --- a/packages/protocol/lib/test-utils.ts +++ b/packages/protocol/lib/test-utils.ts @@ -377,6 +377,7 @@ export const isSameAddress = (minerAddress, otherAddress) => { // TODO(amy): Pull this list from the build artifacts instead export const proxiedContracts: string[] = [ 'Attestations', + // TODO ASv2 revisit if we need to update test-utils 'Escrow', 'GoldToken', 'Registry', @@ -388,6 +389,7 @@ export const proxiedContracts: string[] = [ // TODO(asa): Pull this list from the build artifacts instead export const ownedContracts: string[] = [ 'Attestations', + // TODO ASv2 revisit if we need to update test-utils 'Escrow', 'Exchange', 'Registry', diff --git a/packages/protocol/migrations/25_governance.ts b/packages/protocol/migrations/25_governance.ts index 78a731eb053..e91ad42fc0e 100644 --- a/packages/protocol/migrations/25_governance.ts +++ b/packages/protocol/migrations/25_governance.ts @@ -83,6 +83,7 @@ module.exports = deploymentForCoreContract( 'Escrow', 'Exchange', 'ExchangeEUR', + // TODO ASv2 revisit 'FeeCurrencyWhitelist', 'Freezer', 'GasPriceMinimum', diff --git a/packages/protocol/scripts/build.ts b/packages/protocol/scripts/build.ts index 1f4f78240f7..0a15471c3be 100644 --- a/packages/protocol/scripts/build.ts +++ b/packages/protocol/scripts/build.ts @@ -20,6 +20,7 @@ export const ProxyContracts = [ 'ExchangeBRLProxy', 'ExchangeEURProxy', 'ExchangeProxy', + 'FederatedAttestationsProxy', 'FeeCurrencyWhitelistProxy', 'GasPriceMinimumProxy', 'GoldTokenProxy', @@ -66,6 +67,7 @@ export const CoreContracts = [ // identity 'Attestations', 'Escrow', + 'FederatedAttestations', 'Random', // stability From 3253456cbe18e860d7a9f5f8056162793325d2a6 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Thu, 31 Mar 2022 15:59:00 +0200 Subject: [PATCH 04/12] Add skeleton migration file --- .../migrations/27_federated_attestations.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 packages/protocol/migrations/27_federated_attestations.ts diff --git a/packages/protocol/migrations/27_federated_attestations.ts b/packages/protocol/migrations/27_federated_attestations.ts new file mode 100644 index 00000000000..ad25500c0a6 --- /dev/null +++ b/packages/protocol/migrations/27_federated_attestations.ts @@ -0,0 +1,15 @@ +import { CeloContractName } from '@celo/protocol/lib/registry-utils' +import { deploymentForCoreContract } from '@celo/protocol/lib/web3-utils' +import { config } from '@celo/protocol/migrationsConfig' +import { FederatedAttestationsInstance } from 'types' + +const initializeArgs = async (): Promise<[string]> => { + return [config.registry.predeployedProxyAddress] +} + +module.exports = deploymentForCoreContract( + web3, + artifacts, + CeloContractName.FederatedAttestations, + initializeArgs +) From 58c1f6d0a2e07a57b42919783ceba4308e51a287 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Thu, 31 Mar 2022 20:21:23 +0200 Subject: [PATCH 05/12] Add test scaffolding --- .../protocol/scripts/bash/backupmigrations.sh | 1 + .../test/identity/federatedattestations.ts | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 packages/protocol/test/identity/federatedattestations.ts diff --git a/packages/protocol/scripts/bash/backupmigrations.sh b/packages/protocol/scripts/bash/backupmigrations.sh index e7005a1f8e7..a5cfedf501e 100755 --- a/packages/protocol/scripts/bash/backupmigrations.sh +++ b/packages/protocol/scripts/bash/backupmigrations.sh @@ -45,4 +45,5 @@ else # cp migrations.bak/23_governance_approver_multisig.* migrations/ # cp migrations.bak/24_governance.* migrations/ # cp migrations.bak/25_elect_validators.* migrations/ + # cp migrations.bak/27_federated_attestations.* migrations/ fi \ No newline at end of file diff --git a/packages/protocol/test/identity/federatedattestations.ts b/packages/protocol/test/identity/federatedattestations.ts new file mode 100644 index 00000000000..ab9bc37f9fa --- /dev/null +++ b/packages/protocol/test/identity/federatedattestations.ts @@ -0,0 +1,42 @@ +import { CeloContractName } from '@celo/protocol/lib/registry-utils' +import { + AccountsContract, + AccountsInstance, + FederatedAttestationsContract, + FederatedAttestationsInstance, + RegistryContract, + RegistryInstance, +} from 'types' + +const Accounts: AccountsContract = artifacts.require('Accounts') +const FederatedAttestations: FederatedAttestationsContract = artifacts.require( + 'FederatedAttestations' +) +const Registry: RegistryContract = artifacts.require('Registry') + +contract('Attestations', (accounts: string[]) => { + let accountsInstance: AccountsInstance + let federatedAttestations: FederatedAttestationsInstance + let registry: RegistryInstance + + const caller: string = accounts[0] + // const phoneNumber: string = '+18005551212' + // const phoneHash: string = getPhoneHash(phoneNumber) + + beforeEach('FederatedAttestations setup', async () => { + accountsInstance = await Accounts.new(true) + federatedAttestations = await FederatedAttestations.new(true) + registry = await Registry.new(true) + await accountsInstance.initialize(registry.address) + await registry.setAddressFor(CeloContractName.Accounts, accountsInstance.address) + await federatedAttestations.initialize(registry.address) + }) + + describe('#initialize()', () => { + it('TODO ASv2', async () => { + // TODO ASv2; asserting these just to keep the vars + assert(caller) + assert(federatedAttestations) + }) + }) +}) From 475baf2d45e90fd313367af3efebe429eb3712bd Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Thu, 31 Mar 2022 21:33:59 +0200 Subject: [PATCH 06/12] Add CK wrapper and test --- packages/protocol/lib/registry-utils.ts | 2 +- packages/protocol/migrationsConfig.js | 2 ++ .../test/identity/federatedattestations.ts | 3 ++- packages/sdk/contractkit/src/base.ts | 1 + .../sdk/contractkit/src/contract-cache.ts | 6 +++++ packages/sdk/contractkit/src/kit.ts | 5 ++++ packages/sdk/contractkit/src/proxy.ts | 2 ++ .../contractkit/src/web3-contract-cache.ts | 5 ++++ .../wrappers/FederatedAttestations.test.ts | 26 +++++++++++++++++++ .../src/wrappers/FederatedAttestations.ts | 8 ++++++ 10 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 packages/sdk/contractkit/src/wrappers/FederatedAttestations.test.ts create mode 100644 packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts diff --git a/packages/protocol/lib/registry-utils.ts b/packages/protocol/lib/registry-utils.ts index 64e541ba57b..5d4aa040bff 100644 --- a/packages/protocol/lib/registry-utils.ts +++ b/packages/protocol/lib/registry-utils.ts @@ -57,7 +57,7 @@ export const hasEntryInRegistry: string[] = [ CeloContractName.Escrow, CeloContractName.Exchange, // TODO ASv2 revisit this - // CeloContractName.FederatedAttestations, + CeloContractName.FederatedAttestations, CeloContractName.FeeCurrencyWhitelist, CeloContractName.Freezer, CeloContractName.GasPriceMinimum, diff --git a/packages/protocol/migrationsConfig.js b/packages/protocol/migrationsConfig.js index 6df3ee5d3f2..799b124efd3 100644 --- a/packages/protocol/migrationsConfig.js +++ b/packages/protocol/migrationsConfig.js @@ -26,6 +26,8 @@ const DAY = 24 * HOUR const WEEK = 7 * DAY const YEAR = 365 * DAY +// TODO ASv2 + const DefaultConfig = { attestations: { attestationExpiryBlocks: HOUR / 5, // ~1 hour, diff --git a/packages/protocol/test/identity/federatedattestations.ts b/packages/protocol/test/identity/federatedattestations.ts index ab9bc37f9fa..54070cb21e9 100644 --- a/packages/protocol/test/identity/federatedattestations.ts +++ b/packages/protocol/test/identity/federatedattestations.ts @@ -1,4 +1,5 @@ import { CeloContractName } from '@celo/protocol/lib/registry-utils' +// import { getPhoneHash } from '@celo/utils/lib/phoneNumbers' import { AccountsContract, AccountsInstance, @@ -34,7 +35,7 @@ contract('Attestations', (accounts: string[]) => { describe('#initialize()', () => { it('TODO ASv2', async () => { - // TODO ASv2; asserting these just to keep the vars + // TODO ASv2 assert(caller) assert(federatedAttestations) }) diff --git a/packages/sdk/contractkit/src/base.ts b/packages/sdk/contractkit/src/base.ts index 50e10a369ed..fe1a5376a2e 100644 --- a/packages/sdk/contractkit/src/base.ts +++ b/packages/sdk/contractkit/src/base.ts @@ -11,6 +11,7 @@ export enum CeloContract { Exchange = 'Exchange', ExchangeEUR = 'ExchangeEUR', ExchangeBRL = 'ExchangeBRL', + FederatedAttestations = 'FederatedAttestations', FeeCurrencyWhitelist = 'FeeCurrencyWhitelist', Freezer = 'Freezer', GasPriceMinimum = 'GasPriceMinimum', diff --git a/packages/sdk/contractkit/src/contract-cache.ts b/packages/sdk/contractkit/src/contract-cache.ts index a229c56d97d..bdb1bcd5f2b 100644 --- a/packages/sdk/contractkit/src/contract-cache.ts +++ b/packages/sdk/contractkit/src/contract-cache.ts @@ -13,6 +13,7 @@ import { Erc20Wrapper } from './wrappers/Erc20Wrapper' // import { EpochRewardsWrapper } from './wrappers/EpochRewards' import { EscrowWrapper } from './wrappers/Escrow' import { ExchangeWrapper } from './wrappers/Exchange' +import { FederatedAttestationsWrapper } from './wrappers/FederatedAttestations' import { FreezerWrapper } from './wrappers/Freezer' import { GasPriceMinimumWrapper } from './wrappers/GasPriceMinimum' import { GoldTokenWrapper } from './wrappers/GoldTokenWrapper' @@ -40,6 +41,7 @@ const WrapperFactories = { [CeloContract.Exchange]: ExchangeWrapper, [CeloContract.ExchangeEUR]: ExchangeWrapper, [CeloContract.ExchangeBRL]: ExchangeWrapper, + [CeloContract.FederatedAttestations]: FederatedAttestationsWrapper, // [CeloContract.FeeCurrencyWhitelist]: FeeCurrencyWhitelistWrapper, [CeloContract.Freezer]: FreezerWrapper, [CeloContract.GasPriceMinimum]: GasPriceMinimumWrapper, @@ -76,6 +78,7 @@ interface WrapperCacheMap { [CeloContract.Exchange]?: ExchangeWrapper [CeloContract.ExchangeEUR]?: ExchangeWrapper [CeloContract.ExchangeBRL]?: ExchangeWrapper + [CeloContract.FederatedAttestations]?: FederatedAttestationsWrapper // [CeloContract.FeeCurrencyWhitelist]?: FeeCurrencyWhitelistWrapper, [CeloContract.Freezer]?: FreezerWrapper [CeloContract.GasPriceMinimum]?: GasPriceMinimumWrapper @@ -139,6 +142,9 @@ export class WrapperCache { getFreezer() { return this.getContract(CeloContract.Freezer) } + getFederatedAttestations() { + return this.getContract(CeloContract.FederatedAttestations) + } // getFeeCurrencyWhitelist() { // return this.getWrapper(CeloContract.FeeCurrencyWhitelist, newFeeCurrencyWhitelist) // } diff --git a/packages/sdk/contractkit/src/kit.ts b/packages/sdk/contractkit/src/kit.ts index 2887d7fb192..0478f105d0c 100644 --- a/packages/sdk/contractkit/src/kit.ts +++ b/packages/sdk/contractkit/src/kit.ts @@ -22,6 +22,7 @@ import { BlockchainParametersConfig } from './wrappers/BlockchainParameters' import { DowntimeSlasherConfig } from './wrappers/DowntimeSlasher' import { ElectionConfig } from './wrappers/Election' import { ExchangeConfig } from './wrappers/Exchange' +import { FederatedAttestationsConfig } from './wrappers/FederatedAttestations' import { GasPriceMinimumConfig } from './wrappers/GasPriceMinimum' import { GovernanceConfig } from './wrappers/Governance' import { GrandaMentoConfig } from './wrappers/GrandaMento' @@ -84,6 +85,8 @@ export interface NetworkConfig { stableTokens: EachCeloToken election: ElectionConfig attestations: AttestationsConfig + // TODO ASv2 + federatedattestations: FederatedAttestationsConfig governance: GovernanceConfig lockedGold: LockedGoldConfig sortedOracles: SortedOraclesConfig @@ -147,6 +150,8 @@ export class ContractKit { const configContracts: ValidWrappers[] = [ CeloContract.Election, CeloContract.Attestations, + // TODO ASv2 + CeloContract.FederatedAttestations, CeloContract.Governance, CeloContract.LockedGold, CeloContract.SortedOracles, diff --git a/packages/sdk/contractkit/src/proxy.ts b/packages/sdk/contractkit/src/proxy.ts index 427bc363e1f..c9a4867c713 100644 --- a/packages/sdk/contractkit/src/proxy.ts +++ b/packages/sdk/contractkit/src/proxy.ts @@ -9,6 +9,7 @@ import { ABI as ElectionABI } from './generated/Election' import { ABI as EpochRewardsABI } from './generated/EpochRewards' import { ABI as EscrowABI } from './generated/Escrow' import { ABI as ExchangeABI } from './generated/Exchange' +import { ABI as FederatedAttestationsABI } from './generated/FederatedAttestations' import { ABI as FeeCurrencyWhitelistABI } from './generated/FeeCurrencyWhitelist' import { ABI as FreezerABI } from './generated/Freezer' import { ABI as GasPriceMinimumABI } from './generated/GasPriceMinimum' @@ -104,6 +105,7 @@ const initializeAbiMap = { ExchangeProxy: findInitializeAbi(ExchangeABI), ExchangeEURProxy: findInitializeAbi(ExchangeABI), ExchangeBRLProxy: findInitializeAbi(ExchangeABI), + FederatedAttestationsProxy: findInitializeAbi(FederatedAttestationsABI), FeeCurrencyWhitelistProxy: findInitializeAbi(FeeCurrencyWhitelistABI), FreezerProxy: findInitializeAbi(FreezerABI), GasPriceMinimumProxy: findInitializeAbi(GasPriceMinimumABI), diff --git a/packages/sdk/contractkit/src/web3-contract-cache.ts b/packages/sdk/contractkit/src/web3-contract-cache.ts index d1d56e330fa..43dffb04812 100644 --- a/packages/sdk/contractkit/src/web3-contract-cache.ts +++ b/packages/sdk/contractkit/src/web3-contract-cache.ts @@ -12,6 +12,7 @@ import { newEscrow } from './generated/Escrow' import { newExchange } from './generated/Exchange' import { newExchangeBrl } from './generated/ExchangeBRL' import { newExchangeEur } from './generated/ExchangeEUR' +import { newFederatedAttestations } from './generated/FederatedAttestations' import { newFeeCurrencyWhitelist } from './generated/FeeCurrencyWhitelist' import { newFreezer } from './generated/Freezer' import { newGasPriceMinimum } from './generated/GasPriceMinimum' @@ -48,6 +49,7 @@ export const ContractFactories = { [CeloContract.Exchange]: newExchange, [CeloContract.ExchangeEUR]: newExchangeEur, [CeloContract.ExchangeBRL]: newExchangeBrl, + [CeloContract.FederatedAttestations]: newFederatedAttestations, [CeloContract.FeeCurrencyWhitelist]: newFeeCurrencyWhitelist, [CeloContract.Freezer]: newFreezer, [CeloContract.GasPriceMinimum]: newGasPriceMinimum, @@ -114,6 +116,9 @@ export class Web3ContractCache { getExchange(stableToken: StableToken = StableToken.cUSD) { return this.getContract(this.kit.celoTokens.getExchangeContract(stableToken)) } + getFederatedAttestations() { + return this.getContract(CeloContract.FederatedAttestations) + } getFeeCurrencyWhitelist() { return this.getContract(CeloContract.FeeCurrencyWhitelist) } diff --git a/packages/sdk/contractkit/src/wrappers/FederatedAttestations.test.ts b/packages/sdk/contractkit/src/wrappers/FederatedAttestations.test.ts new file mode 100644 index 00000000000..de69785bb59 --- /dev/null +++ b/packages/sdk/contractkit/src/wrappers/FederatedAttestations.test.ts @@ -0,0 +1,26 @@ +import { testWithGanache } from '@celo/dev-utils/lib/ganache-test' +// import { PhoneNumberUtils } from '@celo/utils' +import { newKitFromWeb3 } from '../kit' +import { FederatedAttestationsWrapper } from './FederatedAttestations' + +testWithGanache('FederatedAttestations Wrapper', (web3) => { + // const PHONE_NUMBER = '+15555555555' + // const IDENTIFIER = PhoneNumberUtils.getPhoneHash(PHONE_NUMBER) + + const kit = newKitFromWeb3(web3) + let accounts: string[] = [] + let federatedAttestations: FederatedAttestationsWrapper + + beforeAll(async () => { + accounts = await web3.eth.getAccounts() + kit.defaultAccount = accounts[0] + }) + + describe('TODO ASv2', () => { + it('TODO ASv2', async () => { + expect(accounts) + federatedAttestations = await kit.contracts.getFederatedAttestations() + expect(federatedAttestations) + }) + }) +}) diff --git a/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts b/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts new file mode 100644 index 00000000000..dbb665ce97a --- /dev/null +++ b/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts @@ -0,0 +1,8 @@ +import { FederatedAttestations } from '../generated/FederatedAttestations' +import { BaseWrapper } from './BaseWrapper' + +export interface FederatedAttestationsConfig { + // TODO ASv2 +} + +export class FederatedAttestationsWrapper extends BaseWrapper {} From e6a192b31306dd7a4db945d56321bbbe07237b3d Mon Sep 17 00:00:00 2001 From: Isabelle Wei Date: Fri, 1 Apr 2022 01:19:03 -0400 Subject: [PATCH 07/12] storage variables, reading and writing functions --- .../identity/FederatedAttestations.sol | 89 ++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index 3483cf0b12f..72ff7420699 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -30,7 +30,17 @@ contract FederatedAttestations is using SafeMath for uint256; using SafeCast for uint256; - // TODO ASv2 State var declarations + struct Attestation { + address account; + uint256 issuedOn; + address signer; + } + // identifier -> issuer -> attestations + mapping(bytes32 => mapping(address => Attestation[])) public identifierToAddresses; + // account -> issuer -> identifiers + mapping(address => mapping(address => bytes32[])) public addressToIdentifiers; + // signer => revocation time + mapping(address => uint256) public revokedSigners; // TODO ASv2 Event declarations @@ -57,4 +67,81 @@ contract FederatedAttestations is function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) { return (1, 1, 0, 0); } + + function _isRevoked(address signer, uint256 time) internal returns (bool) { + if (revokedSigners[signer] > 0 && revokedSigners[signer] >= time) { + return true; + } + return false; + } + + function lookupAttestations(string identifier, address[] trustedIssuers) + public + view + returns (Attestation[]) + { + Attestation[] memory attestations = new Attestation[]; + for (uint256 i = 0; i < trustedIssuers.length; i++) { + address trustedIssuer = trustedIssuers[i]; + for (uint256 j = 0; j < identifierToAddresses[identifier][trustedIssuer].length; j++) { + Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer][j]; + if (!_isRevoked(attestation.signer, attestation.issuedOn)) { + attestations.push(attestation); + } + } + } + return attestations; + } + + function lookupIdentifiersbyAddress(address account, address[] trustedIssuers) + public + view + returns (bytes32[]) + { + bytes32[] memory identifiers = new bytes32[]; + for (uint256 i = 0; i < trustedIssuers.length; i++) { + address trustedIssuer = trustedIssuers[i]; + for (uint256 j = 0; j < addressToIdentifiers[account][trustedIssuer].length; j++) { + bytes32 memory identifier = addressToIdentifiers[account][trustedIssuer][j]; + Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer]; + if (!_isRevoked(attestation.signer, attestation.issuedOn)) { + identifiers.push(identifier); + } + } + } + return identifiers; + } + + function registerAttestation(bytes32 identifier, address issuer, Attestation attestation) public { + require( + msg.sender == attestation.account || msg.sender == issuer || msg.sender == attestation.signer + ); + for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i++) { + require(identifierToAddresses[identifier][issuer][i] != attestation.account); + } + identifierToAddresses[identifier][issuer].push(attestation); + addressToIdentifiers[attestation.account][issuer] = identifier; + } + + function deleteAttestation(bytes32 identifier, address issuer, address account) public { + require(msg.sender == attestation.account || msg.sender == issuer); + for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i++) { + if (identifierToAddresses[identifier][issuer][i].account == account) { + identifierToAddresses[identifier][issuer][i] = identifierToAddresses[identifier][issuer][identifierToAddresses[identifier][issuer] + .length - + 1]; + identifierToAddresses[identifier][issuer].pop(); + for (uint256 i = 0; i < addressToIdentifiers[account][issuer].length; i++) { + if (addressToIdentifiers[account][issuer][i].account == account) { + addressToIdentifiers[account][issuer][i] = addressToIdentifiers[account][issuer][addressToIdentifiers[account][issuer] + .length - + 1]; + addressToIdentifiers[account][issuer].pop(); + break; + } + } + break; + } + } + } } From 9fdc55b75871b98929ef3c38230920caf078a6f3 Mon Sep 17 00:00:00 2001 From: Isabelle Wei Date: Wed, 13 Apr 2022 12:29:09 -0400 Subject: [PATCH 08/12] add function signatures --- .../contracts/identity/FederatedAttestations.sol | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index 72ff7420699..853f28cf868 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -112,6 +112,15 @@ contract FederatedAttestations is return identifiers; } + function validateAttestation( + bytes32 identifier, + address issuer, + Attestation attestation, + uint8 v, + bytes32 r, + bytes32 s + ) public view returns (address) {} + function registerAttestation(bytes32 identifier, address issuer, Attestation attestation) public { require( msg.sender == attestation.account || msg.sender == issuer || msg.sender == attestation.signer @@ -144,4 +153,8 @@ contract FederatedAttestations is } } } + + function revokeSigner(address signer, uint256 revokedOn) public { + revokedSigners[signer] = revokedOn; + } } From c5ac2b679b1c73b01b2ff6d41772ebe81d0ca48d Mon Sep 17 00:00:00 2001 From: Isabelle Wei Date: Mon, 18 Apr 2022 17:46:29 -0400 Subject: [PATCH 09/12] fix merge conflict --- packages/cli/package.json | 2 +- .../migrations/25_federated_attestations.ts | 15 +++++++++++++++ .../{25_governance.ts => 26_governance.ts} | 2 +- ...elect_validators.ts => 27_elect_validators.ts} | 0 .../protocol/scripts/bash/backupmigrations.sh | 7 ++++--- packages/sdk/contractkit/package.json | 2 +- packages/sdk/contractkit/src/kit.ts | 7 ++++--- .../src/wrappers/FederatedAttestations.ts | 6 +++--- packages/sdk/identity/package.json | 2 +- packages/sdk/transactions-uri/package.json | 2 +- 10 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 packages/protocol/migrations/25_federated_attestations.ts rename packages/protocol/migrations/{25_governance.ts => 26_governance.ts} (99%) rename packages/protocol/migrations/{26_elect_validators.ts => 27_elect_validators.ts} (100%) diff --git a/packages/cli/package.json b/packages/cli/package.json index f86b8dfd73d..4fbd1f09d98 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -28,7 +28,7 @@ "generate:shrinkwrap": "npm install --production && npm shrinkwrap", "check:shrinkwrap": "npm install --production && npm shrinkwrap && ./scripts/check_shrinkwrap_dirty.sh", "prepack": "yarn run build && oclif-dev manifest && oclif-dev readme && yarn run check:shrinkwrap", - "test:reset": "yarn --cwd ../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../dev-utils/src/migration-override.json --upto 25 --release_gold_contracts scripts/truffle/releaseGoldExampleConfigs.json", + "test:reset": "yarn --cwd ../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../dev-utils/src/migration-override.json --upto 26 --release_gold_contracts scripts/truffle/releaseGoldExampleConfigs.json", "test:livechain": "yarn --cwd ../protocol devchain run-tar .tmp/devchain.tar.gz", "test": "TZ=UTC jest --runInBand" }, diff --git a/packages/protocol/migrations/25_federated_attestations.ts b/packages/protocol/migrations/25_federated_attestations.ts new file mode 100644 index 00000000000..ad25500c0a6 --- /dev/null +++ b/packages/protocol/migrations/25_federated_attestations.ts @@ -0,0 +1,15 @@ +import { CeloContractName } from '@celo/protocol/lib/registry-utils' +import { deploymentForCoreContract } from '@celo/protocol/lib/web3-utils' +import { config } from '@celo/protocol/migrationsConfig' +import { FederatedAttestationsInstance } from 'types' + +const initializeArgs = async (): Promise<[string]> => { + return [config.registry.predeployedProxyAddress] +} + +module.exports = deploymentForCoreContract( + web3, + artifacts, + CeloContractName.FederatedAttestations, + initializeArgs +) diff --git a/packages/protocol/migrations/25_governance.ts b/packages/protocol/migrations/26_governance.ts similarity index 99% rename from packages/protocol/migrations/25_governance.ts rename to packages/protocol/migrations/26_governance.ts index e91ad42fc0e..a95777f27c2 100644 --- a/packages/protocol/migrations/25_governance.ts +++ b/packages/protocol/migrations/26_governance.ts @@ -83,7 +83,7 @@ module.exports = deploymentForCoreContract( 'Escrow', 'Exchange', 'ExchangeEUR', - // TODO ASv2 revisit + 'FederatedAttestations', 'FeeCurrencyWhitelist', 'Freezer', 'GasPriceMinimum', diff --git a/packages/protocol/migrations/26_elect_validators.ts b/packages/protocol/migrations/27_elect_validators.ts similarity index 100% rename from packages/protocol/migrations/26_elect_validators.ts rename to packages/protocol/migrations/27_elect_validators.ts diff --git a/packages/protocol/scripts/bash/backupmigrations.sh b/packages/protocol/scripts/bash/backupmigrations.sh index a5cfedf501e..6dfa89132ad 100755 --- a/packages/protocol/scripts/bash/backupmigrations.sh +++ b/packages/protocol/scripts/bash/backupmigrations.sh @@ -43,7 +43,8 @@ else # cp migrations.bak/21_double_signing_slasher.* migrations/ # cp migrations.bak/22_downtime_slasher.* migrations/ # cp migrations.bak/23_governance_approver_multisig.* migrations/ - # cp migrations.bak/24_governance.* migrations/ - # cp migrations.bak/25_elect_validators.* migrations/ - # cp migrations.bak/27_federated_attestations.* migrations/ + # cp migrations.bak/24_grandamento.* migrations/ + # cp migrations.bak/25_federated_attestations.* migrations/ + # cp migrations.bak/26_governance.* migrations/ + # cp migrations.bak/27_elect_validators.* migrations/ fi \ No newline at end of file diff --git a/packages/sdk/contractkit/package.json b/packages/sdk/contractkit/package.json index b7616be64b3..5d599cac662 100644 --- a/packages/sdk/contractkit/package.json +++ b/packages/sdk/contractkit/package.json @@ -21,7 +21,7 @@ "clean:all": "yarn clean && rm -rf src/generated", "prepublishOnly": "yarn build", "docs": "typedoc", - "test:reset": "yarn --cwd ../../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../../dev-utils/src/migration-override.json --upto 25", + "test:reset": "yarn --cwd ../../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../../dev-utils/src/migration-override.json --upto 26", "test:livechain": "yarn --cwd ../../protocol devchain run-tar .tmp/devchain.tar.gz", "test": "jest --runInBand", "lint": "tslint -c tslint.json --project ." diff --git a/packages/sdk/contractkit/src/kit.ts b/packages/sdk/contractkit/src/kit.ts index 0478f105d0c..08120e3a649 100644 --- a/packages/sdk/contractkit/src/kit.ts +++ b/packages/sdk/contractkit/src/kit.ts @@ -22,7 +22,8 @@ import { BlockchainParametersConfig } from './wrappers/BlockchainParameters' import { DowntimeSlasherConfig } from './wrappers/DowntimeSlasher' import { ElectionConfig } from './wrappers/Election' import { ExchangeConfig } from './wrappers/Exchange' -import { FederatedAttestationsConfig } from './wrappers/FederatedAttestations' +// TODO ASv2 +// import { FederatedAttestationsConfig } from './wrappers/FederatedAttestations' import { GasPriceMinimumConfig } from './wrappers/GasPriceMinimum' import { GovernanceConfig } from './wrappers/Governance' import { GrandaMentoConfig } from './wrappers/GrandaMento' @@ -86,7 +87,7 @@ export interface NetworkConfig { election: ElectionConfig attestations: AttestationsConfig // TODO ASv2 - federatedattestations: FederatedAttestationsConfig + // federatedattestations: FederatedAttestationsConfig governance: GovernanceConfig lockedGold: LockedGoldConfig sortedOracles: SortedOraclesConfig @@ -151,7 +152,7 @@ export class ContractKit { CeloContract.Election, CeloContract.Attestations, // TODO ASv2 - CeloContract.FederatedAttestations, + // CeloContract.FederatedAttestations, CeloContract.Governance, CeloContract.LockedGold, CeloContract.SortedOracles, diff --git a/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts b/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts index dbb665ce97a..1400ac09a42 100644 --- a/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts +++ b/packages/sdk/contractkit/src/wrappers/FederatedAttestations.ts @@ -1,8 +1,8 @@ import { FederatedAttestations } from '../generated/FederatedAttestations' import { BaseWrapper } from './BaseWrapper' -export interface FederatedAttestationsConfig { - // TODO ASv2 -} +// TODO ASv2 -- add params or delete config if there are none +// & delete other commented-out usages of this Config in CK +// export interface FederatedAttestationsConfig {} export class FederatedAttestationsWrapper extends BaseWrapper {} diff --git a/packages/sdk/identity/package.json b/packages/sdk/identity/package.json index 9a253faf4a7..4a0fb97123a 100644 --- a/packages/sdk/identity/package.json +++ b/packages/sdk/identity/package.json @@ -18,7 +18,7 @@ "build": "tsc -b .", "clean": "tsc -b . --clean", "docs": "typedoc", - "test:reset": "yarn --cwd ../../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../../dev-utils/src/migration-override.json --upto 25", + "test:reset": "yarn --cwd ../../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../../dev-utils/src/migration-override.json --upto 26", "test:livechain": "yarn --cwd ../../protocol devchain run-tar .tmp/devchain.tar.gz", "test": "jest --runInBand", "lint": "tslint -c tslint.json --project .", diff --git a/packages/sdk/transactions-uri/package.json b/packages/sdk/transactions-uri/package.json index bed0f54dfbd..32eda67b3ca 100644 --- a/packages/sdk/transactions-uri/package.json +++ b/packages/sdk/transactions-uri/package.json @@ -17,7 +17,7 @@ "build": "tsc -b .", "clean": "tsc -b . --clean", "docs": "typedoc", - "test:reset": "yarn --cwd ../../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../../dev-utils/src/migration-override.json --upto 25", + "test:reset": "yarn --cwd ../../protocol devchain generate-tar .tmp/devchain.tar.gz --migration_override ../../dev-utils/src/migration-override.json --upto 26", "test:livechain": "yarn --cwd ../../protocol devchain run-tar .tmp/devchain.tar.gz", "test": "jest --runInBand", "lint": "tslint -c tslint.json --project .", From 2bcfadb22cd0839a163a1c0a8541e03b528f84a7 Mon Sep 17 00:00:00 2001 From: Eela Nagaraj <7308464+eelanagaraj@users.noreply.github.com> Date: Wed, 20 Apr 2022 06:55:00 +0200 Subject: [PATCH 10/12] Fix compile errors in initial SC implementation (#9472) * Fix compilation errors * Fix delete such that it compiles * Add TODO for revokeSigner Co-authored-by: Isabelle Wei --- .../identity/FederatedAttestations.sol | 134 +++++++++++++----- 1 file changed, 101 insertions(+), 33 deletions(-) diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index edeb27089b9..c5a259ef038 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -1,4 +1,6 @@ pragma solidity ^0.5.13; +// TODO ASv2 come back to this and possibly flatten structs as arg params +pragma experimental ABIEncoderV2; import "openzeppelin-solidity/contracts/math/SafeMath.sol"; import "openzeppelin-solidity/contracts/ownership/Ownable.sol"; @@ -68,94 +70,160 @@ contract FederatedAttestations is return (1, 1, 0, 0); } - function _isRevoked(address signer, uint256 time) internal returns (bool) { + function _isRevoked(address signer, uint256 time) internal view returns (bool) { if (revokedSigners[signer] > 0 && revokedSigners[signer] >= time) { return true; } return false; } - function lookupAttestations(string identifier, address[] trustedIssuers) - public - view - returns (Attestation[]) - { - Attestation[] memory attestations = new Attestation[]; + function lookupAttestations( + bytes32 identifier, + address[] memory trustedIssuers, + uint256 maxAttestations + ) public view returns (Attestation[] memory) { + // Cannot dynamically allocate an in-memory array + // For now require a max returned parameter to pre-allocate and then shrink + // TODO ASv2 probably need a more gas-efficient lookup for the single most-recent attestation for one trusted issuer + uint256 currIndex = 0; + Attestation[] memory attestations = new Attestation[](maxAttestations); for (uint256 i = 0; i < trustedIssuers.length; i++) { address trustedIssuer = trustedIssuers[i]; for (uint256 j = 0; j < identifierToAddresses[identifier][trustedIssuer].length; j++) { - Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer][j]; - if (!_isRevoked(attestation.signer, attestation.issuedOn)) { - attestations.push(attestation); + // Only create and push new attestation if we haven't hit max + if (currIndex < maxAttestations) { + Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer][j]; + if (!_isRevoked(attestation.signer, attestation.issuedOn)) { + attestations[currIndex] = attestation; + currIndex++; + } + } else { + break; } } } - return attestations; + if (currIndex < maxAttestations) { + Attestation[] memory trimmedAttestations = new Attestation[](currIndex); + for (uint256 i = 0; i < currIndex; i++) { + trimmedAttestations[i] = attestations[i]; + } + return trimmedAttestations; + } else { + return attestations; + } } - function lookupIdentifiersbyAddress(address account, address[] trustedIssuers) - public - view - returns (bytes32[]) - { - bytes32[] memory identifiers = new bytes32[]; + function lookupIdentifiersbyAddress( + address account, + address[] memory trustedIssuers, + uint256 maxIdentifiers + ) public view returns (bytes32[] memory) { + // 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++) { address trustedIssuer = trustedIssuers[i]; for (uint256 j = 0; j < addressToIdentifiers[account][trustedIssuer].length; j++) { - bytes32 memory identifier = addressToIdentifiers[account][trustedIssuer][j]; - Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer]; - if (!_isRevoked(attestation.signer, attestation.issuedOn)) { - identifiers.push(identifier); + // Iterate through the list of identifiers + if (currIndex < maxIdentifiers) { + bytes32 identifier = addressToIdentifiers[account][trustedIssuer][j]; + // Check if this signer for this particular signer is revoked + for (uint256 k = 0; k < identifierToAddresses[identifier][trustedIssuer].length; k++) { + Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer][k]; + // For now, just take the first published, unrevoked signer that matches + // TODO redo this to take into account either recency or the "correct" identifier + // based on the index + if ( + attestation.account == account && + !_isRevoked(attestation.signer, attestation.issuedOn) + ) { + identifiers[currIndex] = identifier; + currIndex++; + break; + } + } + } else { + break; } } } - return identifiers; + if (currIndex < maxIdentifiers) { + // Allocate and fill properly-sized array + bytes32[] memory trimmedIdentifiers = new bytes32[](currIndex); + for (uint256 i = 0; i < currIndex; i++) { + trimmedIdentifiers[i] = identifiers[i]; + } + return trimmedIdentifiers; + } else { + return identifiers; + } } function validateAttestation( bytes32 identifier, address issuer, - Attestation attestation, + Attestation memory attestation, uint8 v, bytes32 r, bytes32 s ) public view returns (address) {} - function registerAttestation(bytes32 identifier, address issuer, Attestation attestation) public { + function registerAttestation(bytes32 identifier, address issuer, Attestation memory attestation) + public + { require( msg.sender == attestation.account || msg.sender == issuer || msg.sender == attestation.signer ); for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i++) { - require(identifierToAddresses[identifier][issuer][i] != attestation.account); + // TODO revisit this: do we only want to check that the account address is not duplicated? + require(identifierToAddresses[identifier][issuer][i].account != attestation.account); } identifierToAddresses[identifier][issuer].push(attestation); - addressToIdentifiers[attestation.account][issuer] = identifier; + addressToIdentifiers[attestation.account][issuer].push(identifier); } - function deleteAttestation(bytes32 identifier, address issuer, address account) public { - require(msg.sender == attestation.account || msg.sender == issuer); + function deleteAttestation(bytes32 identifier, address issuer, address account, uint256 issuedOn) + public + { + // TODO ASv2 this should short-circuit, but double check (i.e. succeeds if msg.sender == account) + require( + msg.sender == account || getAccounts().attestationSignerToAccount(msg.sender) == issuer + ); + // TODO ASv2 need to revisit all of this once we have the invariants set between + // identifier -> addresses and reverse mapping + for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i++) { - if (identifierToAddresses[identifier][issuer][i].account == account) { + Attestation memory attestation = identifierToAddresses[identifier][issuer][i]; + if (attestation.account == account && attestation.issuedOn == issuedOn) { + // Delete only first matching attestation + // TODO ASv2 revisit: alternatively, for not just first match could while loop on attestation.account == account and keep swapping in the last element, then break...weird tho identifierToAddresses[identifier][issuer][i] = identifierToAddresses[identifier][issuer][identifierToAddresses[identifier][issuer] .length - 1]; identifierToAddresses[identifier][issuer].pop(); - for (uint256 i = 0; i < addressToIdentifiers[account][issuer].length; i++) { - if (addressToIdentifiers[account][issuer][i].account == account) { - addressToIdentifiers[account][issuer][i] = addressToIdentifiers[account][issuer][addressToIdentifiers[account][issuer] + + bool deletedIdentifier = false; + for (uint256 j = 0; j < addressToIdentifiers[account][issuer].length; j++) { + // Delete only first matching identifier + if (addressToIdentifiers[account][issuer][j] == identifier) { + addressToIdentifiers[account][issuer][j] = addressToIdentifiers[account][issuer][addressToIdentifiers[account][issuer] .length - 1]; addressToIdentifiers[account][issuer].pop(); + deletedIdentifier = true; break; } } + // Hard requirement to delete from both mappings in unison + require(deletedIdentifier); break; } } } function revokeSigner(address signer, uint256 revokedOn) public { - // TODO: add permissions for who can revoke + // TODO ASv2 add constraints on who can revoke a signer revokedSigners[signer] = revokedOn; } } From ef9c2e87625e0c1f52759b35b766634f331510e2 Mon Sep 17 00:00:00 2001 From: Isabelle Wei Date: Wed, 20 Apr 2022 11:17:23 -0400 Subject: [PATCH 11/12] small function name capitalization --- packages/protocol/contracts/identity/FederatedAttestations.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index edeb27089b9..c668d2eedc2 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -93,7 +93,7 @@ contract FederatedAttestations is return attestations; } - function lookupIdentifiersbyAddress(address account, address[] trustedIssuers) + function lookupIdentifiersByAddress(address account, address[] trustedIssuers) public view returns (bytes32[]) From a3cd6dd0d15c6273efa61ede98a053a8b51ed56d Mon Sep 17 00:00:00 2001 From: Isabelle Wei Date: Thu, 21 Apr 2022 18:47:19 -0400 Subject: [PATCH 12/12] include PR comments --- .../identity/FederatedAttestations.sol | 53 +++++++++++-------- .../migrations/27_federated_attestations.ts | 15 ------ 2 files changed, 30 insertions(+), 38 deletions(-) delete mode 100644 packages/protocol/migrations/27_federated_attestations.ts diff --git a/packages/protocol/contracts/identity/FederatedAttestations.sol b/packages/protocol/contracts/identity/FederatedAttestations.sol index 2cad043eb36..9894576e5b2 100644 --- a/packages/protocol/contracts/identity/FederatedAttestations.sol +++ b/packages/protocol/contracts/identity/FederatedAttestations.sol @@ -1,3 +1,4 @@ +// TODO figure out if we can use a new solidity version for just this one contract pragma solidity ^0.5.13; // TODO ASv2 come back to this and possibly flatten structs as arg params pragma experimental ABIEncoderV2; @@ -32,13 +33,13 @@ contract FederatedAttestations is using SafeMath for uint256; using SafeCast for uint256; - struct Attestation { + struct IdentifierOwnershipAttestation { address account; uint256 issuedOn; address signer; } // identifier -> issuer -> attestations - mapping(bytes32 => mapping(address => Attestation[])) public identifierToAddresses; + mapping(bytes32 => mapping(address => IdentifierOwnershipAttestation[])) public identifierToAddresses; // account -> issuer -> identifiers mapping(address => mapping(address => bytes32[])) public addressToIdentifiers; // signer => revocation time @@ -81,18 +82,20 @@ contract FederatedAttestations is bytes32 identifier, address[] memory trustedIssuers, uint256 maxAttestations - ) public view returns (Attestation[] memory) { + ) public view returns (IdentifierOwnershipAttestation[] memory) { // Cannot dynamically allocate an in-memory array // For now require a max returned parameter to pre-allocate and then shrink // TODO ASv2 probably need a more gas-efficient lookup for the single most-recent attestation for one trusted issuer uint256 currIndex = 0; - Attestation[] memory attestations = new Attestation[](maxAttestations); + IdentifierOwnershipAttestation[] memory attestations = new IdentifierOwnershipAttestation[]( + maxAttestations + ); for (uint256 i = 0; i < trustedIssuers.length; i++) { address trustedIssuer = trustedIssuers[i]; for (uint256 j = 0; j < identifierToAddresses[identifier][trustedIssuer].length; j++) { // Only create and push new attestation if we haven't hit max if (currIndex < maxAttestations) { - Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer][j]; + IdentifierOwnershipAttestation memory attestation = identifierToAddresses[identifier][trustedIssuer][j]; if (!_isRevoked(attestation.signer, attestation.issuedOn)) { attestations[currIndex] = attestation; currIndex++; @@ -103,7 +106,9 @@ contract FederatedAttestations is } } if (currIndex < maxAttestations) { - Attestation[] memory trimmedAttestations = new Attestation[](currIndex); + IdentifierOwnershipAttestation[] memory trimmedAttestations = new IdentifierOwnershipAttestation[]( + currIndex + ); for (uint256 i = 0; i < currIndex; i++) { trimmedAttestations[i] = attestations[i]; } @@ -130,7 +135,7 @@ contract FederatedAttestations is bytes32 identifier = addressToIdentifiers[account][trustedIssuer][j]; // Check if this signer for this particular signer is revoked for (uint256 k = 0; k < identifierToAddresses[identifier][trustedIssuer].length; k++) { - Attestation memory attestation = identifierToAddresses[identifier][trustedIssuer][k]; + IdentifierOwnershipAttestation memory attestation = identifierToAddresses[identifier][trustedIssuer][k]; // For now, just take the first published, unrevoked signer that matches // TODO redo this to take into account either recency or the "correct" identifier // based on the index @@ -163,49 +168,51 @@ contract FederatedAttestations is function validateAttestation( bytes32 identifier, address issuer, - Attestation memory attestation, + IdentifierOwnershipAttestation memory attestation, uint8 v, bytes32 r, bytes32 s - ) public view returns (address) {} + ) public view returns (address) { + // TODO check if signer is revoked and is a valid signer of the account + } - function registerAttestation(bytes32 identifier, address issuer, Attestation memory attestation) - public - { + function registerAttestation( + bytes32 identifier, + address issuer, + IdentifierOwnershipAttestation memory attestation + ) public { + // TODO call validateAttestation here require( msg.sender == attestation.account || msg.sender == issuer || msg.sender == attestation.signer ); for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i++) { - // TODO revisit this: do we only want to check that the account address is not duplicated? + // 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 != attestation.account); } identifierToAddresses[identifier][issuer].push(attestation); addressToIdentifiers[attestation.account][issuer].push(identifier); } - function deleteAttestation(bytes32 identifier, address issuer, address account, uint256 issuedOn) - public - { + function deleteAttestation(bytes32 identifier, address issuer, address account) public { // TODO ASv2 this should short-circuit, but double check (i.e. succeeds if msg.sender == account) require( msg.sender == account || getAccounts().attestationSignerToAccount(msg.sender) == issuer ); - // TODO ASv2 need to revisit all of this once we have the invariants set between - // identifier -> addresses and reverse mapping for (uint256 i = 0; i < identifierToAddresses[identifier][issuer].length; i++) { - Attestation memory attestation = identifierToAddresses[identifier][issuer][i]; - if (attestation.account == account && attestation.issuedOn == issuedOn) { - // Delete only first matching attestation - // TODO ASv2 revisit: alternatively, for not just first match could while loop on attestation.account == account and keep swapping in the last element, then break...weird tho + IdentifierOwnershipAttestation memory attestation = identifierToAddresses[identifier][issuer][i]; + if (attestation.account == account) { + // This is meant to delete the attestation in the array and then move the last element in the array to that empty spot, to avoid having empty elements in the array + // Not sure if this is needed and if the added gas costs from the complexity is worth it identifierToAddresses[identifier][issuer][i] = identifierToAddresses[identifier][issuer][identifierToAddresses[identifier][issuer] .length - 1]; identifierToAddresses[identifier][issuer].pop(); + // TODO revisit if deletedIdentifier check is necessary - not sure if there would ever be a situation where the matching identifier is not present bool deletedIdentifier = false; for (uint256 j = 0; j < addressToIdentifiers[account][issuer].length; j++) { - // Delete only first matching identifier if (addressToIdentifiers[account][issuer][j] == identifier) { addressToIdentifiers[account][issuer][j] = addressToIdentifiers[account][issuer][addressToIdentifiers[account][issuer] .length - diff --git a/packages/protocol/migrations/27_federated_attestations.ts b/packages/protocol/migrations/27_federated_attestations.ts deleted file mode 100644 index ad25500c0a6..00000000000 --- a/packages/protocol/migrations/27_federated_attestations.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { CeloContractName } from '@celo/protocol/lib/registry-utils' -import { deploymentForCoreContract } from '@celo/protocol/lib/web3-utils' -import { config } from '@celo/protocol/migrationsConfig' -import { FederatedAttestationsInstance } from 'types' - -const initializeArgs = async (): Promise<[string]> => { - return [config.registry.predeployedProxyAddress] -} - -module.exports = deploymentForCoreContract( - web3, - artifacts, - CeloContractName.FederatedAttestations, - initializeArgs -)