From 5cdc6e2f653e6f7fefd7a953736fcf747a4a5131 Mon Sep 17 00:00:00 2001 From: Prasad Tare Date: Tue, 14 May 2019 18:16:57 -0400 Subject: [PATCH] Slight improvements to Registry (#412) Signed-off-by: Roz Stengle --- v1/contracts/Registry.sol | 17 +++++++++++++---- v1/contracts/RegistryInterface.sol | 24 ++++++++++++++++-------- v1/migrations/4_deploy_registry.js | 9 +++++++++ v1/scripts/CheckDeploymentValidity.js | 5 +++++ 4 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 v1/migrations/4_deploy_registry.js diff --git a/v1/contracts/Registry.sol b/v1/contracts/Registry.sol index 9d5747a873..cfe3d2ba4b 100644 --- a/v1/contracts/Registry.sol +++ b/v1/contracts/Registry.sol @@ -8,6 +8,10 @@ import "openzeppelin-solidity/contracts/math/SafeMath.sol"; pragma experimental ABIEncoderV2; +/** + * @title Registry for derivatives and approved derivative creators. + * @dev Maintains a whitelist of derivative creators that are allowed to register new derivatives. + */ contract Registry is RegistryInterface, MultiRole { using SafeMath for uint; @@ -27,8 +31,7 @@ contract Registry is RegistryInterface, MultiRole { // This enum is required because a WasValid state is required to ensure that derivatives cannot be re-registered. enum PointerValidity { Invalid, - Valid, - WasValid + Valid } struct Pointer { @@ -49,6 +52,10 @@ contract Registry is RegistryInterface, MultiRole { // Maps from derivative address to the set of parties that are involved in that derivative. mapping(address => PartiesMap) private derivativesToParties; + // TODO(ptare): Used to make doubly sure that roles are initialized only once. Figure out what's going wrong with + // coverage to necessitate this hack. + bool private rolesInitialized; + constructor() public { initializeRolesOnce(); } @@ -114,10 +121,12 @@ contract Registry is RegistryInterface, MultiRole { /* * @notice Do not call this function externally. - * @dev Only called from the constructor, and only extracted to a separate method to make the coverage tool work. If - * called again, the MultiRole checks will cause a revert. + * @dev Only called from the constructor, and only extracted to a separate method to make the coverage tool work. + * Will revert if called again. */ function initializeRolesOnce() public { + require(!rolesInitialized, "Only the constructor should call this method"); + rolesInitialized = true; _createExclusiveRole(uint(Roles.Governance), uint(Roles.Governance), msg.sender); _createExclusiveRole(uint(Roles.Writer), uint(Roles.Governance), msg.sender); // Start with no derivative creators registered. diff --git a/v1/contracts/RegistryInterface.sol b/v1/contracts/RegistryInterface.sol index ed3ba37d10..2714282464 100644 --- a/v1/contracts/RegistryInterface.sol +++ b/v1/contracts/RegistryInterface.sol @@ -1,27 +1,35 @@ -/* - Registry Interface -*/ pragma solidity ^0.5.0; pragma experimental ABIEncoderV2; +/** + * @title Interface for a registry of derivatives and derivative creators + */ interface RegistryInterface { struct RegisteredDerivative { address derivativeAddress; address derivativeCreator; } - // Registers a new derivative. Only authorized derivative creators can call this method. + /** + * @dev Registers a new derivative. Only authorized derivative creators can call this method. + */ function registerDerivative(address[] calldata counterparties, address derivativeAddress) external; - // Returns whether the derivative has been registered with the registry (and is therefore an authorized participant - // in the UMA system). + /** + * @dev Returns whether the derivative has been registered with the registry (and is therefore an authorized + * participant in the UMA system). + */ function isDerivativeRegistered(address derivative) external view returns (bool isRegistered); - // Returns a list of all derivatives that are associated with a particular party. + /** + * @dev Returns a list of all derivatives that are associated with a particular party. + */ function getRegisteredDerivatives(address party) external view returns (RegisteredDerivative[] memory derivatives); - // Returns all registered derivatives. + /** + * @dev Returns all registered derivatives. + */ function getAllRegisteredDerivatives() external view returns (RegisteredDerivative[] memory derivatives); } diff --git a/v1/migrations/4_deploy_registry.js b/v1/migrations/4_deploy_registry.js new file mode 100644 index 0000000000..3f248b4adf --- /dev/null +++ b/v1/migrations/4_deploy_registry.js @@ -0,0 +1,9 @@ +const Registry = artifacts.require("Registry"); +const { getKeysForNetwork, deployAndGet, addToTdr } = require("../../common/MigrationUtils.js"); + +module.exports = async function(deployer, network, accounts) { + const keys = getKeysForNetwork(network, accounts); + + const registry = await deployAndGet(deployer, Registry, { from: keys.deployer }); + await addToTdr(registry, network); +}; diff --git a/v1/scripts/CheckDeploymentValidity.js b/v1/scripts/CheckDeploymentValidity.js index 1fc0f58375..cb7e61e984 100644 --- a/v1/scripts/CheckDeploymentValidity.js +++ b/v1/scripts/CheckDeploymentValidity.js @@ -1,4 +1,5 @@ const Migrations = artifacts.require("Migrations"); +const Registry = artifacts.require("Registry"); const checkDeploymentValidity = async function(callback) { try { @@ -9,6 +10,10 @@ const checkDeploymentValidity = async function(callback) { const migrations = await Migrations.deployed(); await migrations.last_completed_migration(); + // Registry + const registry = await Registry.deployed(); + await registry.getAllRegisteredDerivatives(); + console.log("Deployment looks good!"); } catch (e) { // Forces the script to return a nonzero error code so failure can be detected in bash.