Legacymarket audit fixes

.circleci/config.template.yml

@@ -1,7 +1,7 @@
 # DO NOT EDIT MANUALLY! instead run `npm run build:ci`
 # autogenerated by `.circleci/pack.js` from contents of `jobs` .yml files
 version: 2.1
-orbs: 
+orbs:
   rust: circleci/[email protected]
 commands:
   {{> commands}}

.circleci/config.yml

@@ -1,7 +1,7 @@
 # DO NOT EDIT MANUALLY! instead run `npm run build:ci`
 # autogenerated by `.circleci/pack.js` from contents of `jobs` .yml files
 version: 2.1
-orbs: 
+orbs:
   rust: circleci/[email protected]
 commands:
   cmd-wait-for-port:
@@ -18,7 +18,7 @@ jobs:
   job-audit:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-sec-tools:16.17-ubuntu
+      - image: synthetixio/docker-sec-tools:18.19-ubuntu
     steps:
       - checkout
       - run:
@@ -32,7 +32,7 @@ jobs:
   job-cannon:
     working_directory: ~/repo
     machine:
-      image: ubuntu-2204:2022.04.1
+      image: ubuntu-2204:current
       docker_layer_caching: true
     environment:
       foundry_cache_version: "1"
@@ -96,7 +96,7 @@ jobs:
   job-fork-tests-ovm:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     resource_class: large
     steps:
       - checkout
@@ -123,7 +123,7 @@ jobs:
   job-fork-tests:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     resource_class: large
     steps:
       - checkout
@@ -150,7 +150,7 @@ jobs:
   job-integration-tests:
     working_directory: ~/repo
     machine:
-      image: ubuntu-2204:2022.04.1
+      image: ubuntu-2204:current
       docker_layer_caching: true
     environment:
       foundry_cache_version: "1"
@@ -248,7 +248,7 @@ jobs:
   job-lint:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -257,7 +257,7 @@ jobs:
   job-pack-browser:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -268,7 +268,7 @@ jobs:
   job-prepare:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - run:
           name: Set node & npm versions
@@ -299,7 +299,7 @@ jobs:
   job-simulate-release:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -323,7 +323,7 @@ jobs:
   job-static-analysis:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -340,7 +340,7 @@ jobs:
   job-test-deploy-script:
     working_directory: ~/repo
     machine:
-      image: ubuntu-2204:2022.04.1
+      image: ubuntu-2204:current
       docker_layer_caching: true
     environment:
       foundry_cache_version: "1"
@@ -393,7 +393,7 @@ jobs:
   job-unit-tests-coverage-report:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-sec-tools:16.17-ubuntu
+      - image: synthetixio/docker-sec-tools:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -406,7 +406,7 @@ jobs:
   job-unit-tests-coverage:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     resource_class: large
     parallelism: 8
     steps:
@@ -433,7 +433,7 @@ jobs:
   job-unit-tests-gas-report:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -451,7 +451,7 @@ jobs:
   job-unit-tests:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     resource_class: large
     parallelism: 8
     steps:
@@ -484,7 +484,7 @@ jobs:
   job-validate-deployments:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     steps:
       - checkout
       - attach_workspace:
@@ -500,7 +500,7 @@ jobs:
   job-validate-etherscan:
     working_directory: ~/repo
     docker:
-      - image: synthetixio/docker-node:16.17-ubuntu
+      - image: synthetixio/docker-node:18.19-ubuntu
     parameters:
       network:
         type: string

.circleci/src/snippets/job-header-machine.yml

@@ -1,6 +1,6 @@
 working_directory: ~/repo
 machine:
-  image: ubuntu-2204:2022.04.1
+  image: ubuntu-2204:current
   docker_layer_caching: true
 environment:
   foundry_cache_version: "1"

.circleci/src/snippets/job-header-node.yml

@@ -1,3 +1,3 @@
 working_directory: ~/repo
 docker:
-  - image: synthetixio/docker-node:16.17-ubuntu
+  - image: synthetixio/docker-node:18.19-ubuntu

.circleci/src/snippets/job-header-sec-tools.yml

