Merge pull request #4 from pooltogether/upkeep_multiple_performs_per_…

…block

Upkeep multiple performs per block

README.md

@@ -3,7 +3,7 @@
 [![Coverage Status](https://coveralls.io/repos/github/pooltogether/pooltogether-operations-contracts/badge.svg?branch=main)](https://coveralls.io/github/pooltogether/pooltogether-operations-contracts?branch=main)
 ![Tests](https://github.com/pooltogether/pooltogether-prizepool-upkeep/actions/workflows/main.yml/badge.svg)
 
-PoolTogether Operations contracts is PoolTogether's integration with ChainLinks upkeep system.
+PoolTogether Operations contracts is PoolTogether's integration with ChainLinks upkeep system, [currently in beta](https://docs.chain.link/docs/kovan-keeper-network-beta/).
 
 ## How it works
 
@@ -13,10 +13,9 @@ A registry of these prize pools exists (as an Ownable MappedSinglyLinkedList) an
 
 If upkeep is required then either `startAward()` or `completeAward()` are called on the prize pool. 
 
-To prevent out-of-gas situations, a prize pool upkeep batch size is defined in the constructor. 
-
-The upkeepers performing the upkeep are compensated in LINK so the PrizeStrategyUpkeep contact needs to maintain a balance of LINK. 
+To prevent out-of-gas situations, an owner updatable `upkeepBatchSize` is maintained to prevent attempting to award too many prize strategies in the same transaction.
 
+An owner updatable `upkeepMinimumBlockInterval` is also maintained so as to mitigate multiple `performUpkeep()` transactions within the same block - if a transaction is to be included in the same block, it will revert. 
 
 # Installation
 Install the repo and dependencies by running:

contracts/PrizeStrategyUpkeep.sol

@@ -3,7 +3,6 @@
 pragma solidity ^0.7.6;
 pragma experimental ABIEncoderV2;
 
-
 import "./interfaces/KeeperCompatibleInterface.sol";
 import "./interfaces/PeriodicPrizeStrategyInterface.sol";
 import "./interfaces/PrizePoolRegistryInterface.sol";
@@ -26,19 +25,34 @@ contract PrizeStrategyUpkeep is KeeperCompatibleInterface, Ownable {
     /// @dev Set accordingly to prevent out-of-gas transactions during calls to performUpkeep
     uint256 public upkeepBatchSize;
 
+    /// @notice Stores the last upkeep block number
+    uint256 public upkeepLastUpkeepBlockNumber;
+
+    /// @notice Stores the minimum block interval between permitted performUpkeep() calls
+    uint256 public upkeepMinimumBlockInterval;
+
     /// @notice Emitted when the upkeepBatchSize has been changed
     event UpkeepBatchSizeUpdated(uint256 upkeepBatchSize);
 
     /// @notice Emitted when the prize pool registry has been changed
     event UpkeepPrizePoolRegistryUpdated(AddressRegistry prizePoolRegistry);
 
+    /// @notice Emitted when the Upkeep Minimum Block interval is updated
+    event UpkeepMinimumBlockIntervalUpdated(uint256 upkeepMinimumBlockInterval);
+
+    /// @notice Emitted when the Upkeep has been performed
+    event UpkeepPerformed(uint256 startAwardsPerformed, uint256 completeAwardsPerformed);
+
 
-    constructor(AddressRegistry _prizePoolRegistry, uint256 _upkeepBatchSize) Ownable() public {
+    constructor(AddressRegistry _prizePoolRegistry, uint256 _upkeepBatchSize, uint256 _upkeepMinimumBlockInterval) public Ownable() {
         prizePoolRegistry = _prizePoolRegistry;
         emit UpkeepPrizePoolRegistryUpdated(_prizePoolRegistry);
 
         upkeepBatchSize = _upkeepBatchSize;
         emit UpkeepBatchSizeUpdated(_upkeepBatchSize);
+
+        upkeepMinimumBlockInterval = _upkeepMinimumBlockInterval;
+        emit UpkeepMinimumBlockIntervalUpdated(_upkeepMinimumBlockInterval);
     }
 
 
@@ -70,25 +84,46 @@ contract PrizeStrategyUpkeep is KeeperCompatibleInterface, Ownable {
     /// @param performData Not used in this implementation.
     function performUpkeep(bytes calldata performData) override external {
 
+        uint256 _upkeepLastUpkeepBlockNumber = upkeepLastUpkeepBlockNumber;
+        require(block.number > _upkeepLastUpkeepBlockNumber + upkeepMinimumBlockInterval, "PrizeStrategyUpkeep::minimum block interval not reached");
+
         address[] memory prizePools = prizePoolRegistry.getAddresses();
 
+
         uint256 batchCounter = upkeepBatchSize; //counter for batch
+
         uint256 poolIndex = 0;
-
+        uint256 startAwardCounter = 0;
+        uint256 completeAwardCounter = 0;
+
+        uint256 updatedUpkeepBlockNumber;
+
         while(batchCounter > 0 && poolIndex < prizePools.length){
 
             address prizeStrategy = PrizePoolInterface(prizePools[poolIndex]).prizeStrategy();
 
             if(prizeStrategy.canStartAward()){
                 PeriodicPrizeStrategyInterface(prizeStrategy).startAward();
+                startAwardCounter++;
                 batchCounter--;
             }
             else if(prizeStrategy.canCompleteAward()){
-                PeriodicPrizeStrategyInterface(prizeStrategy).completeAward();
+                PeriodicPrizeStrategyInterface(prizeStrategy).completeAward();       
+                completeAwardCounter++;
                 batchCounter--;
             }
             poolIndex++;            
         }
+
+        if(startAwardCounter > 0 || completeAwardCounter > 0){
+            updatedUpkeepBlockNumber = block.number;
+        }
+
+        // SSTORE upkeepLastUpkeepBlockNumber once
+        if(_upkeepLastUpkeepBlockNumber != updatedUpkeepBlockNumber){
+            upkeepLastUpkeepBlockNumber = updatedUpkeepBlockNumber;
+            emit UpkeepPerformed(startAwardCounter, completeAwardCounter);
+        }
 
     }
 
@@ -108,6 +143,14 @@ contract PrizeStrategyUpkeep is KeeperCompatibleInterface, Ownable {
         emit UpkeepPrizePoolRegistryUpdated(_prizePoolRegistry);
     }
 
+
+    /// @notice Updates the upkeep minimum interval blocks
+    /// @param _upkeepMinimumBlockInterval New upkeepMinimumBlockInterval
+    function updateUpkeepMinimumBlockInterval(uint256 _upkeepMinimumBlockInterval) external onlyOwner {
+        upkeepMinimumBlockInterval = _upkeepMinimumBlockInterval;
+        emit UpkeepMinimumBlockIntervalUpdated(_upkeepMinimumBlockInterval);
+    }
+
 }
 
 

deploy/deploy.js

@@ -23,12 +23,13 @@ module.exports = async (hardhat) => {
     const deployerSigner = namedSigners.deployer
 
     const batchSize = 3
+    const blockInterval = 10;
 
     dim(`deploying PrizeStrategyUpkeep contract from ${deployer}`)
     console.log("prizePoolRegistry at ", prizePoolRegistry)
 
     const prizePoolUpkeep = await deploy('PrizeStrategyUpkeep', {
-      args: [prizePoolRegistry, batchSize],
+      args: [prizePoolRegistry, batchSize, blockInterval],
       from: deployer,
       skipIfAlreadyDeployed: false
     })

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@pooltogether/pooltogether-prizestrategy-upkeep",
-  "version": "1.0.0-beta.1",
+  "version": "1.0.1",
   "description": "Upkeep integration contracts for the PoolTogether protocol",
   "author": "Aodhgan Gleeson",
   "license": "MIT",

scripts/twoTransactions.js

@@ -0,0 +1,66 @@
+const chalk = require('chalk');
+const hardhat = require('hardhat')
+
+
+function dim() {
+  console.log(chalk.dim.call(chalk, ...arguments))
+}
+
+function green() {
+  console.log(chalk.green.call(chalk, ...arguments))
+}
+
+async function fireTwoTransactions(){
+    dim("running fireTwoTransactions")
+
+    const {ethers } = hardhat
+
+    const signer = await hardhat.ethers.getNamedSigner("deployer")
+
+    dim("signer address is", signer._address)
+
+    const prizeStrategyUpkeep = await ethers.getContract("PrizeStrategyUpkeep", signer)
+    const { upkeepNeeded, performData } = await prizeStrategyUpkeep.checkUpkeep([])
+
+    dim("prize strategy at ", prizeStrategyUpkeep.address)
+
+    if(upkeepNeeded){
+        // fire first performUpkeep()
+        dim(`Sending performUpkeep 1`)
+        const unsignedTx1 = await prizeStrategyUpkeep.populateTransaction.performUpkeep([])    
+        const gasLimit1 = ((await prizeStrategyUpkeep.estimateGas.performUpkeep([])).toNumber() * 2) 
+        const gasPrice1 = ((await prizeStrategyUpkeep.gasPrice1.performUpkeep([])).toNumber())
+        console.log(`performUpkeep(). Gas limit: ${gasLimit1.toString()}`)
+        await relayer.sendTransaction({
+          to: unsignedTx1.to,
+          data: unsignedTx1.data,
+          gasLimit1,
+          gasPrice1
+        })
+
+        dim(`Sending performUpkeep 2`)
+        const unsignedTx = await prizeStrategyUpkeep.populateTransaction.performUpkeep([])    
+        const gasLimit = ((await prizeStrategyUpkeep.estimateGas.performUpkeep([])).toNumber() * 2) 
+        const gasPrice = ((await prizeStrategyUpkeep.gasPrice.performUpkeep([])).toNumber() * 2) // double the gasPrice vs first tx
+        console.log(`performUpkeep(). Gas limit: ${gasLimit.toString()}`)
+        await relayer.sendTransaction({
+          to: unsignedTx.to,
+          data: unsignedTx.data,
+          gasLimit,
+          gasPrice
+        })
+
+    }
+
+}
+
+
+
+
+async function run(){
+    while(true){
+        await fireTwoTransactions()
+    }
+}
+
+run()

test/PrizeStrategyUpkeep.test.js

@@ -14,7 +14,7 @@ describe('PrizeStrategyUpkeep', function() {
   let prizePoolUpkeep
 
   let prizePool1, prizePool2
-  let prizeStrategy
+  let prizeStrategy, prizeStrategy2
 
   before(async () => {
 
@@ -24,7 +24,7 @@ describe('PrizeStrategyUpkeep', function() {
     prizePoolRegistry = await prizePoolRegistryContractFactory.deploy("Prize Pool", wallet.address)
 
     const prizePoolUpkeepContractFactory = await hre.ethers.getContractFactory("PrizeStrategyUpkeep", wallet, overrides)
-    prizePoolUpkeep = await prizePoolUpkeepContractFactory.deploy(prizePoolRegistry.address, 10)
+    prizePoolUpkeep = await prizePoolUpkeepContractFactory.deploy(prizePoolRegistry.address, 10, 10)
 
 
     const PrizePool = await hre.artifacts.readArtifact("PrizePool")
@@ -82,7 +82,33 @@ describe('PrizeStrategyUpkeep', function() {
     })
   })
 
-  describe('able to call upkeep keep', () => {
+  describe('able to update the upkeepMinimumBlockInterval', () => {
+    it('owner can update', async () => {
+      await expect(prizePoolUpkeep.updateUpkeepMinimumBlockInterval(10))
+      .to.emit(prizePoolUpkeep, "UpkeepMinimumBlockIntervalUpdated")
+      .withArgs(10)
+    })
+    it('non-owner cannot update', async () => {
+      await expect(prizePoolUpkeep.connect(wallet2).updateUpkeepMinimumBlockInterval(3))
+      .to.be.reverted
+    })
+  })
+
+  describe('able to update the prize pool registry', () => {
+    it('owner can update the registry', async () => {
+      await expect(prizePoolUpkeep.updatePrizePoolRegistry(prizePoolRegistry.address))
+      .to.emit(prizePoolUpkeep, "UpkeepPrizePoolRegistryUpdated")
+      .withArgs(prizePoolRegistry.address)
+    })
+    it('non-owner cannot update', async () => {
+      await expect(prizePoolUpkeep.connect(wallet2).updatePrizePoolRegistry(SENTINAL))
+      .to.be.reverted
+    })
+  })
+
+
+
+  describe('able to call upkeep performUpkeep()', () => {
 
     let mockContractFactory, mockContract
 
@@ -102,6 +128,7 @@ describe('PrizeStrategyUpkeep', function() {
       await prizeStrategy.mock.completeAward.revertsWithReason("completeAward")
       await expect(prizePoolUpkeep.performUpkeep("0x")).to.be.revertedWith("completeAward")
     })
+
     it('cannot startAward()', async () => {
       await prizeStrategy.mock.canCompleteAward.returns(false)
       await prizeStrategy.mock.canStartAward.revertsWithReason("startAward")
@@ -128,6 +155,100 @@ describe('PrizeStrategyUpkeep', function() {
 
   })
 
+  describe('upkeep outside interval', () => {
+
+    let mockContractFactory, mockContract, prizeStrategy2
+    const provider = hre.ethers.provider
+
+    beforeEach(async() => {
+      mockContractFactory = await hre.ethers.getContractFactory("MockContract", wallet3, overrides)
+      mockContract = await mockContractFactory.deploy(SENTINAL)
+
+      const prizePoolRegistryContractFactory = await hre.ethers.getContractFactory("AddressRegistry", wallet, overrides)
+      prizePoolRegistry = await prizePoolRegistryContractFactory.deploy("Prize Pool", wallet.address)
+
+      const prizePoolUpkeepContractFactory = await hre.ethers.getContractFactory("PrizeStrategyUpkeep", wallet, overrides)
+      prizePoolUpkeep = await prizePoolUpkeepContractFactory.deploy(prizePoolRegistry.address, 10, 10)
+
+
+      const PrizePool = await hre.artifacts.readArtifact("PrizePool")
+      prizePool1 = await deployMockContract(wallet, PrizePool.abi, overrides)
+      prizePool2 = await deployMockContract(wallet, PrizePool.abi, overrides)
+
+
+      await prizePoolRegistry.addAddresses([prizePool1.address, prizePool2.address])
+
+
+      const PeriodicPrizeStrategy = await hre.artifacts.readArtifact("PeriodicPrizeStrategyInterface")
+      prizeStrategy = await deployMockContract(wallet, PeriodicPrizeStrategy.abi, overrides)
+
+
+      prizeStrategy2 = await deployMockContract(wallet, PeriodicPrizeStrategy.abi, overrides)
+
+      await prizePool1.mock.prizeStrategy.returns(prizeStrategy.address)
+      await prizePool2.mock.prizeStrategy.returns(prizeStrategy.address)
+
+    })
+
+    it('calls performWork() when interval expired', async () => {
+
+      const upkeepInterval = 5
+
+      await prizeStrategy.mock.canStartAward.returns(true)
+      await prizeStrategy.mock.canCompleteAward.returns(false)
+
+
+      await prizeStrategy.mock.startAward.returns()
+      await prizeStrategy.mock.completeAward.returns()
+
+      await expect(prizePoolUpkeep.performUpkeep("0x")).to.emit(prizePoolUpkeep, "UpkeepPerformed")
+
+      await expect(prizePoolUpkeep.updateUpkeepMinimumBlockInterval(upkeepInterval)).
+      to.emit(prizePoolUpkeep, "UpkeepMinimumBlockIntervalUpdated")
+
+      await expect(prizePoolUpkeep.performUpkeep("0x")).to.be.reverted
+
+      // move forward the upkeepIntervalnumber of blocks
+      for(let index = 0; index < upkeepInterval; index++){
+        await provider.send('evm_mine', [])
+      }
+      // should now be able to perform upkeep since the interval has expired
+      await expect(prizePoolUpkeep.performUpkeep("0x")).to.emit(prizePoolUpkeep, "UpkeepPerformed")
+    })
 
+    it('can execute completeAward() and emit event', async () => {
+      await prizeStrategy.mock.canCompleteAward.returns(true)
+      await prizeStrategy.mock.canStartAward.returns(false)
+      await prizeStrategy.mock.completeAward.returns()
+      const result = await prizePoolUpkeep.performUpkeep("0x")
+      const receipt = await provider.getTransactionReceipt(result.hash)
+      let event = prizePoolUpkeep.interface.parseLog(receipt.logs[0])
+      expect((event.args.startAwardsPerformed).toNumber()).to.equal(0)
+      expect((event.args.completeAwardsPerformed).toNumber()).to.equal(2)
+
+    })
+
+    it('can execute completeAward() and startAward() and emit event', async () => {
+      await prizeStrategy.mock.canCompleteAward.returns(false)
+      await prizeStrategy.mock.canStartAward.returns(true)
+      await prizeStrategy.mock.completeAward.returns()
+      await prizeStrategy.mock.startAward.returns()
+
+      await prizePool2.mock.prizeStrategy.returns(prizeStrategy2.address)
+
+      await prizeStrategy2.mock.canStartAward.returns(false)
+      await prizeStrategy2.mock.canCompleteAward.returns(true)
+      await prizeStrategy2.mock.completeAward.returns()
+      await prizeStrategy2.mock.startAward.returns()
+
+      const result = await prizePoolUpkeep.performUpkeep("0x")
+      const receipt = await provider.getTransactionReceipt(result.hash)
+      let event = prizePoolUpkeep.interface.parseLog(receipt.logs[0])
+      expect((event.args.startAwardsPerformed).toNumber()).to.equal(1)
+      expect((event.args.completeAwardsPerformed).toNumber()).to.equal(1)
+
+    })
+
+  })
 
-});
+});