refactor: Use custom errors instead of string messages

This reduces the gas cost as custom errors always use 4 bytes.

contracts/Marketplace.sol

@@ -12,6 +12,25 @@ import "./Endian.sol";
 import "./Groth16.sol";
 
 contract Marketplace is Proofs, StateRetrieval, Endian {
+  error Marketplace_RepairRewardPercentageTooHigh();
+  error Marketplace_SlashPercentageTooHigh();
+  error Marketplace_MaximumSlashingTooHigh();
+  error Marketplace_InvalidExpiry();
+  error Marketplace_InvalidMaxSlotLoss();
+  error Marketplace_InvalidClientAddress();
+  error Marketplace_RequestAlreadyExists();
+  error Marketplace_InvalidSlot();
+  error Marketplace_SlotNotFree();
+  error Marketplace_InvalidSlotHost();
+  error Marketplace_AlreadyPaid();
+  error Marketplace_TransferFailed();
+  error Marketplace_UnknownRequest();
+  error Marketplace_RequestNotYetTimedOut();
+  error Marketplace_InvalidState();
+  error Marketplace_StartNotBeforeExpiry();
+  error Marketplace_SlotNotAcceptingProofs();
+  error Marketplace_SlotIsFree();
+
   using EnumerableSet for EnumerableSet.Bytes32Set;
   using Requests for Request;
 
@@ -61,20 +80,21 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   ) Proofs(configuration.proofs, verifier) {
     _token = token_;
 
-    require(
+    if (configuration.collateral.repairRewardPercentage > 100) {
-      configuration.collateral.repairRewardPercentage <= 100,
+      revert Marketplace_RepairRewardPercentageTooHigh();
-      "Must be less than 100"
+    }
-    );
+
-    require(
+    if (configuration.collateral.slashPercentage > 100) {
-      configuration.collateral.slashPercentage <= 100,
+      revert Marketplace_SlashPercentageTooHigh();
-      "Must be less than 100"
+    }
-    );
+
-    require(
+    if (
       configuration.collateral.maxNumberOfSlashes *
-        configuration.collateral.slashPercentage <=
+        configuration.collateral.slashPercentage >
-        100,
+      100
-      "Maximum slashing exceeds 100%"
+    ) {
-    );
+      revert Marketplace_MaximumSlashingTooHigh();
+    }
     _config = configuration;
   }
 
