Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASv2 Onchain Registry #9431

Merged
merged 14 commits into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions packages/protocol/contracts/identity/FederatedAttestations.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
pragma solidity ^0.5.13;
isabellewei marked this conversation as resolved.
Show resolved Hide resolved

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;

struct Attestation {
isabellewei marked this conversation as resolved.
Show resolved Hide resolved
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a method signature for revokingSigners? logic doesn't have to be implemented yet but would make it a bit easier/cleaner to get started on the CK wrappers IMO, lmk what you think!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also want to be able to un-revoke signers? is revoking permanent?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I think revoking is likely permanent, since the keys would be compromised? Personally think un-revoking is probably for few enough use cases that issuers should just register a new signer


// TODO ASv2 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 ASv2 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);
}

function _isRevoked(address signer, uint256 time) internal returns (bool) {
eelanagaraj marked this conversation as resolved.
Show resolved Hide resolved
if (revokedSigners[signer] > 0 && revokedSigners[signer] >= time) {
return true;
}
return false;
}

function lookupAttestations(string identifier, address[] trustedIssuers)
isabellewei marked this conversation as resolved.
Show resolved Hide resolved
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];
isabellewei marked this conversation as resolved.
Show resolved Hide resolved
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
);
isabellewei marked this conversation as resolved.
Show resolved Hide resolved
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 {
isabellewei marked this conversation as resolved.
Show resolved Hide resolved
require(msg.sender == attestation.account || msg.sender == issuer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of checking against the issuer, should we check that msg.sender is an authorized signer for issuer? Otherwise we require issuers to use their issuer key instead of delegating this to the signers, which could be insecure

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();
Comment on lines +208 to +211
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

for (uint256 i = 0; i < addressToIdentifiers[account][issuer].length; i++) {
isabellewei marked this conversation as resolved.
Show resolved Hide resolved
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;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pragma solidity ^0.5.13;

// TODO ASv2 add external, view, and only owner function sigs
// separated into these three groups for clarity
interface IFederatedAttestations {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pragma solidity ^0.5.13;

import "../../common/Proxy.sol";

/* solhint-disable no-empty-blocks */
contract FederatedAttestationsProxy is Proxy {}
3 changes: 3 additions & 0 deletions packages/protocol/lib/registry-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export enum CeloContractName {
Exchange = 'Exchange',
ExchangeEUR = 'ExchangeEUR',
ExchangeBRL = 'ExchangeBRL',
FederatedAttestations = 'FederatedAttestations',
FeeCurrencyWhitelist = 'FeeCurrencyWhitelist',
Freezer = 'Freezer',
GasPriceMinimum = 'GasPriceMinimum',
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions packages/protocol/lib/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/migrations/25_governance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ module.exports = deploymentForCoreContract<GovernanceInstance>(
'Escrow',
'Exchange',
'ExchangeEUR',
// TODO ASv2 revisit
'FeeCurrencyWhitelist',
'Freezer',
'GasPriceMinimum',
Expand Down
15 changes: 15 additions & 0 deletions packages/protocol/migrations/27_federated_attestations.ts
Original file line number Diff line number Diff line change
@@ -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<FederatedAttestationsInstance>(
web3,
artifacts,
CeloContractName.FederatedAttestations,
initializeArgs
)
2 changes: 2 additions & 0 deletions packages/protocol/migrationsConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/scripts/bash/backupmigrations.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions packages/protocol/scripts/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const ProxyContracts = [
'ExchangeBRLProxy',
'ExchangeEURProxy',
'ExchangeProxy',
'FederatedAttestationsProxy',
'FeeCurrencyWhitelistProxy',
'GasPriceMinimumProxy',
'GoldTokenProxy',
Expand Down Expand Up @@ -66,6 +67,7 @@ export const CoreContracts = [
// identity
'Attestations',
'Escrow',
'FederatedAttestations',
'Random',

// stability
Expand Down
43 changes: 43 additions & 0 deletions packages/protocol/test/identity/federatedattestations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { CeloContractName } from '@celo/protocol/lib/registry-utils'
// import { getPhoneHash } from '@celo/utils/lib/phoneNumbers'
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
assert(caller)
assert(federatedAttestations)
})
})
})
1 change: 1 addition & 0 deletions packages/sdk/contractkit/src/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export enum CeloContract {
Exchange = 'Exchange',
ExchangeEUR = 'ExchangeEUR',
ExchangeBRL = 'ExchangeBRL',
FederatedAttestations = 'FederatedAttestations',
FeeCurrencyWhitelist = 'FeeCurrencyWhitelist',
Freezer = 'Freezer',
GasPriceMinimum = 'GasPriceMinimum',
Expand Down
6 changes: 6 additions & 0 deletions packages/sdk/contractkit/src/contract-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
// }
Expand Down
5 changes: 5 additions & 0 deletions packages/sdk/contractkit/src/kit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -84,6 +85,8 @@ export interface NetworkConfig {
stableTokens: EachCeloToken<StableTokenConfig>
election: ElectionConfig
attestations: AttestationsConfig
// TODO ASv2
federatedattestations: FederatedAttestationsConfig
governance: GovernanceConfig
lockedGold: LockedGoldConfig
sortedOracles: SortedOraclesConfig
Expand Down Expand Up @@ -147,6 +150,8 @@ export class ContractKit {
const configContracts: ValidWrappers[] = [
CeloContract.Election,
CeloContract.Attestations,
// TODO ASv2
CeloContract.FederatedAttestations,
CeloContract.Governance,
CeloContract.LockedGold,
CeloContract.SortedOracles,
Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/contractkit/src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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),
Expand Down
Loading