combine global & specific used keys on DharmaKeyRegistryV2

neuromancer1729 · Oct 31, 2019 · d74234c · d74234c
1 parent d5c70d6
commit d74234c
Show file tree

Hide file tree

Showing 10 changed files with 179 additions and 44 deletions.
diff --git a/.solcover.js b/.solcover.js
@@ -35,6 +35,7 @@ module.exports = {
     'registries/DharmaKeyRegistryV1.sol',
     'test/BadBeacon.sol',
     'test/BadBeaconTwo.sol',
+    'test/DharmaTestingMultisig.sol',
     'test/TimelockTwoStepOwnableTestContract.sol',
     'test/UpgradeBeaconImplementationCheck.sol'
   ]

diff --git a/README.md b/README.md
@@ -44,7 +44,7 @@ These contracts have been audited by Trail of Bits - <a href="https://github.com
 | <a href="https://etherscan.io/address/0x0000000000bda2152794ac8c76b2dc86cba57cad#code" target="_blank">DharmaKeyRingUpgradeBeacon</a>           | DharmaSmartWalletImplementationV3                                                                                                            |                                                           Dharma Key Ring                                                             |
 | <a href="https://etherscan.io/address/0x000000000067503c398f4c9652530dbc4ea95c02#code" target="_blank">DharmaUpgradeBeaconEnvoy</a>             |                                                                                                                                              | <a href="https://etherscan.io/address/0x00dd005247b300f700cfdff89c00e2acc94c7b00#code" target="_blank">DharmaKeyRingFactoryV1</a>     |
 | <a href="https://etherscan.io/address/0x000000005d7065eb9716a410070ee62d51092c98#code" target="_blank">DharmaKeyRegistryV1</a>                  |                                                               Dharma Key Ring                                                                | <a href="https://etherscan.io/address/0x2484000059004afb720000dc738434fa6200f49d#code" target="_blank">DharmaKeyRingFactoryV2</a>     |
-| <a href="https://etherscan.io/address/0x00000000ea4044b4b60091836a6d47554f43a100#code" target="_blank">DharmaKeyRegistryV2</a>                  | <a href="https://etherscan.io/address/0x00000000480003d5ee4f51134ce73cc9ac00f693#code" target="_blank">AdharmaKeyRingImplementation</a>      | DharmaKeyRingFactoryV3                                                                                                                |
+| <a href="https://etherscan.io/address/0x00000000ea4044b4b60091836a6d47554f43a100#code" target="_blank">DharmaKeyRegistryV2</a>                  | <a href="https://etherscan.io/address/0x000000000d38df53b45c5733c7b34000de0bdf52#code" target="_blank">AdharmaKeyRingImplementation</a>      | DharmaKeyRingFactoryV3                                                                                                                |
 | <a href="https://etherscan.io/address/0x00000000004cda75701eea02d1f2f9bdce54c10d#code" target="_blank">DharmaAccountRecoveryManager</a>         | <a href="https://etherscan.io/address/0x00000000ea0007cdab60e07437e59e9cc41c2e49#code" target="_blank">DharmaKeyRingImplementationV1</a>     |                                                                                                                                       |
 
 ## Overview