@@ -87,18 +107,20 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   }
 
   function requestStorage(Request calldata request) public {
-    require(request.client == msg.sender, "Invalid client address");
+    if (request.client != msg.sender) {
+      revert Marketplace_InvalidClientAddress();
+    }
 
     RequestId id = request.id();
-    require(_requests[id].client == address(0), "Request already exists");
+    if (_requests[id].client != address(0)) {
-    require(
+      revert Marketplace_RequestAlreadyExists();
-      request.expiry > 0 && request.expiry < request.ask.duration,
+    }
-      "Expiry not in range"
+    if (request.expiry == 0 || request.expiry >= request.ask.duration) {
-    );
+      revert Marketplace_InvalidExpiry();
-    require(
+    }
-      request.ask.maxSlotLoss <= request.ask.slots,
+    if (request.ask.maxSlotLoss > request.ask.slots) {
-      "maxSlotLoss exceeds slots"
+      revert Marketplace_InvalidMaxSlotLoss();
-    );
+    }
 
     _requests[id] = request;
     _requestContexts[id].endsAt = block.timestamp + request.ask.duration;
@@ -120,14 +142,18 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     Groth16Proof calldata proof
   ) public requestIsKnown(requestId) {
     Request storage request = _requests[requestId];
-    require(slotIndex < request.ask.slots, "Invalid slot");
+    if (slotIndex >= request.ask.slots) {
+      revert Marketplace_InvalidSlot();
+    }
 
     SlotId slotId = Requests.slotId(requestId, slotIndex);
     Slot storage slot = _slots[slotId];
     slot.requestId = requestId;
     slot.slotIndex = slotIndex;
 
-    require(slotState(slotId) == SlotState.Free, "Slot is not free");
+    if (slotState(slotId) != SlotState.Free) {
+      revert Marketplace_SlotNotFree();
+    }
 
     _startRequiringProofs(slotId, request.ask.proofProbability);
     submitProof(slotId, proof);
@@ -160,9 +186,14 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
 
   function freeSlot(SlotId slotId) public slotIsNotFree(slotId) {
     Slot storage slot = _slots[slotId];
-    require(slot.host == msg.sender, "Slot filled by other host");
+    if (slot.host != msg.sender) {
+      revert Marketplace_InvalidSlotHost();
+    }
+
     SlotState state = slotState(slotId);
-    require(state != SlotState.Paid, "Already paid");
+    if (state == SlotState.Paid) {
+      revert Marketplace_AlreadyPaid();
+    }
 
     if (state == SlotState.Finished) {
       _payoutSlot(slot.requestId, slotId);
@@ -209,7 +240,10 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   }
 
   function markProofAsMissing(SlotId slotId, Period period) public {
-    require(slotState(slotId) == SlotState.Filled, "Slot not accepting proofs");
+    if (slotState(slotId) != SlotState.Filled) {
+      revert Marketplace_SlotNotAcceptingProofs();
+    }
+
     _markProofAsMissing(slotId, period);
     Slot storage slot = _slots[slotId];
     Request storage request = _requests[slot.requestId];
@@ -296,13 +330,18 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   /// @param requestId the id of the request
   function withdrawFunds(RequestId requestId) public {
     Request storage request = _requests[requestId];
-    require(
+    if (block.timestamp <= requestExpiry(requestId)) {
-      block.timestamp > requestExpiry(requestId),
+      revert Marketplace_RequestNotYetTimedOut();
-      "Request not yet timed out"
+    }
-    );
+
-    require(request.client == msg.sender, "Invalid client address");
+    if (request.client != msg.sender) {
+      revert Marketplace_InvalidClientAddress();
+    }
+
     RequestContext storage context = _requestContexts[requestId];
-    require(context.state == RequestState.New, "Invalid state");
+    if (context.state != RequestState.New) {
+      revert Marketplace_InvalidState();
+    }
 
     // Update request state to Cancelled. Handle in the withdraw transaction
     // as there needs to be someone to pay for the gas to update the state
@@ -327,7 +366,10 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   }
 
   modifier requestIsKnown(RequestId requestId) {
-    require(_requests[requestId].client != address(0), "Unknown request");
+    if (_requests[requestId].client == address(0)) {
+      revert Marketplace_UnknownRequest();
+    }
+
     _;
   }
 
@@ -338,7 +380,9 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   }
 
   modifier slotIsNotFree(SlotId slotId) {
-    require(_slots[slotId].state != SlotState.Free, "Slot is free");
+    if (_slots[slotId].state == SlotState.Free) {
+      revert Marketplace_SlotIsFree();
+    }
     _;
   }
 
@@ -362,11 +406,9 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     uint256 startingTimestamp
   ) private view returns (uint256) {
     Request storage request = _requests[requestId];
-    require(
+    if (startingTimestamp >= requestExpiry(requestId)) {
-      startingTimestamp < requestExpiry(requestId),
+      revert Marketplace_StartNotBeforeExpiry();
-      "Start not before expiry"
+    }
-    );
-
     return (requestExpiry(requestId) - startingTimestamp) * request.ask.reward;
   }
 
@@ -415,7 +457,9 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
 
   function _transferFrom(address sender, uint256 amount) internal {
     address receiver = address(this);
-    require(_token.transferFrom(sender, receiver, amount), "Transfer failed");
+    if (!_token.transferFrom(sender, receiver, amount)) {
+      revert Marketplace_TransferFailed();
+    }
   }
 
   event StorageRequested(RequestId requestId, Ask ask, uint256 expiry);

contracts/Proofs.sol

@@ -7,6 +7,15 @@ import "./Periods.sol";
 import "./Groth16.sol";
 
 abstract contract Proofs is Periods {
+  error Proofs_InsufficientBlockHeight();
+  error Proofs_InvalidProof();
+  error Proofs_ProofAlreadySubmitted();
+  error Proofs_PeriodNotEnded();
+  error Proofs_ValidationTimedOut();
+  error Proofs_ProofNotMissing();
+  error Proofs_ProofNotRequired();
+  error Proofs_ProofAlreadyMarkedMissing();
+
   ProofConfig private _config;
   IGroth16Verifier private _verifier;
 
@@ -14,7 +23,10 @@ abstract contract Proofs is Periods {
     ProofConfig memory config,
     IGroth16Verifier verifier
   ) Periods(config.period) {
-    require(block.number > 256, "Insufficient block height");
+    if (block.number <= 256) {
+      revert Proofs_InsufficientBlockHeight();
+    }
+
     _config = config;
     _verifier = verifier;
   }
@@ -113,19 +125,40 @@ abstract contract Proofs is Periods {
     Groth16Proof calldata proof,
     uint[] memory pubSignals
   ) internal {
-    require(!_received[id][_blockPeriod()], "Proof already submitted");
+    if (_received[id][_blockPeriod()]) {
-    require(_verifier.verify(proof, pubSignals), "Invalid proof");
+      revert Proofs_ProofAlreadySubmitted();
+    }
+
+    if (!_verifier.verify(proof, pubSignals)) {
+      revert Proofs_InvalidProof();
+    }
+
     _received[id][_blockPeriod()] = true;
     emit ProofSubmitted(id);
   }
 
   function _markProofAsMissing(SlotId id, Period missedPeriod) internal {
     uint256 end = _periodEnd(missedPeriod);
-    require(end < block.timestamp, "Period has not ended yet");
+    if (end >= block.timestamp) {
-    require(block.timestamp < end + _config.timeout, "Validation timed out");
+      revert Proofs_PeriodNotEnded();
-    require(!_received[id][missedPeriod], "Proof was submitted, not missing");
+    }
-    require(_isProofRequired(id, missedPeriod), "Proof was not required");
+
-    require(!_missing[id][missedPeriod], "Proof already marked as missing");
+    if (block.timestamp >= end + _config.timeout) {
+      revert Proofs_ValidationTimedOut();
+    }
+
+    if (_received[id][missedPeriod]) {
+      revert Proofs_ProofNotMissing();
+    }
+
+    if (!_isProofRequired(id, missedPeriod)) {
+      revert Proofs_ProofNotRequired();
+    }
+
+    if (_missing[id][missedPeriod]) {
+      revert Proofs_ProofAlreadyMarkedMissing();
+    }
+
     _missing[id][missedPeriod] = true;
     _missed[id] += 1;
   }

test/Marketplace.test.js

@@ -57,18 +57,24 @@ describe("Marketplace constructor", function () {
     await revert()
   })
 
-  function testPercentageOverflow(property) {
+  function testPercentageOverflow(property, expectedError) {
     it(`should reject for ${property} overflowing percentage values`, async () => {
       config.collateral[property] = 101
 
       await expect(
         Marketplace.deploy(config, token.address, verifier.address)
-      ).to.be.revertedWith("Must be less than 100")
+      ).to.be.revertedWith(expectedError)
     })
   }
 
-  testPercentageOverflow("repairRewardPercentage")
+  testPercentageOverflow(
-  testPercentageOverflow("slashPercentage")
+    "repairRewardPercentage",
+    "Marketplace_RepairRewardPercentageTooHigh"
+  )
+  testPercentageOverflow(
+    "slashPercentage",
+    "Marketplace_SlashPercentageTooHigh"
+  )
 
   it("should reject when total slash percentage exceeds 100%", async () => {
     config.collateral.slashPercentage = 1
@@ -76,7 +82,7 @@ describe("Marketplace constructor", function () {
 
     await expect(
       Marketplace.deploy(config, token.address, verifier.address)
-    ).to.be.revertedWith("Maximum slashing exceeds 100%")
+    ).to.be.revertedWith("Marketplace_MaximumSlashingTooHigh")
   })
 })
 
@@ -159,7 +165,7 @@ describe("Marketplace", function () {
       let invalid = { ...request, client: host.address }
       await token.approve(marketplace.address, price(invalid))
       await expect(marketplace.requestStorage(invalid)).to.be.revertedWith(
-        "Invalid client address"
+        "Marketplace_InvalidClientAddress"
       )
     })
 
@@ -176,19 +182,19 @@ describe("Marketplace", function () {
 
       request.expiry = request.ask.duration + 1
       await expect(marketplace.requestStorage(request)).to.be.revertedWith(
-        "Expiry not in range"
+        "Marketplace_InvalidExpiry"
       )
 
       request.expiry = 0
       await expect(marketplace.requestStorage(request)).to.be.revertedWith(
-        "Expiry not in range"
+        "Marketplace_InvalidExpiry"
       )
     })
 
     it("is rejected when maxSlotLoss exceeds slots", async function () {
       request.ask.maxSlotLoss = request.ask.slots + 1
       await expect(marketplace.requestStorage(request)).to.be.revertedWith(
-        "maxSlotLoss exceeds slots"
+        "Marketplace_InvalidMaxSlotLoss"
       )
     })
 
@@ -196,7 +202,7 @@ describe("Marketplace", function () {
       await token.approve(marketplace.address, price(request) * 2)
       await marketplace.requestStorage(request)
       await expect(marketplace.requestStorage(request)).to.be.revertedWith(
-        "Request already exists"
+        "Marketplace_RequestAlreadyExists"
       )
     })
   })
