createCommitment & blob to shares #329
Conversation
v1.8.2
WalkthroughThe recent changes introduce enhancements across multiple files in the project. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommitmentSystem
participant MerkleTree
User->>CommitmentSystem: Create Commitment
CommitmentSystem->>MerkleTree: Generate Shares
MerkleTree-->>CommitmentSystem: Return Shares
CommitmentSystem-->>User: Commitment Created
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- foundry.toml (1 hunks)
- lib/ds-test (1 hunks)
- src/lib/commitment/Commitment.sol (1 hunks)
- src/lib/commitment/test/Commitment.t.sol (1 hunks)
- src/lib/tree/Types.sol (1 hunks)
- src/lib/tree/binary/BinaryMerkleTree.sol (1 hunks)
- src/lib/tree/binary/TreeHasher.sol (1 hunks)
- src/lib/tree/namespace/NamespaceMerkleTree.sol (5 hunks)
- src/lib/verifier/DAVerifier.sol (2 hunks)
Files skipped from review due to trivial changes (2)
- lib/ds-test
- src/lib/tree/binary/TreeHasher.sol
Additional comments not posted (8)
foundry.toml (1)
5-5: LGTM! Addition offs_permissionsis appropriate.The introduction of file system permissions enhances security and operational control. The configuration is correctly formatted.
src/lib/tree/Types.sol (1)
31-35: LGTM! Verify the constants for reserved namespace range.The function
isReservedNamespacecorrectly checks if a namespace is reserved using byte comparison.However, ensure that the constants
upperandloweraccurately represent the intended reserved range.Run the following script to verify the constants:
src/lib/tree/binary/BinaryMerkleTree.sol (1)
129-129: Verify the change in function visibility fromprivatetopublic.The visibility of the function
_computeRootMultihas been changed fromprivatetopublic. Ensure that this change is intentional and does not expose sensitive functionality.Run the following script to verify the function usage and ensure the change is intentional:
src/lib/tree/namespace/NamespaceMerkleTree.sol (3)
Line range hint
15-18: Verify necessity of visibility change forverifyProof.The change from
internal puretointernalsuggests that the function may now interact with state variables. Verify if this change is necessary and ensure that it aligns with the intended functionality.
Line range hint
20-23: Verify necessity of visibility change forvalidateProof.The change from
internal puretointernalsuggests that the function may now interact with state variables. Verify if this change is necessary and ensure that it aligns with the intended functionality.
253-271: Consider implications of making_computeTreeRootpublic.The change from
private puretopublicsuggests plans for external access. Ensure that this change is necessary and aligns with the intended functionality. Additionally, consider implementing error handling as noted in the TODO comment.src/lib/verifier/DAVerifier.sol (2)
Line range hint
97-105: Verify necessity of visibility change forverifySharesToDataRootTupleRoot.The change from
viewtointernalrestricts external access and suggests potential state modifications. Verify if this change is necessary and ensure that it aligns with the intended functionality.
Line range hint
124-131: Verify necessity of modifier change forverifyMultiRowRootsToDataRootTupleRootProof.The change from
puretointernalsuggests potential state interactions. Verify if this change is necessary and ensure that it aligns with the intended functionality.
| function testBytesToSharesV0() view external { | ||
|
|
||
| // test vectors were generated here: https://github.com/S1nus/share-test-vec-gen | ||
| string memory path = "./src/lib/commitment/test/testVectors.json"; | ||
| string memory jsonData = vm.readFile(path); | ||
| bytes memory vecsData = vm.parseJson(jsonData); | ||
| TestVector[] memory vecs = abi.decode(vecsData, (TestVector[])); | ||
|
|
||
| for (uint i = 0; i < vecs.length; i++) { | ||
| bytes29 nsString = bytes29(fromHex(vecs[i].namespace)); | ||
| Namespace memory ns = toNamespace(nsString); | ||
| bytes memory data = fromHex(vecs[i].data); | ||
| (bytes[] memory shares, bool err) = _bytesToSharesV0(data, ns); | ||
| string memory out = ""; | ||
| for (uint j = 0; j < shares.length; j++) { | ||
| out = string.concat(out, _bytesToHexString(shares[j])); | ||
| } | ||
| // none of the test vectors should cause an error | ||
| //assert(!err); | ||
| //assert(compareStrings(out, vecs[i].shares)); | ||
| if (!compareStrings(out, vecs[i].shares)) { | ||
| console.log("expected: ", vecs[i].shares); | ||
| console.log("got: ", out); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Uncomment assertions for complete test coverage in testBytesToSharesV0.
The test function testBytesToSharesV0 includes commented-out assertions that should be uncommented to ensure complete test coverage.
Uncomment the following lines to enable assertions:
- //assert(!err);
- //assert(compareStrings(out, vecs[i].shares));
+ assert(!err);
+ assert(compareStrings(out, vecs[i].shares));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function testBytesToSharesV0() view external { | |
| // test vectors were generated here: https://github.com/S1nus/share-test-vec-gen | |
| string memory path = "./src/lib/commitment/test/testVectors.json"; | |
| string memory jsonData = vm.readFile(path); | |
| bytes memory vecsData = vm.parseJson(jsonData); | |
| TestVector[] memory vecs = abi.decode(vecsData, (TestVector[])); | |
| for (uint i = 0; i < vecs.length; i++) { | |
| bytes29 nsString = bytes29(fromHex(vecs[i].namespace)); | |
| Namespace memory ns = toNamespace(nsString); | |
| bytes memory data = fromHex(vecs[i].data); | |
| (bytes[] memory shares, bool err) = _bytesToSharesV0(data, ns); | |
| string memory out = ""; | |
| for (uint j = 0; j < shares.length; j++) { | |
| out = string.concat(out, _bytesToHexString(shares[j])); | |
| } | |
| // none of the test vectors should cause an error | |
| //assert(!err); | |
| //assert(compareStrings(out, vecs[i].shares)); | |
| if (!compareStrings(out, vecs[i].shares)) { | |
| console.log("expected: ", vecs[i].shares); | |
| console.log("got: ", out); | |
| } | |
| } | |
| } | |
| function testBytesToSharesV0() view external { | |
| // test vectors were generated here: https://github.com/S1nus/share-test-vec-gen | |
| string memory path = "./src/lib/commitment/test/testVectors.json"; | |
| string memory jsonData = vm.readFile(path); | |
| bytes memory vecsData = vm.parseJson(jsonData); | |
| TestVector[] memory vecs = abi.decode(vecsData, (TestVector[])); | |
| for (uint i = 0; i < vecs.length; i++) { | |
| bytes29 nsString = bytes29(fromHex(vecs[i].namespace)); | |
| Namespace memory ns = toNamespace(nsString); | |
| bytes memory data = fromHex(vecs[i].data); | |
| (bytes[] memory shares, bool err) = _bytesToSharesV0(data, ns); | |
| string memory out = ""; | |
| for (uint j = 0; j < shares.length; j++) { | |
| out = string.concat(out, _bytesToHexString(shares[j])); | |
| } | |
| // none of the test vectors should cause an error | |
| assert(!err); | |
| assert(compareStrings(out, vecs[i].shares)); | |
| if (!compareStrings(out, vecs[i].shares)) { | |
| console.log("expected: ", vecs[i].shares); | |
| console.log("got: ", out); | |
| } | |
| } | |
| } |
| function testCreateCommitmentV0() external { | ||
| string memory path = "./src/lib/commitment/test/testVectors2.json"; | ||
| string memory jsonData = vm.readFile(path); | ||
| bytes memory vecsData = vm.parseJson(jsonData); | ||
| TestVector[] memory vecs = abi.decode(vecsData, (TestVector[])); | ||
|
|
||
|
|
||
| //for (uint i = 0; i < vecs.length; i++) { | ||
| bytes29 nsString = bytes29(fromHex(vecs[0].namespace)); | ||
| Namespace memory ns = toNamespace(nsString); | ||
| bytes memory data = fromHex(vecs[0].data); | ||
| (bytes[] memory shares, bool err) = _bytesToSharesV0(data, ns); | ||
| bytes32 commitment = _createCommitment(shares, ns); | ||
| if (!compareStrings(_bytesToHexString(abi.encodePacked(commitment)), vecs[0].commitment)) { | ||
| console.log("Commitment mismatch for vector ", 0); | ||
| console.log("expected: ", vecs[0].commitment); | ||
| console.log("got: ", _bytesToHexString(abi.encodePacked(commitment))); | ||
| return; | ||
| } | ||
| //} | ||
| console.log("All good :)"); | ||
|
|
There was a problem hiding this comment.
Uncomment assertions for complete test coverage in testCreateCommitmentV0.
The test function testCreateCommitmentV0 includes commented-out code that should be uncommented to ensure complete test coverage.
Uncomment the following lines to enable assertions:
- //for (uint i = 0; i < vecs.length; i++) {
+ for (uint i = 0; i < vecs.length; i++) {
- //}
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function testCreateCommitmentV0() external { | |
| string memory path = "./src/lib/commitment/test/testVectors2.json"; | |
| string memory jsonData = vm.readFile(path); | |
| bytes memory vecsData = vm.parseJson(jsonData); | |
| TestVector[] memory vecs = abi.decode(vecsData, (TestVector[])); | |
| //for (uint i = 0; i < vecs.length; i++) { | |
| bytes29 nsString = bytes29(fromHex(vecs[0].namespace)); | |
| Namespace memory ns = toNamespace(nsString); | |
| bytes memory data = fromHex(vecs[0].data); | |
| (bytes[] memory shares, bool err) = _bytesToSharesV0(data, ns); | |
| bytes32 commitment = _createCommitment(shares, ns); | |
| if (!compareStrings(_bytesToHexString(abi.encodePacked(commitment)), vecs[0].commitment)) { | |
| console.log("Commitment mismatch for vector ", 0); | |
| console.log("expected: ", vecs[0].commitment); | |
| console.log("got: ", _bytesToHexString(abi.encodePacked(commitment))); | |
| return; | |
| } | |
| //} | |
| console.log("All good :)"); | |
| function testCreateCommitmentV0() external { | |
| string memory path = "./src/lib/commitment/test/testVectors2.json"; | |
| string memory jsonData = vm.readFile(path); | |
| bytes memory vecsData = vm.parseJson(jsonData); | |
| TestVector[] memory vecs = abi.decode(vecsData, (TestVector[])); | |
| for (uint i = 0; i < vecs.length; i++) { | |
| bytes29 nsString = bytes29(fromHex(vecs[i].namespace)); | |
| Namespace memory ns = toNamespace(nsString); | |
| bytes memory data = fromHex(vecs[i].data); | |
| (bytes[] memory shares, bool err) = _bytesToSharesV0(data, ns); | |
| bytes32 commitment = _createCommitment(shares, ns); | |
| if (!compareStrings(_bytesToHexString(abi.encodePacked(commitment)), vecs[i].commitment)) { | |
| console.log("Commitment mismatch for vector ", i); | |
| console.log("expected: ", vecs[i].commitment); | |
| console.log("got: ", _bytesToHexString(abi.encodePacked(commitment))); | |
| return; | |
| } | |
| } | |
| console.log("All good :)"); |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/lib/commitment/test/Commitment.t.sol (1 hunks)
- src/lib/tree/namespace/NamespaceMerkleTree.sol (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/lib/commitment/test/Commitment.t.sol
- src/lib/tree/namespace/NamespaceMerkleTree.sol
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/lib/commitment/Commitment.sol (1 hunks)
- src/lib/commitment/test/Commitment.t.sol (1 hunks)
- src/lib/tree/namespace/NamespaceMerkleTree.sol (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/lib/commitment/Commitment.sol
- src/lib/commitment/test/Commitment.t.sol
- src/lib/tree/namespace/NamespaceMerkleTree.sol
Overview
solidity implementation of bytesToShares and createCommitment
Summary by CodeRabbit
New Features
Bug Fixes
Chores