diff --git a/contracts/factories/smart-wallet/DharmaSmartWalletFactoryV2.sol b/contracts/factories/smart-wallet/DharmaSmartWalletFactoryV2.sol
@@ -30,7 +30,7 @@ contract DharmaSmartWalletFactoryV2 is DharmaSmartWalletFactoryV2Interface {
 
   // DharmaKeyRegistryV2 holds a public key for verifying meta-transactions.
   DharmaKeyRegistryInterface internal constant _DHARMA_KEY_REGISTRY = (
-    DharmaKeyRegistryInterface(0x00000000EA4044b4b60091836A6D47554F43a100)
+    DharmaKeyRegistryInterface(0x000000000D38df53b45C5733c7b34000dE0BDF52)
   );
 
   // Use the smart wallet instance runtime code hash to verify expected targets.

diff --git a/contracts/implementations/smart-wallet/DharmaSmartWalletImplementationV3.sol b/contracts/implementations/smart-wallet/DharmaSmartWalletImplementationV3.sol
@@ -63,7 +63,7 @@ contract DharmaSmartWalletImplementationV3 is
 
   // DharmaKeyRegistryV2 holds a public key for verifying meta-transactions.
   DharmaKeyRegistryInterface internal constant _DHARMA_KEY_REGISTRY = (
-    DharmaKeyRegistryInterface(0x00000000EA4044b4b60091836A6D47554F43a100)
+    DharmaKeyRegistryInterface(0x000000000D38df53b45C5733c7b34000dE0BDF52)
   );
 
   // Account recovery is facilitated using a hard-coded recovery manager,

diff --git a/contracts/implementations/smart-wallet/DharmaSmartWalletImplementationV4.sol b/contracts/implementations/smart-wallet/DharmaSmartWalletImplementationV4.sol
@@ -66,7 +66,7 @@ contract DharmaSmartWalletImplementationV4 is
 
   // The Dharma Key Registry holds a public key for verifying meta-transactions.
   DharmaKeyRegistryInterface internal constant _DHARMA_KEY_REGISTRY = (
-    DharmaKeyRegistryInterface(0x00000000EA4044b4b60091836A6D47554F43a100)
+    DharmaKeyRegistryInterface(0x000000000D38df53b45C5733c7b34000dE0BDF52)
   );
 
   // Account recovery is facilitated using a hard-coded recovery manager,

diff --git a/contracts/multisigs/DharmaKeyRegistryMultisig.sol b/contracts/multisigs/DharmaKeyRegistryMultisig.sol
@@ -21,7 +21,7 @@ contract DharmaKeyRegistryMultisig {
 
   // V2 of the Dharma Key Registry is the only account the multisig can call.
   address private constant _DESTINATION = address(
-    0x00000000EA4044b4b60091836A6D47554F43a100
+    0x000000000D38df53b45C5733c7b34000dE0BDF52
   );
 
   // The threshold is an exact number of valid signatures that must be supplied.

diff --git a/contracts/registries/DharmaKeyRegistryV2.sol b/contracts/registries/DharmaKeyRegistryV2.sol
@@ -29,11 +29,8 @@ contract DharmaKeyRegistryV2 is TwoStepOwnable, DharmaKeyRegistryInterface {
   // Specific keys may also be set on a per-caller basis.
   mapping (address => address) private _specificKeys;
 
-  // Maintain a mapping of all used global keys (to prevent reuse).
-  mapping (address => bool) private _usedGlobalKeys;
-
-  // Maintain a mapping of all used specific keys per user (to prevent reuse).
-  mapping (address => mapping(address => bool)) private _usedSpecificKeys;
+  // Maintain a mapping of all used keys (to prevent reuse).
+  mapping (address => bool) private _usedKeys;
 
   /**
    * @notice In the constructor, set the initial global key and the initial
@@ -90,20 +87,17 @@ contract DharmaKeyRegistryV2 is TwoStepOwnable, DharmaKeyRegistryInterface {
   function setSpecificKey(
     address account, address specificKey
   ) external onlyOwner {
-    // Ensure that the specific key has not been used previously.
-    require(
-      !_usedSpecificKeys[account][specificKey],
-      "Key has been used previously for this specific account."
-    );
+    // Ensure that the key has not been used previously.
+    require(!_usedKeys[specificKey], "Key has been used previously.");
 
     // Emit an event signifying that the specific key has been modified.
     emit NewSpecificKey(account, _specificKeys[account], specificKey);
 
     // Update specific key for provided account to the provided specific key.
     _specificKeys[account] = specificKey;
 
-    // Mark the specific key as having been used previously.
-    _usedSpecificKeys[account][specificKey] = true;
+    // Mark the key as having been used previously.
+    _usedKeys[specificKey] = true;
   }
 
   /**
@@ -175,8 +169,8 @@ contract DharmaKeyRegistryV2 is TwoStepOwnable, DharmaKeyRegistryInterface {
    * @param globalKey address The new global public key.
    */
   function _registerGlobalKey(address globalKey) internal {
-    // Ensure that the global key has not been used previously.
-    require(!_usedGlobalKeys[globalKey], "Key has been used previously.");
+    // Ensure that the key has not been used previously.
+    require(!_usedKeys[globalKey], "Key has been used previously.");
 
     // Emit an event signifying that the global key has been modified.
     emit NewGlobalKey(_globalKey, globalKey);
@@ -185,6 +179,6 @@ contract DharmaKeyRegistryV2 is TwoStepOwnable, DharmaKeyRegistryInterface {
     _globalKey = globalKey;
 
     // Mark the key as having been used previously.
-    _usedGlobalKeys[globalKey] = true;
+    _usedKeys[globalKey] = true;
   }
 }
diff --git a/scripts/test/constants.js b/scripts/test/constants.js
@@ -195,10 +195,10 @@ module.exports = Object.freeze({
     ['202020202020446861726d614b65795265676973747279563120202020202020']
   ),
   KEY_REGISTRY_V2_ADDRESS: (
-    '0x00000000EA4044b4b60091836A6D47554F43a100'
+    '0x000000000D38df53b45C5733c7b34000dE0BDF52'
   ),
   KEY_REGISTRY_V2_SALT: (
-    '0x000000000000000000000000000000000000000052160c623e62a80d09010000'
+    '0x0000000000000000000000000000000000000000035383a4848e750d83000000'
   ),
   KEY_REGISTRY_V2_METADATA: (
     ['202020202020446861726d614b65795265676973747279563220202020202020']

diff --git a/scripts/test/deploy.js b/scripts/test/deploy.js
@@ -2007,31 +2007,188 @@ module.exports = {test: async function (provider, testingContext) {
     'deploy'
   )
 
-  /*
+  await runTest(
+    `DharmaUpgradeMultisig contract deployment fails if threshold is not met`,
+    DharmaUpgradeMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000001'
+    ]],
+    false
+  )
+
+  await runTest(
+    `DharmaUpgradeMultisig contract deployment fails if sigs are out of order`,
+    DharmaUpgradeMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000005',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000001'
+    ]],
+    false
+  )
+
+  await runTest(
+    `DharmaUpgradeMultisig contract deployment fails with too many owners`,
+    DharmaUpgradeMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000001',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000005',
+      '0x0000000000000000000000000000000000000006',
+      '0x0000000000000000000000000000000000000007',
+      '0x0000000000000000000000000000000000000008',
+      '0x0000000000000000000000000000000000000009',
+      '0x000000000000000000000000000000000000000a',
+      '0x000000000000000000000000000000000000000b'
+    ]],
+    false
+  )
+
   const DharmaUpgradeMultisig = await runTest(
     `DharmaUpgradeMultisig contract deployment`,
     DharmaUpgradeMultisigDeployer,
     '',
     'deploy',
-    [[address], 1]
+    [[
+      '0x0000000000000000000000000000000000000001',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000005'
+    ]]
+  )
+
+  await runTest(
+    `DharmaAccountRecoveryMultisig contract deployment fails if threshold is not met`,
+    DharmaAccountRecoveryMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000001'
+    ]],
+    false
+  )
+
+  await runTest(
+    `DharmaAccountRecoveryMultisig contract deployment fails if sigs are out of order`,
+    DharmaAccountRecoveryMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000005',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000001'
+    ]],
+    false
+  )
+
+  await runTest(
+    `DharmaAccountRecoveryMultisig contract deployment fails with too many owners`,
+    DharmaAccountRecoveryMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000001',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000005',
+      '0x0000000000000000000000000000000000000006',
+      '0x0000000000000000000000000000000000000007',
+      '0x0000000000000000000000000000000000000008',
+      '0x0000000000000000000000000000000000000009',
+      '0x000000000000000000000000000000000000000a',
+      '0x000000000000000000000000000000000000000b'
+    ]],
+    false
   )
 
   const DharmaAccountRecoveryMultisig = await runTest(
     `DharmaAccountRecoveryMultisig contract deployment`,
     DharmaAccountRecoveryMultisigDeployer,
     '',
     'deploy',
-    [[address], 1]
+    [[
+      '0x0000000000000000000000000000000000000001',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000005'
+    ]]
+  )
+
+  await runTest(
+    `DharmaKeyRegistryMultisig contract deployment fails if threshold is not met`,
+    DharmaKeyRegistryMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000001'
+    ]],
+    false
+  )
+
+  await runTest(
+    `DharmaKeyRegistryMultisig contract deployment fails if sigs are out of order`,
+    DharmaKeyRegistryMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000005',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000001'
+    ]],
+    false
+  )
+
+  await runTest(
+    `DharmaKeyRegistryMultisig contract deployment fails with too many owners`,
+    DharmaKeyRegistryMultisigDeployer,
+    '',
+    'deploy',
+    [[
+      '0x0000000000000000000000000000000000000001',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000005',
+      '0x0000000000000000000000000000000000000006',
+      '0x0000000000000000000000000000000000000007',
+      '0x0000000000000000000000000000000000000008',
+      '0x0000000000000000000000000000000000000009',
+      '0x000000000000000000000000000000000000000a',
+      '0x000000000000000000000000000000000000000b'
+    ]],
+    false
   )
 
   const DharmaKeyRegistryMultisig = await runTest(
     `DharmaKeyRegistryMultisig contract deployment`,
     DharmaKeyRegistryMultisigDeployer,
     '',
     'deploy',
-    [[address], 1]
+    [[
+      '0x0000000000000000000000000000000000000001',
+      '0x0000000000000000000000000000000000000002',
+      '0x0000000000000000000000000000000000000003',
+      '0x0000000000000000000000000000000000000004',
+      '0x0000000000000000000000000000000000000005'
+    ]]
   )
-  */
 
   const IndestructibleRegistry = await runTest(
     `IndestructibleRegistry contract deployment`,

diff --git a/scripts/test/test.js b/scripts/test/test.js
@@ -2945,23 +2945,6 @@ module.exports = {test: async function (provider, testingContext) {
     }
   )
 
-  await runTest(
-    'V3 UserSmartWallet can get next custom action ID to set a user signing key',
-    UserSmartWalletV3,
-    'getNextCustomActionID',
-    'call',
-    [
-      1, // SetUserSigningKey,
-      constants.FULL_APPROVAL, // This value shouldn't matter
-      addressTwo,
-      0
-    ],
-    true,
-    value => {
-      customActionId = value
-    }
-  )
-
   let currentNonce
   await runTest(
     'UserSmartWallet can get the nonce',