refactor: Use custom errors instead of string messages

This reduces the gas cost as custom errors always use 4 bytes.
This commit is contained in:
r4bbit 2024-08-06 11:07:03 +02:00 committed by Adam Uhlíř
parent dfab6102e7
commit 6e9c1a7cb8
No known key found for this signature in database
GPG Key ID: 1D17A9E81F76155B
4 changed files with 172 additions and 89 deletions

View File

@ -13,6 +13,25 @@ import "./Endian.sol";
import "./Groth16.sol";
contract Marketplace is SlotReservations, 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 EnumerableSet for EnumerableSet.AddressSet;
using Requests for Request;
@ -71,20 +90,21 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
{
_token = token_;
require(
configuration.collateral.repairRewardPercentage <= 100,
"Must be less than 100"
);
require(
configuration.collateral.slashPercentage <= 100,
"Must be less than 100"
);
require(
if (configuration.collateral.repairRewardPercentage > 100) {
revert Marketplace_RepairRewardPercentageTooHigh();
}
if (configuration.collateral.slashPercentage > 100) {
revert Marketplace_SlashPercentageTooHigh();
}
if (
configuration.collateral.maxNumberOfSlashes *
configuration.collateral.slashPercentage <=
100,
"Maximum slashing exceeds 100%"
);
configuration.collateral.slashPercentage >
100
) {
revert Marketplace_MaximumSlashingTooHigh();
}
_config = configuration;
}
@ -97,19 +117,21 @@ contract Marketplace is SlotReservations, 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");
require(
request.expiry > 0 && request.expiry < request.ask.duration,
"Expiry not in range"
);
require(request.ask.slots > 0, "Insufficient slots");
require(
request.ask.maxSlotLoss <= request.ask.slots,
"maxSlotLoss exceeds slots"
);
if (_requests[id].client != address(0)) {
revert Marketplace_RequestAlreadyExists();
}
if (request.expiry == 0 || request.expiry >= request.ask.duration) {
revert Marketplace_InvalidExpiry();
}
// TODO: require(request.ask.slots > 0, "Insufficient slots");
if (request.ask.maxSlotLoss > request.ask.slots) {
revert Marketplace_InvalidMaxSlotLoss();
}
_requests[id] = request;
_requestContexts[id].endsAt = block.timestamp + request.ask.duration;
@ -139,7 +161,9 @@ contract Marketplace is SlotReservations, 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);
require(_reservations[slotId].contains(msg.sender), "Reservation required");
@ -149,11 +173,10 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
slot.slotIndex = slotIndex;
RequestContext storage context = _requestContexts[requestId];
require(
slotState(slotId) == SlotState.Free ||
slotState(slotId) == SlotState.Repair,
"Slot is not free"
);
if (slotState(slotId) != SlotState.Free &&
slotState(slotId) != SlotState.Repair) {
revert Marketplace_SlotNotFree();
}
_startRequiringProofs(slotId, request.ask.proofProbability);
submitProof(slotId, proof);
@ -221,9 +244,14 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
address collateralRecipient
) 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, rewardRecipient, collateralRecipient);
@ -276,7 +304,10 @@ contract Marketplace is SlotReservations, 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];
@ -411,15 +442,20 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
address withdrawRecipient
) public {
Request storage request = _requests[requestId];
require(request.client == msg.sender, "Invalid client address");
RequestContext storage context = _requestContexts[requestId];
if (block.timestamp <= requestExpiry(requestId)) {
revert Marketplace_RequestNotYetTimedOut(); // TODO: Delete
}
if (request.client != msg.sender) {
revert Marketplace_InvalidClientAddress();
}
RequestState state = requestState(requestId);
require(
state == RequestState.Cancelled ||
state == RequestState.Failed ||
state == RequestState.Finished,
"Invalid state"
);
if (state != RequestState.Cancelled &&
state != RequestState.Failed &&
state != RequestState.Finished) {
revert Marketplace_InvalidState();
}
// fundsToReturnToClient == 0 is used for "double-spend" protection, once the funds are withdrawn
// then this variable is set to 0.
@ -464,7 +500,10 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
}
modifier requestIsKnown(RequestId requestId) {
require(_requests[requestId].client != address(0), "Unknown request");
if (_requests[requestId].client == address(0)) {
revert Marketplace_UnknownRequest();
}
_;
}
@ -475,7 +514,9 @@ contract Marketplace is SlotReservations, 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();
}
_;
}
@ -523,8 +564,9 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
uint256 endingTimestamp
) private view returns (uint256) {
Request storage request = _requests[requestId];
require(startingTimestamp < endingTimestamp, "Start not before expiry");
if (startingTimestamp >= endingTimestamp) {
revert Marketplace_StartNotBeforeExpiry();
}
return (endingTimestamp - startingTimestamp) * request.ask.reward;
}
@ -574,7 +616,9 @@ contract Marketplace is SlotReservations, 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);

View File

