Merge pull request #902 from keep-network/adding-relay-request-timeout

Sequencing relay requests

In this PR we block upcoming requests from processing if there 
is one in progress. After a node generates a relay entry, then it can
again start processing a new request.

contracts/solidity/contracts/KeepRandomBeaconOperator.sol

@@ -62,6 +62,8 @@ contract KeepRandomBeaconOperator is Ownable {
 
     uint256[] public tickets;
     bytes[] public submissions;
+    uint256 internal currentEntryStartBlock;
+    bool internal entryInProgress;
 
     bool public groupSelectionInProgress;
 
@@ -83,7 +85,7 @@ contract KeepRandomBeaconOperator is Ownable {
 
     // Timeout in blocks for a relay entry to appear on the chain. Blocks are
     // counted from the moment relay request occur.
-    uint256 public relayRequestTimeout;
+    uint256 public relayEntryTimeout;
 
     // expiredGroupOffset is pointing to the first active group, it is also the
     // expired groups counter
@@ -162,7 +164,7 @@ contract KeepRandomBeaconOperator is Ownable {
      * @param _activeGroupsThreshold is the minimal number of groups that cannot be marked as expired and
      * needs to be greater than 0.
      * @param _groupActiveTime is the time in block after which a group expires.
-     * @param _relayRequestTimeout Timeout in blocks for a relay entry to appear on the chain.
+     * @param _relayEntryTimeout Timeout in blocks for a relay entry to appear on the chain.
      * Blocks are counted from the moment relay request occur.
      */
     function initialize(
@@ -178,7 +180,7 @@ contract KeepRandomBeaconOperator is Ownable {
         uint256 _resultPublicationBlockStep,
         uint256 _activeGroupsThreshold,
         uint256 _groupActiveTime,
-        uint256 _relayRequestTimeout,
+        uint256 _relayEntryTimeout,
         uint256 _genesisEntry,
         bytes memory _genesisGroupPubKey
     ) public onlyOwner {
@@ -197,7 +199,7 @@ contract KeepRandomBeaconOperator is Ownable {
         resultPublicationBlockStep = _resultPublicationBlockStep;
         activeGroupsThreshold = _activeGroupsThreshold;
         groupActiveTime = _groupActiveTime;
-        relayRequestTimeout = _relayRequestTimeout;
+        relayEntryTimeout = _relayEntryTimeout;
         groupSelectionSeed = _genesisEntry;
 
         // Create initial relay entry request. This will allow relayEntry to be called once
@@ -580,7 +582,7 @@ contract KeepRandomBeaconOperator is Ownable {
      * performing any operations.
      */
     function groupStaleTime(Group memory group) internal view returns(uint256) {
-        return groupActiveTimeOf(group) + relayRequestTimeout;
+        return groupActiveTimeOf(group) + relayEntryTimeout;
     }
 
     /**
@@ -695,6 +697,12 @@ contract KeepRandomBeaconOperator is Ownable {
             "At least one group needed to serve the request."
         );
 
+        uint256 entryTimeout = currentEntryStartBlock + relayEntryTimeout;
+        require(!entryInProgress || block.number > entryTimeout, "Relay entry is in progress.");
+
+        currentEntryStartBlock = block.number;
+        entryInProgress = true;
+
         bytes memory groupPubKey = selectGroup(previousEntry);
 
         signingRequestCounter++;
@@ -723,5 +731,7 @@ contract KeepRandomBeaconOperator is Ownable {
         emit SignatureSubmitted(_signingId, _groupSignature, _groupPubKey, _previousEntry, _seed);
 
         ServiceContract(serviceContract).entryCreated(requestId, _groupSignature, _seed);
+
+        entryInProgress = false;
     }
 }

contracts/solidity/migrations/2_deploy_contracts.js

@@ -22,7 +22,21 @@ const timeoutChallenge = 4;
 const resultPublicationBlockStep = 3;
 const activeGroupsThreshold = 5;
 const groupActiveTime = 300;
-const relayRequestTimeout = 10;
+// Time in blocks it takes to execute relay entry signing.
+// 1 state with state.MessagingStateDelayBlocks which is set to 1
+// 1 state with state.MessagingStateActiveBlocks which is set to 3
+const relayEntrySigningTime = 4
+
+// Deadline in blocks for relay entry publication after the first 
+// group member becomes eligible to submit the result.
+// Deadline should not be shorter than the time it takes for the
+// last group member to become eligible plus at least one block 
+// to submit.
+const relayEntryPublicationDeadline = 20
+
+// The maximum time it may take for relay entry to appear on 
+// chain after relay request has been published
+const relayEntryTimeout = relayEntrySigningTime + relayEntryPublicationDeadline
 
 // timeDKG - Timeout in blocks after DKG result is complete and ready to be published.
 // 7 states with state.MessagingStateActiveBlocks which is set to 3
@@ -55,7 +69,7 @@ module.exports = async function(deployer) {
   keepRandomBeaconOperator.initialize(
     StakingProxy.address, KeepRandomBeaconServiceProxy.address, minStake, groupThreshold, groupSize,
     timeoutInitial, timeoutSubmission, timeoutChallenge, timeDKG, resultPublicationBlockStep,
-    activeGroupsThreshold, groupActiveTime, relayRequestTimeout,
+    activeGroupsThreshold, groupActiveTime, relayEntryTimeout,
     web3.utils.toBN('31415926535897932384626433832795028841971693993751058209749445923078164062862'),
     "0x1f1954b33144db2b5c90da089e8bde287ec7089d5d6433f3b6becaefdb678b1b2a9de38d14bef2cf9afc3c698a4211fa7ada7b4f036a2dfef0dc122b423259d0",
   );

contracts/solidity/test/TestKeepRandomBeaconOperatorRelayEntryTimeout.js

@@ -0,0 +1,54 @@
+import expectThrowWithMessage from './helpers/expectThrowWithMessage';
+import {bls} from './helpers/data';
+import {initContracts} from './helpers/initContracts';
+import mineBlocks from './helpers/mineBlocks';
+
+contract('TestKeepRandomBeaconOperatorRelayEntryTimeout', function(accounts) {
+  let operatorContract;
+  const blocksForward = 20;
+  const requestCounter = 0;
+
+  describe("RelayRequestTimeout", function() {
+
+    beforeEach(async () => {
+
+      let contracts = await initContracts(
+        accounts,
+        artifacts.require('./KeepToken.sol'),
+        artifacts.require('./StakingProxy.sol'),
+        artifacts.require('./TokenStaking.sol'),
+        artifacts.require('./KeepRandomBeaconServiceProxy.sol'),
+        artifacts.require('./KeepRandomBeaconService.sol'),
+        artifacts.require('./KeepRandomBeaconOperatorStub.sol')
+      );
+
+      operatorContract = contracts.operatorContract;
+
+      // Using stub method to add first group to help testing.
+      await operatorContract.registerNewGroup(bls.groupPubKey);
+      // Passing a sender's authorization. accounts[0] is a msg.sender on blockchain
+      await operatorContract.addServiceContract(accounts[0])
+    });
+
+    it("should not throw an error when sigining is in progress and the block number > relay entry timeout", async function() {
+      await operatorContract.sign(requestCounter, bls.seed, bls.previousEntry);
+      mineBlocks(blocksForward)
+      await operatorContract.sign(requestCounter, bls.seed, bls.previousEntry);
+
+      assert.equal((await operatorContract.getPastEvents())[0].event, 'SignatureRequested', "SignatureRequested event should occur on operator contract.");
+    })
+
+    it("should throw an error when sigining is in progress and the block number <= relay entry timeout", async function() {
+      await operatorContract.sign(requestCounter, bls.seed, bls.previousEntry);
+
+      await expectThrowWithMessage(operatorContract.sign(requestCounter, bls.seed, bls.previousEntry), 'Relay entry is in progress.');
+    })
+
+    it("should not throw an error when sigining is not in progress and the block number > relay entry timeout", async function() {
+      await operatorContract.sign(requestCounter, bls.seed, bls.previousEntry);
+
+      assert.equal((await operatorContract.getPastEvents())[0].event, 'SignatureRequested', "SignatureRequested event should occur on operator contract.");
+    })
+
+  })
+});

contracts/solidity/test/TestKeepRandomBeaconServiceViaProxy.js

@@ -13,7 +13,7 @@ contract('TestKeepRandomBeaconServiceViaProxy', function(accounts) {
     account_two = accounts[1],
     account_three = accounts[2];
 
-  before(async () => {
+  beforeEach(async () => {
     let contracts = await initContracts(
       accounts,
       artifacts.require('./KeepToken.sol'),
@@ -51,14 +51,14 @@ contract('TestKeepRandomBeaconServiceViaProxy', function(accounts) {
 
     let contractBalanceViaProxy = await web3.eth.getBalance(serviceContractProxy.address);
     assert.equal(contractBalanceViaProxy, 100, "Keep Random Beacon service contract new balance should be visible via serviceContractProxy.");
-
   });
 
   it("should be able to request relay entry via serviceContractProxy contract with enough ether", async function() {
     await exceptThrow(serviceContractProxy.sendTransaction({from: account_two, value: 1000}));
 
     await web3.eth.sendTransaction({
-      from: account_two, value: 100, gas: 200000, to: serviceContractProxy.address,
+      // if you see a plain 'revert' error, it's probably because of not enough gas
+      from: account_two, value: 200, gas: 300000, to: serviceContractProxy.address,
       data: encodeCall('requestRelayEntry', ['uint256'], [0])
     });
 
@@ -72,7 +72,6 @@ contract('TestKeepRandomBeaconServiceViaProxy', function(accounts) {
   });
 
   it("owner should be able to withdraw ether from random beacon service contract", async function() {
-
     await serviceContract.requestRelayEntry(0, {from: account_one, value: 100})
 
     // should fail to withdraw if not owner

contracts/solidity/test/helpers/expectThrowWithMessage.js

@@ -0,0 +1,10 @@
+export default async (promise, message) => {
+  try {
+    await promise;
+  } catch (error) {
+    assert.include(error.message, message);
+
+     return;
+  }
+  assert.fail('Expected throw not received');
+};