Skip to content

Commit

Permalink
fix smtlib unified address and solhint warning
Browse files Browse the repository at this point in the history
  • Loading branch information
daveroga committed Feb 21, 2025
1 parent 42afd61 commit 825e786
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 79 deletions.
16 changes: 3 additions & 13 deletions contracts/lib/ArrayUtils.sol
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.27;

error LenghtShouldBeGreaterThanZero();
error LengthLimitExceeded(uint256 limit);
error StartIndexOutOfBounds(uint256 arrLength);

/// @title A common functions for arrays.
library ArrayUtils {
/**
Expand All @@ -21,15 +17,9 @@ library ArrayUtils {
uint256 length,
uint256 limit
) internal pure returns (uint256, uint256) {
if (length == 0) {
revert LenghtShouldBeGreaterThanZero();
}
if (length > limit) {
revert LengthLimitExceeded(limit);
}
if (start >= arrLength) {
revert StartIndexOutOfBounds(arrLength);
}
require(length > 0, "Length should be greater than 0");

Check warning on line 20 in contracts/lib/ArrayUtils.sol

View workflow job for this annotation

GitHub Actions / solhint

Use Custom Errors instead of require statements
require(length <= limit, "Length limit exceeded");

Check warning on line 21 in contracts/lib/ArrayUtils.sol

View workflow job for this annotation

GitHub Actions / solhint

Use Custom Errors instead of require statements
require(start < arrLength, "Start index out of bounds");

Check warning on line 22 in contracts/lib/ArrayUtils.sol

View workflow job for this annotation

GitHub Actions / solhint

Use Custom Errors instead of require statements

uint256 end = start + length;
if (end > arrLength) {
Expand Down
2 changes: 0 additions & 2 deletions contracts/lib/Poseidon.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.27;

Check warning on line 2 in contracts/lib/Poseidon.sol

View workflow job for this annotation

GitHub Actions / solhint

Found more than One contract per file. 8 contracts found!

/* solhint-disable no-empty-blocks */
library PoseidonUnit1L {
function poseidon(uint256[1] calldata) public pure returns (uint256) {}

Check warning on line 5 in contracts/lib/Poseidon.sol

View workflow job for this annotation

GitHub Actions / solhint

Code contains empty blocks
}
Expand All @@ -25,7 +24,6 @@ library PoseidonUnit5L {
library PoseidonUnit6L {
function poseidon(uint256[6] calldata) public pure returns (uint256) {}

Check warning on line 25 in contracts/lib/Poseidon.sol

View workflow job for this annotation

GitHub Actions / solhint

Code contains empty blocks
}
/* solhint-enable no-empty-blocks */

library SpongePoseidon {
uint32 internal constant BATCH_SIZE = 6;
Expand Down
52 changes: 12 additions & 40 deletions contracts/lib/SmtLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,6 @@ pragma solidity 0.8.27;
import {PoseidonUnit2L, PoseidonUnit3L} from "./Poseidon.sol";
import {ArrayUtils} from "./ArrayUtils.sol";

error RootDoesNotExist();
error InvalidNodeType();
error NoFutureTimestampsAllowed();
error NoFutureBlocksAllowed();
error MaxDepthMustBeGreaterThanZero();
error MaxDepthCanOnlyBeIncreased();
error MaxDepthIsGreaterThanHardCap();
error SmtAlreadyInitialized();
error SmtNotInitialized();
error MaxDepthReached();
error InvalidSearchType();

/// @title A sparse merkle tree implementation, which keeps tree history.
// Note that this SMT implementation can manage duplicated roots in the history,
// which may happen when some leaf change its value and then changes it back to the original value.
Expand Down Expand Up @@ -144,9 +132,7 @@ library SmtLib {
* @param root SMT root.
*/
modifier onlyExistingRoot(Data storage self, uint256 root) {
if (!rootExists(self, root)) {
revert RootDoesNotExist();
}
require(rootExists(self, root), "Root does not exist");
_;
}

Expand Down Expand Up @@ -278,7 +264,7 @@ library SmtLib {
proof.siblings[i] = node.childRight;
}
} else {
revert InvalidNodeType();
revert("Invalid node type");
}
}
return proof;
Expand Down Expand Up @@ -327,9 +313,7 @@ library SmtLib {
Data storage self,
uint256 timestamp
) public view returns (RootEntryInfo memory) {
if (timestamp > block.timestamp) {
revert NoFutureTimestampsAllowed();
}
require(timestamp <= block.timestamp, "No future timestamps allowed");

return
_getRootInfoByTimestampOrBlock(
Expand All @@ -348,9 +332,7 @@ library SmtLib {
Data storage self,
uint256 blockN
) public view returns (RootEntryInfo memory) {
if (blockN > block.number) {
revert NoFutureBlocksAllowed();
}
require(blockN <= block.number, "No future blocks allowed");

return _getRootInfoByTimestampOrBlock(self, blockN, BinarySearchSmtRoots.SearchType.BLOCK);
}
Expand Down Expand Up @@ -426,15 +408,9 @@ library SmtLib {
* @param maxDepth max depth
*/
function setMaxDepth(Data storage self, uint256 maxDepth) public {
if (maxDepth == 0) {
revert MaxDepthMustBeGreaterThanZero();
}
if (maxDepth <= self.maxDepth) {
revert MaxDepthCanOnlyBeIncreased();
}
if (maxDepth > MAX_DEPTH_HARD_CAP) {
revert MaxDepthIsGreaterThanHardCap();
}
require(maxDepth > 0, "Max depth must be greater than zero");
require(maxDepth > self.maxDepth, "Max depth can only be increased");
require(maxDepth <= MAX_DEPTH_HARD_CAP, "Max depth is greater than hard cap");
self.maxDepth = maxDepth;
}

Expand All @@ -451,18 +427,14 @@ library SmtLib {
* @param maxDepth Max depth of the SMT.
*/
function initialize(Data storage self, uint256 maxDepth) external {
if (isInitialized(self)) {
revert SmtAlreadyInitialized();
}
require(!isInitialized(self), "Smt is already initialized");
setMaxDepth(self, maxDepth);
_addEntry(self, 0, 0, 0);
self.initialized = true;
}

modifier onlyInitialized(Data storage self) {
if (!isInitialized(self)) {
revert SmtNotInitialized();
}
require(isInitialized(self), "Smt is not initialized");
_;
}

Expand All @@ -477,7 +449,7 @@ library SmtLib {
uint256 depth
) internal returns (uint256) {
if (depth > self.maxDepth) {
revert MaxDepthReached();
revert("Max depth reached");
}

Node memory node = self.nodes[nodeHash];
Expand Down Expand Up @@ -530,7 +502,7 @@ library SmtLib {
// no reason to continue if we are at max possible depth
// as, anyway, we exceed the depth going down the tree
if (depth >= self.maxDepth) {
revert MaxDepthReached();
revert("Max depth reached");
}

Node memory newNodeMiddle;
Expand Down Expand Up @@ -724,7 +696,7 @@ library BinarySearchSmtRoots {
} else if (st == SearchType.TIMESTAMP) {
return rti.createdAtTimestamp;
} else {
revert InvalidSearchType();
revert("Invalid search type");
}
}
}
2 changes: 1 addition & 1 deletion contracts/payment/VCPayment.sol
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ contract VCPayment is Ownable2StepUpgradeable, ReentrancyGuardUpgradeable {
return $.ownerBalance;
}

function _withdraw(address to, uint amount) internal {
function _withdraw(address to, uint256 amount) internal {
if (amount == 0) {
revert NoBalanceToWithdraw(to);
}
Expand Down
36 changes: 19 additions & 17 deletions test/smtLib/smtLib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -982,23 +982,23 @@ describe("Merkle tree proofs of SMT", () => {
description: "Negative: add two leaves with maximum depth + 1 (less significant bits SET)",
leavesToInsert: [
{ i: genMaxBinaryNumber(64), v: 100 }, //1111111111111111111111111111111111111111111111111111111111111111
{ i: genMaxBinaryNumber(65), v: 100, error: "MaxDepthReached()" }, //11111111111111111111111111111111111111111111111111111111111111111
{ i: genMaxBinaryNumber(65), v: 100, error: "Max depth reached" }, //11111111111111111111111111111111111111111111111111111111111111111
],
},
{
description:
"Negative: add two leaves with maximum depth + 1 (less significant bits NOT SET)",
leavesToInsert: [
{ i: 0, v: 100 },
{ i: genMaxBinaryNumber(64) + BigInt(1), v: 100, error: "MaxDepthReached()" }, // 10000000000000000000000000000000000000000000000000000000000000000
{ i: genMaxBinaryNumber(64) + BigInt(1), v: 100, error: "Max depth reached" }, // 10000000000000000000000000000000000000000000000000000000000000000
],
},
{
description:
"Negative: add two leaves with maximum depth + 1 (less significant bits are both SET and NOT SET",
leavesToInsert: [
{ i: "17713686966169915918", v: 100 }, //1111010111010011101010000111000111010001000001100101001000001110
{ i: "36160431039879467534", v: 100, error: "MaxDepthReached()" }, //11111010111010011101010000111000111010001000001100101001000001110
{ i: "36160431039879467534", v: 100, error: "Max depth reached" }, //11111010111010011101010000111000111010001000001100101001000001110
],
},
];
Expand Down Expand Up @@ -1061,16 +1061,16 @@ describe("Root history requests", function () {
});

it("should revert if length is zero", async () => {
await expect(smt.getRootHistory(0, 0)).to.be.rejectedWith("LenghtShouldBeGreaterThanZero()");
await expect(smt.getRootHistory(0, 0)).to.be.rejectedWith("Length should be greater than 0");
});

it("should revert if length limit exceeded", async () => {
await expect(smt.getRootHistory(0, 10 ** 6)).to.be.rejectedWith("LengthLimitExceeded(1000)");
await expect(smt.getRootHistory(0, 10 ** 6)).to.be.rejectedWith("Length limit exceeded");
});

it("should revert if out of bounds", async () => {
await expect(smt.getRootHistory(historyLength, 100)).to.be.rejectedWith(
"StartIndexOutOfBounds(3)",
"Start index out of bounds",
);
});

Expand Down Expand Up @@ -1126,7 +1126,7 @@ describe("Root history duplicates", function () {
const riDoubleRoot = await smt.getRootInfoListByRoot(doubleRoot, 0, 100);
const riTripleRoot = await smt.getRootInfoListByRoot(tripleRoot, 0, 100);
await expect(smt.getRootInfoListByRoot(nonExistingRoot, 0, 100)).to.be.rejectedWith(
"RootDoesNotExist()",
"Root does not exist",
);

expect(riSingleRoot.length).to.be.equal(1);
Expand Down Expand Up @@ -1158,15 +1158,15 @@ describe("Root history duplicates", function () {
await smt.add(1, 1);
const root = await smt.getRoot();
await expect(smt.getRootInfoListByRoot(root, 0, 0)).to.be.rejectedWith(
"LenghtShouldBeGreaterThanZero()",
"Length should be greater than 0",
);
});

it("should revert if length limit exceeded", async () => {
await smt.add(1, 1);
const root = await smt.getRoot();
await expect(smt.getRootInfoListByRoot(root, 0, 10 ** 6)).to.be.rejectedWith(
"LengthLimitExceeded(1000)",
"Length limit exceeded",
);
});

Expand All @@ -1176,7 +1176,7 @@ describe("Root history duplicates", function () {
await smt.add(1, 1);
const root = await smt.getRoot();
await expect(smt.getRootInfoListByRoot(root, 3, 100)).to.be.rejectedWith(
"StartIndexOutOfBounds(2)",
"Start index out of bounds",
);
});

Expand Down Expand Up @@ -1769,14 +1769,14 @@ describe("Edge cases with exceptions", () => {
await smt.add(1, 1);
const root = await smt.getRoot();
await expect(smt.getRootInfo(root)).not.to.be.rejected;
await expect(smt.getRootInfo(root + 1n)).to.be.rejectedWith("RootDoesNotExist()");
await expect(smt.getRootInfo(root + 1n)).to.be.rejectedWith("Root does not exist");
});

it("getProofByRoot() should throw when root does not exist", async () => {
await smt.add(1, 1);
const root = await smt.getRoot();
await expect(smt.getProofByRoot(1, root)).not.to.be.rejected;
await expect(smt.getProofByRoot(1, root + 1n)).to.be.rejectedWith("RootDoesNotExist()");
await expect(smt.getProofByRoot(1, root + 1n)).to.be.rejectedWith("Root does not exist");
});
});

Expand Down Expand Up @@ -1804,20 +1804,22 @@ describe("maxDepth setting tests", () => {
});

it("Should throw when decrease max depth", async () => {
await expect(smt.setMaxDepth(127)).to.be.rejectedWith("MaxDepthCanOnlyBeIncreased()");
await expect(smt.setMaxDepth(127)).to.be.rejectedWith("Max depth can only be increased");
});

it("Should throw when max depth is set to the same value", async () => {
await expect(smt.setMaxDepth(128)).to.be.rejectedWith("MaxDepthCanOnlyBeIncreased()");
await expect(smt.setMaxDepth(128)).to.be.rejectedWith("Max depth can only be increased");
});

it("Should throw when max depth is set to 0", async () => {
await expect(smt.setMaxDepth(0)).to.be.rejectedWith("MaxDepthMustBeGreaterThanZero()");
await expect(smt.setMaxDepth(0)).to.be.rejectedWith("Max depth must be greater than zero");
});

it("Should throw when max depth is set to greater than hard cap", async () => {
await expect(smt.setMaxDepth(257)).to.be.rejectedWith("MaxDepthIsGreaterThanHardCap()");
await expect(smt.setMaxDepth(1000000000)).to.be.rejectedWith("MaxDepthIsGreaterThanHardCap()");
await expect(smt.setMaxDepth(257)).to.be.rejectedWith("Max depth is greater than hard cap");
await expect(smt.setMaxDepth(1000000000)).to.be.rejectedWith(
"Max depth is greater than hard cap",
);
});
});

Expand Down
12 changes: 6 additions & 6 deletions test/stateLib/stateLib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,20 +114,20 @@ describe("StateInfo history", function () {

it("should be reverted if length is zero", async () => {
await expect(stateLibWrpr.getStateInfoHistoryById(id1, 0, 0)).to.be.rejectedWith(
"LenghtShouldBeGreaterThanZero()",
"Length should be greater than 0",
);
});

it("should be reverted if length limit exceeded", async () => {
await expect(stateLibWrpr.getStateInfoHistoryById(id1, 0, 10 ** 6)).to.be.rejectedWith(
"LengthLimitExceeded(1000)",
"Length limit exceeded",
);
});

it("should be reverted if startIndex is out of bounds", async () => {
await expect(
stateLibWrpr.getStateInfoHistoryById(id1, id1HistoryLength, 100),
).to.be.rejectedWith("StartIndexOutOfBounds(2)");
).to.be.rejectedWith("Start index out of bounds");
});

it("should not revert if startIndex + length >= historyLength", async () => {
Expand Down Expand Up @@ -248,7 +248,7 @@ describe("State history duplicates", function () {
const state = 1;
await stateLibWrpr.addState(id, state);
await expect(stateLibWrpr.getStateInfoListByIdAndState(id, state, 0, 0)).to.be.rejectedWith(
"LenghtShouldBeGreaterThanZero()",
"Length should be greater than 0",
);
});

Expand All @@ -258,7 +258,7 @@ describe("State history duplicates", function () {
await stateLibWrpr.addState(id, state);
await expect(
stateLibWrpr.getStateInfoListByIdAndState(id, state, 0, 10 ** 6),
).to.be.rejectedWith("LengthLimitExceeded(1000)");
).to.be.rejectedWith("Length limit exceeded");
});

it("should revert if out of bounds", async () => {
Expand All @@ -268,7 +268,7 @@ describe("State history duplicates", function () {
await stateLibWrpr.addState(id, state);
await stateLibWrpr.addState(id, state);
await expect(stateLibWrpr.getStateInfoListByIdAndState(id, state, 3, 100)).to.be.rejectedWith(
"StartIndexOutOfBounds(3)",
"Start index out of bounds",
);

it("should NOT revert if startIndex + length >= historyLength", async () => {
Expand Down

0 comments on commit 825e786

Please sign in to comment.