@@ -224,7 +230,7 @@ describe("Marketplace", function () {
 
     it("fails to retrieve a request of an empty slot", async function () {
       expect(marketplace.getActiveSlot(slotId(slot))).to.be.revertedWith(
-        "Slot is free"
+        "Marketplace_SlotIsFree"
       )
     })
 
@@ -238,21 +244,21 @@ describe("Marketplace", function () {
     it("is rejected when proof is incorrect", async function () {
       await expect(
         marketplace.fillSlot(slot.request, slot.index, invalidProof())
-      ).to.be.revertedWith("Invalid proof")
+      ).to.be.revertedWith("Proofs_InvalidProof")
     })
 
     it("is rejected when slot already filled", async function () {
       await marketplace.fillSlot(slot.request, slot.index, proof)
       await expect(
         marketplace.fillSlot(slot.request, slot.index, proof)
-      ).to.be.revertedWith("Slot is not free")
+      ).to.be.revertedWith("Marketplace_SlotNotFree")
     })
 
     it("is rejected when request is unknown", async function () {
       let unknown = await exampleRequest()
       await expect(
         marketplace.fillSlot(requestId(unknown), 0, proof)
-      ).to.be.revertedWith("Unknown request")
+      ).to.be.revertedWith("Marketplace_UnknownRequest")
     })
 
     it("is rejected when request is cancelled", async function () {
@@ -264,7 +270,7 @@ describe("Marketplace", function () {
       switchAccount(host)
       await expect(
         marketplace.fillSlot(requestId(expired), slot.index, proof)
-      ).to.be.revertedWith("Slot is not free")
+      ).to.be.revertedWith("Marketplace_SlotNotFree")
     })
 
     it("is rejected when request is finished", async function () {
@@ -272,7 +278,7 @@ describe("Marketplace", function () {
       await waitUntilFinished(marketplace, slot.request)
       await expect(
         marketplace.fillSlot(slot.request, slot.index, proof)
-      ).to.be.revertedWith("Slot is not free")
+      ).to.be.revertedWith("Marketplace_SlotNotFree")
     })
 
     it("is rejected when request is failed", async function () {
@@ -280,14 +286,14 @@ describe("Marketplace", function () {
       await waitUntilFailed(marketplace, request)
       await expect(
         marketplace.fillSlot(slot.request, slot.index, proof)
-      ).to.be.revertedWith("Slot is not free")
+      ).to.be.revertedWith("Marketplace_SlotNotFree")
     })
 
     it("is rejected when slot index not in range", async function () {
       const invalid = request.ask.slots
       await expect(
         marketplace.fillSlot(slot.request, invalid, proof)
-      ).to.be.revertedWith("Invalid slot")
+      ).to.be.revertedWith("Marketplace_InvalidSlot")
     })
 
     it("fails when all slots are already filled", async function () {
@@ -302,7 +308,7 @@ describe("Marketplace", function () {
       }
       await expect(
         marketplace.fillSlot(slot.request, lastSlot, proof)
-      ).to.be.revertedWith("Slot is not free")
+      ).to.be.revertedWith("Marketplace_SlotNotFree")
     })
   })
 
@@ -436,7 +442,7 @@ describe("Marketplace", function () {
       slot.index = 5
       let nonExistentId = slotId(slot)
       await expect(marketplace.freeSlot(nonExistentId)).to.be.revertedWith(
-        "Slot is free"
+        "Marketplace_SlotIsFree"
       )
     })
 
@@ -444,7 +450,7 @@ describe("Marketplace", function () {
       await waitUntilStarted(marketplace, request, proof, token)
       switchAccount(client)
       await expect(marketplace.freeSlot(id)).to.be.revertedWith(
-        "Slot filled by other host"
+        "Marketplace_InvalidSlotHost"
       )
     })
 
@@ -513,7 +519,7 @@ describe("Marketplace", function () {
       await waitUntilFinished(marketplace, requestId(request))
       await marketplace.freeSlot(slotId(slot))
       await expect(marketplace.freeSlot(slotId(slot))).to.be.revertedWith(
-        "Already paid"
+        "Marketplace_AlreadyPaid"
       )
     })
 
@@ -571,7 +577,7 @@ describe("Marketplace", function () {
       }
       await expect(
         marketplace.fillSlot(slot.request, lastSlot, proof)
-      ).to.be.revertedWith("Slot is not free")
+      ).to.be.revertedWith("Marketplace_SlotNotFree")
     })
   })
 
