Merge pull request #1227 from keep-network/no-two-entries

Do not allow to submit entry more than one time

Operators were allowed to submit valid relay entry multiple times until the next relay request. This was wrong for at least two reasons:

- Each new entry emits an event and application listening for those events may do some action on a new entry.
- Each new entry submission costs some gas; in case two or more operators race for submitting a new entry, we should minimize the gas expenditure of losers.

Two other minor changes in this PR:

- Separated corrupted and invalid relay entry submission tests
    I had to import expectThrowWithMessage for the no-duplicate-entry unit test and couldn't help myself and had to refactor and split invalid entry test to use that import as well

 - Implemented isEntryInProgressFunction
    This function wraps currentEntryStartBlock != 0 condition. There is a negligible gas cost of having this function but it nicely describes what are we checking and we have cleaner code this way:
    - The relay request gas cost increased by 41 units.
    - The relay entry submission gas cost increased by 35 units.
    - The size of operator contract increased by 17 bytes.

contracts/solidity/contracts/KeepRandomBeaconOperator.sol

@@ -418,7 +418,7 @@ contract KeepRandomBeaconOperator {
         address serviceContract,
         uint256 entryVerificationAndProfitFee
     ) internal {
-        require(currentEntryStartBlock == 0 || hasEntryTimedOut(), "Beacon is busy");
+        require(!isEntryInProgress() || hasEntryTimedOut(), "Beacon is busy");
 
         currentEntryStartBlock = block.number;
 
@@ -441,6 +441,7 @@ contract KeepRandomBeaconOperator {
      * previous entry and seed.
      */
     function relayEntry(bytes memory _groupSignature) public {
+        require(isEntryInProgress(), "Entry was submitted");
         require(!hasEntryTimedOut(), "Entry timed out");
 
         bytes memory groupPubKey = groups.getGroupPublicKey(signingRequest.groupIndex);
@@ -535,6 +536,14 @@ contract KeepRandomBeaconOperator {
         delayFactor = ((remainingBlocks.mul(decimals).div(submissionWindow))**2).div(decimals);
     }
 
+    /**
+     * @dev Returns true if generation of a new relay entry is currently in
+     * progress.
+     */
+    function isEntryInProgress() internal view returns (bool) {
+        return currentEntryStartBlock != 0;
+    }
+
     /**
      * @dev Returns true if the currently ongoing new relay entry generation
      * operation timed out. There is a certain timeout for a new relay entry

contracts/solidity/test/TestKeepRandomBeaconOperatorRelayEntry.js

@@ -1,6 +1,8 @@
 import expectThrow from './helpers/expectThrow';
+import expectThrowWithMessage from './helpers/expectThrowWithMessage';
 import {bls} from './helpers/data';
 import {initContracts} from './helpers/initContracts';
+import {createSnapshot, restoreSnapshot} from "./helpers/snapshot";
 
 contract('KeepRandomBeaconOperator', (accounts) => {
   let serviceContract, operatorContract;
@@ -30,6 +32,14 @@ contract('KeepRandomBeaconOperator', (accounts) => {
     await serviceContract.methods['requestRelayEntry()']({value: entryFeeEstimate});
   });
 
+  beforeEach(async () => {
+    await createSnapshot()
+  });
+
+  afterEach(async () => {
+    await restoreSnapshot()
+  });
+
   it("should keep relay entry submission at reasonable price", async () => {
     let gasEstimate = await operatorContract.relayEntry.estimateGas(bls.groupSignature);
 
@@ -38,12 +48,21 @@ contract('KeepRandomBeaconOperator', (accounts) => {
     assert.isBelow(gasEstimate, 369544, "Relay entry submission is too expensive")
   });
 
-  it("should not allow to submit invalid relay entry", async () => {
-      // Invalid signature
-      let groupSignature = "0x0fb34abfa2a9844a58776650e399bca3e08ab134e42595e03e3efc5a0472bcd8";
+  it("should not allow to submit corrupted relay entry", async () => {
+      // This is not a valid G1 point
+      let groupSignature = "0x11134abfa2a9844a58776650e399bca3e08ab134e42595e03e3efc5a0472bcd8";
 
       await expectThrow(operatorContract.relayEntry(groupSignature));
-    });
+  })
+
+  it("should not allow to submit invalid relay entry", async () => {
+      // Signature is a valid G1 point but it is not a signature over the
+      // expected input.
+      await expectThrowWithMessage(
+        operatorContract.relayEntry(bls.nextGroupSignature),
+        "Invalid signature"
+      );
+  });
 
   it("should allow to submit valid relay entry", async () => {
     await operatorContract.relayEntry(bls.groupSignature);
@@ -52,4 +71,13 @@ contract('KeepRandomBeaconOperator', (accounts) => {
       bls.groupSignatureNumber.toString(), "Should emit event with generated entry"
     );
   });
+
+  it("should allow to submit only one entry", async () => {
+    await operatorContract.relayEntry(bls.groupSignature);
+
+    await expectThrowWithMessage(
+      operatorContract.relayEntry(bls.groupSignature),
+      "Entry was submitted"
+    );
+  });
 });