@ -11,6 +11,15 @@ import "./Groth16.sol";
* @notice Abstract contract that handles proofs tracking, validation and reporting functionality
*/
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;
@ -22,7 +31,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;
}
@ -189,8 +201,14 @@ abstract contract Proofs is Periods {
Groth16Proof calldata proof,
uint[] memory pubSignals
) internal {
require(!_received[id][_blockPeriod()], "Proof already submitted");
require(_verifier.verify(proof, pubSignals), "Invalid proof");
if (_received[id][_blockPeriod()]) {
revert Proofs_ProofAlreadySubmitted();
}
if (!_verifier.verify(proof, pubSignals)) {
revert Proofs_InvalidProof();
}
_received[id][_blockPeriod()] = true;
emit ProofSubmitted(id);
}
@ -209,11 +227,26 @@ abstract contract Proofs is Periods {
*/
function _markProofAsMissing(SlotId id, Period missedPeriod) internal {
uint256 end = _periodEnd(missedPeriod);
require(end < block.timestamp, "Period has not ended yet");
require(block.timestamp < end + _config.timeout, "Validation timed out");
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 (end >= block.timestamp) {
revert Proofs_PeriodNotEnded();
}
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;
}

View File

@ -58,18 +58,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("slashPercentage")
testPercentageOverflow(
"repairRewardPercentage",
"Marketplace_RepairRewardPercentageTooHigh"
)
testPercentageOverflow(
"slashPercentage",
"Marketplace_SlashPercentageTooHigh"
)
it("should reject when total slash percentage exceeds 100%", async () => {
config.collateral.slashPercentage = 1
@ -77,7 +83,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")
})
})
@ -185,7 +191,7 @@ describe("Marketplace", function () {
let invalid = { ...request, client: host.address }
await token.approve(marketplace.address, maxPrice(invalid))
await expect(marketplace.requestStorage(invalid)).to.be.revertedWith(
"Invalid client address"
"Marketplace_InvalidClientAddress"
)
})
@ -202,12 +208,12 @@ 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"
)
})
@ -221,7 +227,7 @@ describe("Marketplace", function () {
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"
)
})
@ -229,7 +235,7 @@ describe("Marketplace", function () {
await token.approve(marketplace.address, maxPrice(request) * 2)
await marketplace.requestStorage(request)
await expect(marketplace.requestStorage(request)).to.be.revertedWith(
"Request already exists"
"Marketplace_RequestAlreadyExists"
)
})
})
@ -286,7 +292,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"
)
})
@ -302,7 +308,7 @@ describe("Marketplace", function () {
await marketplace.reserveSlot(slot.request, slot.index)
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 () {
@ -310,14 +316,14 @@ describe("Marketplace", 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 () {
@ -330,7 +336,7 @@ describe("Marketplace", function () {
await marketplace.reserveSlot(requestId(expired), slot.index)
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 () {
@ -338,7 +344,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 () {
@ -346,14 +352,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 () {
@ -369,7 +375,7 @@ describe("Marketplace", function () {
}
await expect(
marketplace.fillSlot(slot.request, lastSlot, proof)
).to.be.revertedWith("Slot is not free")
).to.be.revertedWith("Marketplace_SlotNotFree")
})
it("fails if slot is not reserved first", async function () {
@ -514,7 +520,7 @@ describe("Marketplace", function () {
slot.index = 5
let nonExistentId = slotId(slot)
await expect(marketplace.freeSlot(nonExistentId)).to.be.revertedWith(
"Slot is free"
"Marketplace_SlotIsFree"
)
})
@ -522,7 +528,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"
)
})
@ -714,7 +720,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"
)
})
@ -776,7 +782,7 @@ describe("Marketplace", function () {
}
await expect(
marketplace.fillSlot(slot.request, lastSlot, proof)
).to.be.revertedWith("Slot is not free")
).to.be.revertedWith("Marketplace_SlotNotFree")
})
})
@ -793,14 +799,14 @@ describe("Marketplace", function () {
switchAccount(client)
await expect(
marketplace.withdrawFunds(slot.request, clientWithdrawRecipient.address)
).to.be.revertedWith("Invalid state")
).to.be.revertedWith("Marketplace_InvalidState")
})
it("rejects withdraw when wrong account used", async function () {
await waitUntilCancelled(request)
await expect(
marketplace.withdrawFunds(slot.request, clientWithdrawRecipient.address)
).to.be.revertedWith("Invalid client address")
).to.be.revertedWith("Marketplace_InvalidClientAddress")
})
it("rejects withdraw when in wrong state", async function () {
@ -818,7 +824,7 @@ describe("Marketplace", function () {
switchAccount(client)
await expect(
marketplace.withdrawFunds(slot.request, clientWithdrawRecipient.address)
).to.be.revertedWith("Invalid state")
).to.be.revertedWith("Marketplace_InvalidClientAddress")
})
it("rejects withdraw when already withdrawn", async function () {
@ -1256,7 +1262,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 () {

View File

@ -208,14 +208,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 () {
@ -229,7 +229,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 () {
@ -247,7 +247,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 () {
@ -256,7 +256,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 () {
@ -267,7 +267,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 () {
@ -280,7 +280,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 () {
@ -291,7 +291,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 () {