@@ -587,14 +593,14 @@ describe("Marketplace", function () {
     it("rejects withdraw when request not yet timed out", async function () {
       switchAccount(client)
       await expect(marketplace.withdrawFunds(slot.request)).to.be.revertedWith(
-        "Request not yet timed out"
+        "Marketplace_RequestNotYetTimedOut"
       )
     })
 
     it("rejects withdraw when wrong account used", async function () {
       await waitUntilCancelled(request)
       await expect(marketplace.withdrawFunds(slot.request)).to.be.revertedWith(
-        "Invalid client address"
+        "Marketplace_InvalidClientAddress"
       )
     })
 
@@ -611,7 +617,7 @@ describe("Marketplace", function () {
       await waitUntilCancelled(request)
       switchAccount(client)
       await expect(marketplace.withdrawFunds(slot.request)).to.be.revertedWith(
-        "Invalid state"
+        "Marketplace_InvalidState"
       )
     })
 
@@ -930,7 +936,7 @@ describe("Marketplace", function () {
       let missedPeriod = periodOf(await currentTime())
       await expect(
         marketplace.markProofAsMissing(slotId(slot), missedPeriod)
-      ).to.be.revertedWith("Slot not accepting proofs")
+      ).to.be.revertedWith("Marketplace_SlotNotAcceptingProofs")
     })
 
     describe("slashing when missing proofs", function () {

test/Proofs.test.js

@@ -207,14 +207,14 @@ describe("Proofs", function () {
       let invalid = exampleProof()
       await expect(
         proofs.proofReceived(slotId, invalid, pubSignals)
-      ).to.be.revertedWith("Invalid proof")
+      ).to.be.revertedWith("Proofs_InvalidProof")
     })
 
     it("fails proof submission when public input is incorrect", async function () {
       let invalid = [1, 2, 3]
       await expect(
         proofs.proofReceived(slotId, proof, invalid)
-      ).to.be.revertedWith("Invalid proof")
+      ).to.be.revertedWith("Proofs_InvalidProof")
     })
 
     it("emits an event when proof was submitted", async function () {
@@ -228,7 +228,7 @@ describe("Proofs", function () {
       await proofs.proofReceived(slotId, proof, pubSignals)
       await expect(
         proofs.proofReceived(slotId, proof, pubSignals)
-      ).to.be.revertedWith("Proof already submitted")
+      ).to.be.revertedWith("Proofs_ProofAlreadySubmitted")
     })
 
     it("marks a proof as missing", async function () {
@@ -246,7 +246,7 @@ describe("Proofs", function () {
       let currentPeriod = periodOf(await currentTime())
       await expect(
         proofs.markProofAsMissing(slotId, currentPeriod)
-      ).to.be.revertedWith("Period has not ended yet")
+      ).to.be.revertedWith("Proofs_PeriodNotEnded")
     })
 
     it("does not mark a proof as missing after timeout", async function () {
@@ -255,7 +255,7 @@ describe("Proofs", function () {
       await advanceTimeToForNextBlock(periodEnd(currentPeriod) + timeout)
       await expect(
         proofs.markProofAsMissing(slotId, currentPeriod)
-      ).to.be.revertedWith("Validation timed out")
+      ).to.be.revertedWith("Proofs_ValidationTimedOut")
     })
 
     it("does not mark a received proof as missing", async function () {
@@ -266,7 +266,7 @@ describe("Proofs", function () {
       await mine()
       await expect(
         proofs.markProofAsMissing(slotId, receivedPeriod)
-      ).to.be.revertedWith("Proof was submitted, not missing")
+      ).to.be.revertedWith("Proofs_ProofNotMissing")
     })
 
     it("does not mark proof as missing when not required", async function () {
@@ -279,7 +279,7 @@ describe("Proofs", function () {
       await mine()
       await expect(
         proofs.markProofAsMissing(slotId, currentPeriod)
-      ).to.be.revertedWith("Proof was not required")
+      ).to.be.revertedWith("Proofs_ProofNotRequired")
     })
 
     it("does not mark proof as missing twice", async function () {
@@ -290,7 +290,7 @@ describe("Proofs", function () {
       await proofs.markProofAsMissing(slotId, missedPeriod)
       await expect(
         proofs.markProofAsMissing(slotId, missedPeriod)
-      ).to.be.revertedWith("Proof already marked as missing")
+      ).to.be.revertedWith("Proofs_ProofAlreadyMarkedMissing")
     })
 
     it("requires no proofs when slot is finished", async function () {