Merge pull request #550 from graphprotocol/ariel/multicall-staged

Add multicall to staking contract + keep legacy bulk calls
graphprotocol · Mar 23, 2022 · 3538e00 · 3538e00
2 parents 3541624 + fdf526c
commit 3538e00
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 47 deletions.
diff --git a/audits/ConsenSysDiligence/2021-08-staking-multicall-and-delegation-fixes.pdf b/audits/ConsenSysDiligence/2021-08-staking-multicall-and-delegation-fixes.pdf
diff --git a/cli/commands/contracts/staking.ts b/cli/commands/contracts/staking.ts
@@ -36,7 +36,8 @@ export const allocate = async (cli: CLIEnvironment, cliArgs: CLIArgs): Promise<v
   const staking = cli.contracts.Staking
 
   logger.info(`Allocating ${cliArgs.amount} tokens on ${subgraphDeploymentID}...`)
-  await sendTransaction(cli.wallet, staking, 'allocate', [
+  await sendTransaction(cli.wallet, staking, 'allocateFrom', [
+    cli.walletAddress,
     subgraphDeploymentID,
     amount,
     allocationID,

diff --git a/contracts/staking/Staking.sol b/contracts/staking/Staking.sol
@@ -5,6 +5,7 @@ pragma abicoder v2;
 
 import "@openzeppelin/contracts/cryptography/ECDSA.sol";
 
+import "../base/Multicall.sol";
 import "../upgrades/GraphUpgradeable.sol";
 import "../utils/TokenUtils.sol";
 
@@ -20,7 +21,7 @@ import "./libs/Stakes.sol";
  * Allocations on a Subgraph. It also allows Delegators to Delegate towards an Indexer. The
  * contract also has the slashing functionality.
  */
-contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
+contract Staking is StakingV2Storage, GraphUpgradeable, IStaking, Multicall {
     using SafeMath for uint256;
     using Stakes for Stakes.Indexer;
     using Rebates for Rebates.Pool;

diff --git a/test/disputes/poi.test.ts b/test/disputes/poi.test.ts
@@ -79,7 +79,8 @@ describe('DisputeManager:POI', async () => {
       await staking.connect(indexerAccount.signer).stake(indexerTokens)
       await staking
         .connect(indexerAccount.signer)
-        .allocate(
+        .allocateFrom(
+          indexerAccount.address,
           subgraphDeploymentID,
           indexerAllocatedTokens,
           allocationID,
@@ -147,7 +148,8 @@ describe('DisputeManager:POI', async () => {
       await staking.connect(indexer.signer).stake(indexerTokens)
       const tx1 = await staking
         .connect(indexer.signer)
-        .allocate(
+        .allocateFrom(
+          indexer.address,
           subgraphDeploymentID,
           indexerAllocatedTokens,
           allocationID,

diff --git a/test/disputes/query.test.ts b/test/disputes/query.test.ts
@@ -113,7 +113,8 @@ describe('DisputeManager:Query', async () => {
       await staking.connect(indexerAccount.signer).stake(indexerTokens)
       await staking
         .connect(indexerAccount.signer)
-        .allocate(
+        .allocateFrom(
+          indexerAccount.address,
           dispute.receipt.subgraphDeploymentID,
           indexerAllocatedTokens,
           allocationID,
@@ -194,7 +195,8 @@ describe('DisputeManager:Query', async () => {
       await staking.connect(indexer.signer).stake(indexerTokens)
       const tx1 = await staking
         .connect(indexer.signer)
-        .allocate(
+        .allocateFrom(
+          indexer.address,
           dispute.receipt.subgraphDeploymentID,
           indexerAllocatedTokens,
           indexer1ChannelKey.address,

diff --git a/test/payments/allocationExchange.test.ts b/test/payments/allocationExchange.test.ts
@@ -102,7 +102,8 @@ describe('AllocationExchange', () => {
     await staking.connect(indexer.signer).stake(stakeTokens)
     await staking
       .connect(indexer.signer)
-      .allocate(
+      .allocateFrom(
+        indexer.address,
         subgraphDeploymentID,
         stakeTokens,
         allocationID,

diff --git a/test/payments/withdrawHelper.test.ts b/test/payments/withdrawHelper.test.ts
@@ -86,7 +86,8 @@ describe('WithdrawHelper', () => {
       await staking.connect(indexer.signer).stake(stakeTokens)
       await staking
         .connect(indexer.signer)
-        .allocate(
+        .allocateFrom(
+          indexer.address,
           subgraphDeploymentID,
           stakeTokens,
           allocationID,

diff --git a/test/rewards/rewards.test.ts b/test/rewards/rewards.test.ts
@@ -391,7 +391,8 @@ describe('Rewards', () => {
         await staking.connect(indexer1.signer).stake(tokensToAllocate)
         await staking
           .connect(indexer1.signer)
-          .allocate(
+          .allocateFrom(
+            indexer1.address,
             subgraphDeploymentID1,
             tokensToAllocate,
             allocationID,
@@ -428,7 +429,8 @@ describe('Rewards', () => {
         await staking.connect(indexer1.signer).stake(tokensToAllocate)
         await staking
           .connect(indexer1.signer)
-          .allocate(
+          .allocateFrom(
+            indexer1.address,
             subgraphDeploymentID1,
             tokensToAllocate,
             allocationID,
@@ -471,7 +473,8 @@ describe('Rewards', () => {
         await staking.connect(indexer1.signer).stake(tokensToAllocate)
         await staking
           .connect(indexer1.signer)
-          .allocate(
+          .allocateFrom(
+            indexer1.address,
             subgraphDeploymentID1,
             tokensToAllocate,
             allocationID,
@@ -516,7 +519,8 @@ describe('Rewards', () => {
         await staking.connect(indexer1.signer).stake(tokensToAllocate)
         await staking
           .connect(indexer1.signer)
-          .allocate(
+          .allocateFrom(
+            indexer1.address,
             subgraphDeploymentID1,
             tokensToAllocate,
             allocationID,
@@ -558,7 +562,8 @@ describe('Rewards', () => {
         // Allocate
         await staking
           .connect(indexer1.signer)
-          .allocate(
+          .allocateFrom(
+            indexer1.address,
             subgraphDeploymentID1,
             tokensToAllocate,
             allocationID,
@@ -754,7 +759,8 @@ describe('Rewards', () => {
       await staking.connect(indexer1.signer).stake(tokensToAllocate)
       await staking
         .connect(indexer1.signer)
-        .allocate(
+        .allocateFrom(
+          indexer1.address,
           subgraphDeploymentID1,
           tokensToAllocate,
           allocationID,

diff --git a/test/staking/allocation.test.ts b/test/staking/allocation.test.ts
@@ -1,5 +1,5 @@
 import { expect } from 'chai'
-import { constants, BigNumber } from 'ethers'
+import { constants, BigNumber, PopulatedTransaction } from 'ethers'
 
 import { Curation } from '../../build/types/Curation'
 import { EpochManager } from '../../build/types/EpochManager'
@@ -67,7 +67,8 @@ describe('Staking:Allocation', () => {
   const allocate = async (tokens: BigNumber) => {
     return staking
       .connect(indexer.signer)
-      .allocate(
+      .allocateFrom(
+        indexer.address,
         subgraphDeploymentID,
         tokens,
         allocationID,
@@ -206,7 +207,14 @@ describe('Staking:Allocation', () => {
     it('reject allocate with invalid allocationID', async function () {
       const tx = staking
         .connect(indexer.signer)
-        .allocate(subgraphDeploymentID, tokensToAllocate, AddressZero, metadata, randomHexBytes(20))
+        .allocateFrom(
+          indexer.address,
+          subgraphDeploymentID,
+          tokensToAllocate,
+          AddressZero,
+          metadata,
+          randomHexBytes(20),
+        )
       await expect(tx).revertedWith('!alloc')
     })
 
@@ -273,7 +281,8 @@ describe('Staking:Allocation', () => {
           const invalidProof = await channelKey.generateProof(randomHexBytes(20))
           const tx = staking
             .connect(indexer.signer)
-            .allocate(
+            .allocateFrom(
+              indexer.address,
               subgraphDeploymentID,
               tokensToAllocate,
               indexer.address,
@@ -286,7 +295,8 @@ describe('Staking:Allocation', () => {
         it('invalid proof signature format', async function () {
           const tx = staking
             .connect(indexer.signer)
-            .allocate(
+            .allocateFrom(
+              indexer.address,
               subgraphDeploymentID,
               tokensToAllocate,
               indexer.address,
@@ -631,7 +641,8 @@ describe('Staking:Allocation', () => {
       const allocationID2 = channelKey2.address
       await staking
         .connect(indexer.signer)
-        .allocate(
+        .allocateFrom(
+          indexer.address,
           subgraphDeploymentID,
           tokensToAllocate,
           allocationID2,
@@ -644,17 +655,21 @@ describe('Staking:Allocation', () => {
       await advanceToNextEpoch(epochManager)
 
       // Close multiple allocations in one tx
-      const requests = [
-        {
-          allocationID: allocationID,
-          poi: poi,
-        },
-        {
-          allocationID: allocationID2,
-          poi: poi,
-        },
-      ]
-      await staking.connect(indexer.signer).closeAllocationMany(requests)
+      const requests = await Promise.all(
+        [
+          {
+            allocationID: allocationID,
+            poi: poi,
+          },
+          {
+            allocationID: allocationID2,
+            poi: poi,
+          },
+        ].map(({ allocationID, poi }) =>
+          staking.connect(indexer.signer).populateTransaction.closeAllocation(allocationID, poi),
+        ),
+      ).then((e) => e.map((e: PopulatedTransaction) => e.data))
+      await staking.connect(indexer.signer).multicall(requests)
     })
   })
 
@@ -672,19 +687,22 @@ describe('Staking:Allocation', () => {
       // Close and allocate
       const newChannelKey = deriveChannelKey()
       const newAllocationID = newChannelKey.address
-      const tx = staking
-        .connect(indexer.signer)
-        .closeAndAllocate(
-          allocationID,
-          HashZero,
-          indexer.address,
-          subgraphDeploymentID,
-          tokensToAllocate,
-          newAllocationID,
-          metadata,
-          await newChannelKey.generateProof(indexer.address),
-        )
-      await tx
+
+      // Close multiple allocations in one tx
+      const requests = await Promise.all([
+        staking.connect(indexer.signer).populateTransaction.closeAllocation(allocationID, poi),
+        staking
+          .connect(indexer.signer)
+          .populateTransaction.allocateFrom(
+            indexer.address,
+            subgraphDeploymentID,
+            tokensToAllocate,
+            newAllocationID,
+            metadata,
+            await newChannelKey.generateProof(indexer.address),
+          ),
+      ]).then((e) => e.map((e: PopulatedTransaction) => e.data))
+      await staking.connect(indexer.signer).multicall(requests)
     })
   })
 
@@ -870,7 +888,10 @@ describe('Staking:Allocation', () => {
 
         // Claim with restake
         expect(await staking.getAllocationState(allocationID)).eq(AllocationState.Finalized)
-        await staking.connect(indexer.signer).claimMany([allocationID], true)
+        const tx = await staking
+          .connect(indexer.signer)
+          .populateTransaction.claim(allocationID, true)
+        await staking.connect(indexer.signer).multicall([tx.data])
 
         // Verify that the claimed tokens are restaked
         const afterIndexerStake = await staking.getIndexerStakedTokens(indexer.address)

diff --git a/test/staking/delegation.test.ts b/test/staking/delegation.test.ts
@@ -534,7 +534,8 @@ describe('Staking::Delegation', () => {
     const setupAllocation = async (tokens: BigNumber) => {
       return staking
         .connect(indexer.signer)
-        .allocate(
+        .allocateFrom(
+          indexer.address,
           subgraphDeploymentID,
           tokens,
           allocationID,

diff --git a/test/staking/staking.test.ts b/test/staking/staking.test.ts
@@ -53,7 +53,8 @@ describe('Staking:Stakes', () => {
   const allocate = async (tokens: BigNumber) => {
     return staking
       .connect(indexer.signer)
-      .allocate(
+      .allocateFrom(
+        indexer.address,
         subgraphDeploymentID,
         tokens,
         allocationID,