privacy-scaling-explorations / zk-kit

A monorepo of reusable libraries for zero-knowledge technologies.
https://zkkit.pse.dev
MIT License
284 stars 69 forks source link

`_insertMany()` can be further gas optimized #225

Closed 0xbok closed 6 months ago

0xbok commented 6 months ago

Describe the improvement you're thinking about On top of PR https://github.com/privacy-scaling-explorations/zk-kit/pull/221, there is this gas opt to remove 1 if condition in the loop:

https://github.com/privacy-scaling-explorations/zk-kit/blob/56320f9d67abe4302799c5b140467b875907dccf/packages/imt.sol/contracts/internal/InternalLeanIMT.sol#L156-L172

uint256 parentNode;

if ((i + nextLevelStartIndex) * 2 + 1 < currentLevelSize) {
    // Assign the right node if the value exists.
    uint256 rightNode = currentLevel[(i + nextLevelStartIndex) * 2 + 1 - currentLevelStartIndex];
    // If it has a right child the result will be the hash(leftNode, rightNode)
    parentNode = PoseidonT3.hash([leftNode, rightNode]);
} else {
    // If right child doesn't exist, it will be the leftNode.
    parentNode = leftNode;
}

With L2s this gas difference shouldn't matter enough, and this may be hurting readability. So feel free to keep the code as-is.

Additional context PSE audit.

cedoor commented 6 months ago

Closing this issue as the amount of gas saved is minimal and it's not worth considering readability.