Fixed unsafeCopy() function. Added more getters for pointers. (#101)

* Fixed `unsafeCopy()` function. Added more getters for pointers. * simplify a bit --------- Co-authored-by: Artem Chystiakov <[email protected]>
dl-solarity · May 13, 2024 · 59b1654 · 59b1654
1 parent af7e26e
commit 59b1654
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 63 deletions.
diff --git a/contracts/libs/utils/MemoryUtils.sol b/contracts/libs/utils/MemoryUtils.sol
@@ -7,27 +7,29 @@ pragma solidity ^0.8.4;
  */
 library MemoryUtils {
     /**
-     * @notice Copies the contents of the source string to the destination string.
+     * @notice Copies the contents of the source bytes to the destination bytes. strings can be casted
+     * to bytes in order to use this function.
      *
-     * @param source_ The source string to copy from.
-     * @return destination_ The newly allocated string.
+     * @param source_ The source bytes to copy from.
+     * @return destination_ The newly allocated bytes.
      */
-    function copy(string memory source_) internal view returns (string memory destination_) {
-        destination_ = new string(bytes(source_).length);
+    function copy(bytes memory source_) internal view returns (bytes memory destination_) {
+        destination_ = new bytes(source_.length);
 
-        unsafeMemoryCopy(getPointer(source_), getPointer(destination_), bytes(source_).length);
+        unsafeCopy(getDataPointer(source_), getDataPointer(destination_), source_.length);
     }
 
     /**
-     * @notice Copies the contents of the source bytes to the destination bytes.
+     * @notice Copies the contents of the source bytes32 array to the destination bytes32 array.
+     * uint256[], address[] array can be casted to bytes32[] via `TypeCaster` library.
      *
-     * @param source_ The source bytes to copy from.
-     * @return destination_ The newly allocated bytes.
+     * @param source_ The source bytes32 array to copy from.
+     * @return destination_ The newly allocated bytes32 array.
      */
-    function copy(bytes memory source_) internal view returns (bytes memory destination_) {
-        destination_ = new bytes(source_.length);
+    function copy(bytes32[] memory source_) internal view returns (bytes32[] memory destination_) {
+        destination_ = new bytes32[](source_.length);
 
-        unsafeMemoryCopy(getPointer(source_), getPointer(destination_), source_.length);
+        unsafeCopy(getDataPointer(source_), getDataPointer(destination_), source_.length * 32);
     }
 
     /**
@@ -41,40 +43,51 @@ library MemoryUtils {
      * This signature of calling identity precompile is:
      * staticcall(gas(), address(0x04), argsOffset, argsSize, retOffset, retSize)
      */
-    function unsafeMemoryCopy(
+    function unsafeCopy(
         uint256 sourcePointer_,
         uint256 destinationPointer_,
         uint256 size_
     ) internal view {
         assembly {
-            pop(
-                staticcall(
-                    gas(),
-                    4,
-                    add(sourcePointer_, 32),
-                    size_,
-                    add(destinationPointer_, 32),
-                    size_
-                )
-            )
+            pop(staticcall(gas(), 4, sourcePointer_, size_, destinationPointer_, size_))
+        }
+    }
+
+    /**
+     * @notice Returns the memory pointer to the given bytes starting position including the length.
+     */
+    function getPointer(bytes memory data_) internal pure returns (uint256 pointer_) {
+        assembly {
+            pointer_ := data_
+        }
+    }
+
+    /**
+     * @notice Returns the memory pointer to the given bytes starting position including the length.
+     * Cast uint256[] and address[] to bytes32[] via `TypeCaster` library.
+     */
+    function getPointer(bytes32[] memory data_) internal pure returns (uint256 pointer_) {
+        assembly {
+            pointer_ := data_
         }
     }
 
     /**
-     * @notice Returns the memory pointer of the given bytes data.
+     * @notice Returns the memory pointer to the given bytes data starting position skipping the length.
      */
-    function getPointer(bytes memory data) internal pure returns (uint256 pointer) {
+    function getDataPointer(bytes memory data_) internal pure returns (uint256 pointer_) {
         assembly {
-            pointer := data
+            pointer_ := add(data_, 32)
         }
     }
 
     /**
-     * @notice Returns the memory pointer of the given string data.
+     * @notice Returns the memory pointer to the given bytes data starting position skipping the length.
+     * Cast uint256[] and address[] to bytes32[] via `TypeCaster` library.
      */
-    function getPointer(string memory data) internal pure returns (uint256 pointer) {
+    function getDataPointer(bytes32[] memory data_) internal pure returns (uint256 pointer_) {
         assembly {
-            pointer := data
+            pointer_ := add(data_, 32)
         }
     }
 }
diff --git a/contracts/mock/libs/utils/MemoryUtilsMock.sol b/contracts/mock/libs/utils/MemoryUtilsMock.sol
@@ -2,81 +2,95 @@
 pragma solidity ^0.8.4;
 
 import {MemoryUtils} from "../../../libs/utils/MemoryUtils.sol";
+import {TypeCaster} from "../../../libs/utils/TypeCaster.sol";
 
 contract MemoryUtilsMock {
+    using TypeCaster for *;
     using MemoryUtils for *;
 
-    function testStringMemoryCopy(string memory data_) external view {
-        string memory someString = new string(bytes(data_).length);
+    function testBytesMemoryCopy(bytes memory data_) external view {
+        bytes memory someBytes_ = new bytes(data_.length);
 
         require(
-            keccak256(bytes(data_)) != keccak256(bytes(someString)),
-            "MemoryUtilsMock: testStringMemoryCopy failed. Initial data and someString are equal"
+            keccak256(data_) != keccak256(someBytes_),
+            "MemoryUtilsMock: testBytesMemoryCopy failed. Initial data and someBytes are equal"
         );
 
-        someString = data_.copy();
+        someBytes_ = data_.copy();
 
         require(
-            keccak256(bytes(data_)) == keccak256(bytes(someString)),
-            "MemoryUtilsMock: testStringMemoryCopy failed. Initial data and someString are not equal"
+            keccak256(data_) == keccak256(someBytes_),
+            "MemoryUtilsMock: testBytesMemoryCopy failed. Initial data and someBytes are not equal"
         );
     }
 
-    function testBytesMemoryCopy(bytes memory data_) external view {
-        bytes memory someBytes = new bytes(data_.length);
+    function testBytes32MemoryCopy(bytes32[] memory data_) external view {
+        bytes32[] memory someBytes_ = new bytes32[](data_.length);
 
         require(
-            keccak256(data_) != keccak256(someBytes),
-            "MemoryUtilsMock: testBytesMemoryCopy failed. Initial data and someBytes are equal"
+            keccak256(abi.encode(data_)) != keccak256(abi.encode(someBytes_)),
+            "MemoryUtilsMock: testBytes32MemoryCopy failed. Initial data and someBytes are equal"
         );
 
-        someBytes = data_.copy();
+        someBytes_ = data_.copy();
 
         require(
-            keccak256(data_) == keccak256(someBytes),
-            "MemoryUtilsMock: testBytesMemoryCopy failed. Initial data and someBytes are not equal"
+            keccak256(abi.encode(data_)) == keccak256(abi.encode(someBytes_)),
+            "MemoryUtilsMock: testBytes32MemoryCopy failed. Initial data and someBytes are not equal"
         );
     }
 
     function testUnsafeMemoryCopy(bytes memory data_) external view {
-        bytes memory someBytes = new bytes(data_.length);
+        bytes memory someBytes_ = new bytes(data_.length);
 
         require(
-            keccak256(data_) != keccak256(someBytes),
+            keccak256(data_) != keccak256(someBytes_),
             "MemoryUtilsMock: testBigMemory failed. Initial data and someBytes are equal"
         );
 
-        MemoryUtils.unsafeMemoryCopy(
-            MemoryUtils.getPointer(someBytes),
-            MemoryUtils.getPointer(data_),
-            someBytes.length
+        MemoryUtils.unsafeCopy(
+            MemoryUtils.getDataPointer(someBytes_),
+            MemoryUtils.getDataPointer(data_),
+            someBytes_.length
         );
 
         require(
-            keccak256(data_) == keccak256(someBytes),
+            keccak256(data_) == keccak256(someBytes_),
             "MemoryUtilsMock: testBigMemory failed. Initial data and someBytes are not equal"
         );
     }
 
     function testPartialCopy(bytes memory data_) external view {
-        bytes memory someBytes = new bytes(data_.length / 2);
+        bytes memory someBytes_ = new bytes(data_.length / 2);
 
         require(
-            keccak256(data_) != keccak256(someBytes),
+            keccak256(data_) != keccak256(someBytes_),
             "MemoryUtilsMock: testPartialCopy failed. Initial data and someBytes are equal"
         );
 
-        MemoryUtils.unsafeMemoryCopy(
-            MemoryUtils.getPointer(someBytes),
-            MemoryUtils.getPointer(data_),
-            someBytes.length
+        MemoryUtils.unsafeCopy(
+            MemoryUtils.getDataPointer(someBytes_),
+            MemoryUtils.getDataPointer(data_),
+            someBytes_.length
         );
 
-        for (uint256 i = 0; i < someBytes.length; i++) {
+        for (uint256 i = 0; i < someBytes_.length; i++) {
             require(
-                someBytes[i] == data_[i],
+                someBytes_[i] == data_[i],
                 "MemoryUtilsMock: testPartialCopy failed. Initial data and someBytes are not equal"
             );
         }
     }
+
+    /**
+     * @dev Since the underlying logic of `getPointer()/getDataPointer()` is only specific to EVMs,
+     * we only do a simple mock test for coverage.
+     */
+    function testForCoverage() external pure {
+        MemoryUtils.getPointer(new bytes(1));
+        MemoryUtils.getPointer((new uint256[](1)).asBytes32Array());
+
+        MemoryUtils.getDataPointer(new bytes(1));
+        MemoryUtils.getDataPointer((new uint256[](1)).asBytes32Array());
+    }
 }
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@solarity/solidity-lib",
-  "version": "2.7.5",
+  "version": "2.7.6",
   "license": "MIT",
   "author": "Distributed Lab",
   "readme": "README.md",

diff --git a/test/libs/utils/MemoryUtils.test.ts b/test/libs/utils/MemoryUtils.test.ts
@@ -24,8 +24,9 @@ describe("MemoryUtils", () => {
       await expect(mock.testBytesMemoryCopy(ethers.randomBytes(20))).to.be.eventually.fulfilled;
     });
 
-    it("should copy arbitrary chunks of memory (string)", async () => {
-      await expect(mock.testStringMemoryCopy("Hello, world!")).to.be.eventually.fulfilled;
+    it("should copy arbitrary chunks of memory (bytes32[])", async () => {
+      await expect(mock.testBytes32MemoryCopy([ethers.randomBytes(32), ethers.randomBytes(32)])).to.be.eventually
+        .fulfilled;
     });
 
     it("should copy 20 bytes of memory", async () => {
@@ -47,5 +48,9 @@ describe("MemoryUtils", () => {
     it("should copy partial chunks of memory", async () => {
       await expect(mock.testPartialCopy(ethers.randomBytes(15))).to.be.eventually.fulfilled;
     });
+
+    it("should cover getter functions", async () => {
+      await expect(mock.testForCoverage()).to.be.eventually.fulfilled;
+    });
   });
 });