@@ -1,3 +1,3 @@
 working_directory: ~/repo
 docker:
-  - image: synthetixio/docker-sec-tools:16.17-ubuntu
+  - image: synthetixio/docker-sec-tools:18.19-ubuntu

.github/workflows/slither-analysis.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
 
     container:
-      image: synthetixio/docker-sec-tools:16.17-ubuntu
+      image: synthetixio/docker-sec-tools:18.19-ubuntu
 
     steps:
       - name: Checkout

contracts/BaseRewardEscrowV2.sol

@@ -24,6 +24,7 @@ contract BaseRewardEscrowV2 is Owned, IRewardEscrowV2, LimitedSetup(8 weeks), Mi
 
     /* Mapping of nominated address to recieve account merging */
     mapping(address => address) public nominatedReceiver;
+    mapping(address => bool) public permittedEscrowCreators;
 
     /* Max escrow duration */
     uint public max_duration = 2 * 52 weeks; // Default max 2 years duration
@@ -307,6 +308,10 @@ contract BaseRewardEscrowV2 is Owned, IRewardEscrowV2, LimitedSetup(8 weeks), Mi
         synthetixERC20().transfer(transferTo, amount);
     }
 
+    function setPermittedEscrowCreator(address creator, bool permitted) external onlyOwner {
+        permittedEscrowCreators[creator] = permitted;
+    }
+
     /**
      * @notice Create an escrow entry to lock SNX for a given duration in seconds
      * @dev This call expects that the depositor (msg.sender) has already approved the Reward escrow contract
@@ -318,6 +323,7 @@ contract BaseRewardEscrowV2 is Owned, IRewardEscrowV2, LimitedSetup(8 weeks), Mi
         uint256 duration
     ) external {
         require(beneficiary != address(0), "Cannot create escrow with address(0)");
+        require(permittedEscrowCreators[msg.sender], "Only permitted escrow creators can create escrow entries");
 
         /* Transfer SNX from msg.sender */
         require(synthetixERC20().transferFrom(msg.sender, address(this), deposit), "token transfer failed");

contracts/BaseSynthetix.sol

@@ -449,34 +449,28 @@ contract BaseSynthetix is IERC20, ExternStateToken, MixinResolver, ISynthetix {
         return success;
     }
 
-    /**
-     * @notice allows for migration from v2x to v3 when an account has pending escrow entries
-     */
-    function revokeAllEscrow(address account) external systemActive {
-        address legacyMarketAddress = resolver.getAddress(CONTRACT_V3_LEGACYMARKET);
-        require(msg.sender == legacyMarketAddress, "Only LegacyMarket can revoke escrow");
-        rewardEscrowV2().revokeFrom(account, legacyMarketAddress, rewardEscrowV2().totalEscrowedAccountBalance(account), 0);
-    }
-
     function migrateAccountBalances(address account)
         external
         systemActive
         returns (uint totalEscrowRevoked, uint totalLiquidBalance)
     {
         address debtMigratorOnEthereum = resolver.getAddress(CONTRACT_DEBT_MIGRATOR_ON_ETHEREUM);
-        require(msg.sender == debtMigratorOnEthereum, "Only L1 DebtMigrator");
+        require(
+            msg.sender == debtMigratorOnEthereum || msg.sender == resolver.getAddress(CONTRACT_V3_LEGACYMARKET),
+            "Only L1 DebtMigrator or LegacyMarket"
+        );
 
         // get their liquid SNX balance and transfer it to the migrator contract
         totalLiquidBalance = tokenState.balanceOf(account);
         if (totalLiquidBalance > 0) {
-            bool succeeded = _transferByProxy(account, debtMigratorOnEthereum, totalLiquidBalance);
+            bool succeeded = _transferByProxy(account, msg.sender, totalLiquidBalance);
             require(succeeded, "snx transfer failed");
         }
 
         // get their escrowed SNX balance and revoke it all
         totalEscrowRevoked = rewardEscrowV2().totalEscrowedAccountBalance(account);
         if (totalEscrowRevoked > 0) {
-            rewardEscrowV2().revokeFrom(account, debtMigratorOnEthereum, totalEscrowRevoked, 0);
+            rewardEscrowV2().revokeFrom(account, msg.sender, totalEscrowRevoked, 0);
         }
     }
 

contracts/interfaces/ISynthetix.sol

@@ -140,7 +140,5 @@ interface ISynthetix {
 
     function burnSecondary(address account, uint amount) external;
 
-    function revokeAllEscrow(address account) external;
-
     function migrateAccountBalances(address account) external returns (uint totalEscrowRevoked, uint totalLiquidBalance);
 }

publish/src/commands/deploy/configure-reward-escrow.js

@@ -15,6 +15,8 @@ module.exports = async ({ addressOf, deployer, runStep }) => {
 		// Synthetix,
 		RewardEscrowV2Storage,
 		RewardEscrowV2Frozen,
+		LiquidatorRewards,
+		DebtMigratorOnOptimism,
 	} = deployer.deployedContracts;
 
 	// SIP-252 rewards escrow migration
@@ -102,4 +104,30 @@ module.exports = async ({ addressOf, deployer, runStep }) => {
 		writeArg: addressOf(RewardEscrowV2),
 		comment: 'Ensure the RewardsDistribution can read the RewardEscrowV2 address',
 	});
+
+	if (DebtMigratorOnOptimism) {
+		await runStep({
+			contract: 'RewardEscrowV2',
+			target: RewardEscrowV2,
+			read: 'permittedEscrowCreators',
+			readArg: addressOf(DebtMigratorOnOptimism),
+			expected: input => input,
+			write: 'setPermittedEscrowCreator',
+			writeArg: [addressOf(DebtMigratorOnOptimism), true],
+			comment: 'Allow escrow entry creation by DebtMigratorOnOptimism',
+		});
+	}
+
+	if (RewardEscrowV2.permittedEscrowCreators) {
+		await runStep({
+			contract: 'RewardEscrowV2',
+			target: RewardEscrowV2,
+			read: 'permittedEscrowCreators',
+			readArg: addressOf(LiquidatorRewards),
+			expected: input => input,
+			write: 'setPermittedEscrowCreator',
+			writeArg: [addressOf(LiquidatorRewards), true],
+			comment: 'Allow escrow entry creation by LiquidatorRewards',
+		});
+	}
 };

test/contracts/BaseSynthetix.js

@@ -119,7 +119,6 @@ contract('BaseSynthetix', async accounts => {
 				'mint',
 				'mintSecondary',
 				'mintSecondaryRewards',
-				'revokeAllEscrow',
 				'settle',
 				'transfer',
 				'transferFrom',
@@ -617,6 +616,7 @@ contract('BaseSynthetix', async accounts => {
 			beforeEach(async () => {
 				// give the account some balance to test with
 				await baseSynthetixProxy.transfer(account3, toUnit('200'), { from: owner });
+				await rewardEscrowV2.setPermittedEscrowCreator(owner, true, { from: owner });
 				await rewardEscrowV2.createEscrowEntry(account3, toUnit('100'), 1, { from: owner });
 
 				assert.bnEqual(await baseSynthetixImpl.collateral(account3), toUnit('300'));
@@ -643,20 +643,6 @@ contract('BaseSynthetix', async accounts => {
 			});
 		});
 
-		// SIP-299
-		describe('revokeAllEscrow', () => {
-			it('restricted to legacy market', async () => {
-				await addressResolver.importAddresses(['LegacyMarket'].map(toBytes32), [account2], {
-					from: owner,
-				});
-				await rewardEscrowV2.createEscrowEntry(account1, toUnit('100'), 1, { from: owner });
-				await assert.revert(
-					baseSynthetixImpl.revokeAllEscrow(account1, { from: owner }),
-					'Only LegacyMarket can revoke escrow'
-				);
-			});
-		});
-
 		it('should transfer when legacy market address is non-zero', async () => {
 			await addressResolver.importAddresses(['LegacyMarket'].map(toBytes32), [account2], {
 				from: owner,