diff --git a/src/unaudited/BLSSigCheckOperatorStateRetriever.sol b/src/unaudited/BLSSigCheckOperatorStateRetriever.sol index 1f511c62..9333ff47 100644 --- a/src/unaudited/BLSSigCheckOperatorStateRetriever.sol +++ b/src/unaudited/BLSSigCheckOperatorStateRetriever.sol @@ -10,7 +10,7 @@ import {BitmapUtils} from "../libraries/BitmapUtils.sol"; import {BN254} from "../libraries/BN254.sol"; import {BN256G2} from "./BN256G2.sol"; import {OperatorStateRetriever} from "../OperatorStateRetriever.sol"; -import {ECUtils} from "./ECUtils.sol"; +import {BLSSigCheckUtils, Arrays} from "./BLSSigCheckUtils.sol"; /** * @title BLSSigCheckOperatorStateRetriever with view functions that allow to retrieve the state of an AVSs registry system. @@ -18,9 +18,10 @@ import {ECUtils} from "./ECUtils.sol"; * @author Bread coop */ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever { - using ECUtils for BN254.G1Point; + using BLSSigCheckUtils for BN254.G1Point; using BN254 for BN254.G1Point; using BitmapUtils for uint256; + using Arrays for bytes32[]; /// @dev Thrown when the signature is not on the curve. error InvalidSigma(); @@ -150,9 +151,13 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever { // Trim the nonSignerOperatorIds array to the actual count bytes32[] memory trimmedNonSignerOperatorIds = new bytes32[](nonSignerOperatorsCount); - BN254.G1Point[] memory nonSignerPubkeys = new BN254.G1Point[](nonSignerOperatorsCount); for (uint256 i = 0; i < nonSignerOperatorsCount; i++) { trimmedNonSignerOperatorIds[i] = nonSignerOperatorIds[i]; + } + trimmedNonSignerOperatorIds = Arrays.sort(trimmedNonSignerOperatorIds); + + BN254.G1Point[] memory nonSignerPubkeys = new BN254.G1Point[](nonSignerOperatorsCount); + for (uint256 i = 0; i < nonSignerOperatorsCount; i++) { address nonSignerOperator = registryCoordinator.getOperatorFromId(trimmedNonSignerOperatorIds[i]); (nonSignerPubkeys[i],) = m.blsApkRegistry.getRegisteredPubkey(nonSignerOperator); diff --git a/src/unaudited/BLSSigCheckUtils.sol b/src/unaudited/BLSSigCheckUtils.sol new file mode 100644 index 00000000..d2abaa77 --- /dev/null +++ b/src/unaudited/BLSSigCheckUtils.sol @@ -0,0 +1,805 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.20; + +import {BN254} from "../libraries/BN254.sol"; +import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; + +// The following libraries (Comparators, Arrays and SlotDerivation) are copy-pasted from OpenZeppelin v5.1 +// since an earlier version of OpenZeppelin is being used that doesn't include these utilities. +// This allows us to use these newer utilities without upgrading the entire dependency and messing with already +// existing code. +/** + * @dev Provides a set of functions to compare values. + * + * _Available since v5.1._ + */ +library Comparators { + function lt(uint256 a, uint256 b) internal pure returns (bool) { + return a < b; + } + + function gt(uint256 a, uint256 b) internal pure returns (bool) { + return a > b; + } +} + +/** + * @dev Library for computing storage (and transient storage) locations from namespaces and deriving slots + * corresponding to standard patterns. The derivation method for array and mapping matches the storage layout used by + * the solidity language / compiler. + * + * See https://docs.soliditylang.org/en/v0.8.20/internals/layout_in_storage.html#mappings-and-dynamic-arrays[Solidity docs for mappings and dynamic arrays.]. + * + * Example usage: + * ```solidity + * contract Example { + * // Add the library methods + * using StorageSlot for bytes32; + * using SlotDerivation for bytes32; + * + * // Declare a namespace + * string private constant _NAMESPACE = ""; // eg. OpenZeppelin.Slot + * + * function setValueInNamespace(uint256 key, address newValue) internal { + * _NAMESPACE.erc7201Slot().deriveMapping(key).getAddressSlot().value = newValue; + * } + * + * function getValueInNamespace(uint256 key) internal view returns (address) { + * return _NAMESPACE.erc7201Slot().deriveMapping(key).getAddressSlot().value; + * } + * } + * ``` + * + * TIP: Consider using this library along with {StorageSlot}. + * + * NOTE: This library provides a way to manipulate storage locations in a non-standard way. Tooling for checking + * upgrade safety will ignore the slots accessed through this library. + * + * _Available since v5.1._ + */ +library SlotDerivation { + /** + * @dev Derive an ERC-7201 slot from a string (namespace). + */ + function erc7201Slot( + string memory namespace + ) internal pure returns (bytes32 slot) { + assembly ("memory-safe") { + mstore(0x00, sub(keccak256(add(namespace, 0x20), mload(namespace)), 1)) + slot := and(keccak256(0x00, 0x20), not(0xff)) + } + } + + /** + * @dev Add an offset to a slot to get the n-th element of a structure or an array. + */ + function offset(bytes32 slot, uint256 pos) internal pure returns (bytes32 result) { + unchecked { + return bytes32(uint256(slot) + pos); + } + } + + /** + * @dev Derive the location of the first element in an array from the slot where the length is stored. + */ + function deriveArray( + bytes32 slot + ) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + mstore(0x00, slot) + result := keccak256(0x00, 0x20) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping(bytes32 slot, address key) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + mstore(0x00, and(key, shr(96, not(0)))) + mstore(0x20, slot) + result := keccak256(0x00, 0x40) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping(bytes32 slot, bool key) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + mstore(0x00, iszero(iszero(key))) + mstore(0x20, slot) + result := keccak256(0x00, 0x40) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping(bytes32 slot, bytes32 key) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + mstore(0x00, key) + mstore(0x20, slot) + result := keccak256(0x00, 0x40) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping(bytes32 slot, uint256 key) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + mstore(0x00, key) + mstore(0x20, slot) + result := keccak256(0x00, 0x40) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping(bytes32 slot, int256 key) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + mstore(0x00, key) + mstore(0x20, slot) + result := keccak256(0x00, 0x40) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping( + bytes32 slot, + string memory key + ) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + let length := mload(key) + let begin := add(key, 0x20) + let end := add(begin, length) + let cache := mload(end) + mstore(end, slot) + result := keccak256(begin, add(length, 0x20)) + mstore(end, cache) + } + } + + /** + * @dev Derive the location of a mapping element from the key. + */ + function deriveMapping(bytes32 slot, bytes memory key) internal pure returns (bytes32 result) { + assembly ("memory-safe") { + let length := mload(key) + let begin := add(key, 0x20) + let end := add(begin, length) + let cache := mload(end) + mstore(end, slot) + result := keccak256(begin, add(length, 0x20)) + mstore(end, cache) + } + } +} + +/** + * @dev Collection of functions related to array types. + */ +library Arrays { + using SlotDerivation for bytes32; + using StorageSlot for bytes32; + + /** + * @dev Sort an array of uint256 (in memory) following the provided comparator function. + * + * This function does the sorting "in place", meaning that it overrides the input. The object is returned for + * convenience, but that returned value can be discarded safely if the caller has a memory pointer to the array. + * + * NOTE: this function's cost is `O(n · log(n))` in average and `O(n²)` in the worst case, with n the length of the + * array. Using it in view functions that are executed through `eth_call` is safe, but one should be very careful + * when executing this as part of a transaction. If the array being sorted is too large, the sort operation may + * consume more gas than is available in a block, leading to potential DoS. + * + * IMPORTANT: Consider memory side-effects when using custom comparator functions that access memory in an unsafe way. + */ + function sort( + uint256[] memory array, + function(uint256, uint256) pure returns (bool) comp + ) internal pure returns (uint256[] memory) { + _quickSort(_begin(array), _end(array), comp); + return array; + } + + /** + * @dev Variant of {sort} that sorts an array of uint256 in increasing order. + */ + function sort( + uint256[] memory array + ) internal pure returns (uint256[] memory) { + sort(array, Comparators.lt); + return array; + } + + /** + * @dev Sort an array of address (in memory) following the provided comparator function. + * + * This function does the sorting "in place", meaning that it overrides the input. The object is returned for + * convenience, but that returned value can be discarded safely if the caller has a memory pointer to the array. + * + * NOTE: this function's cost is `O(n · log(n))` in average and `O(n²)` in the worst case, with n the length of the + * array. Using it in view functions that are executed through `eth_call` is safe, but one should be very careful + * when executing this as part of a transaction. If the array being sorted is too large, the sort operation may + * consume more gas than is available in a block, leading to potential DoS. + * + * IMPORTANT: Consider memory side-effects when using custom comparator functions that access memory in an unsafe way. + */ + function sort( + address[] memory array, + function(address, address) pure returns (bool) comp + ) internal pure returns (address[] memory) { + sort(_castToUint256Array(array), _castToUint256Comp(comp)); + return array; + } + + /** + * @dev Variant of {sort} that sorts an array of address in increasing order. + */ + function sort( + address[] memory array + ) internal pure returns (address[] memory) { + sort(_castToUint256Array(array), Comparators.lt); + return array; + } + + /** + * @dev Sort an array of bytes32 (in memory) following the provided comparator function. + * + * This function does the sorting "in place", meaning that it overrides the input. The object is returned for + * convenience, but that returned value can be discarded safely if the caller has a memory pointer to the array. + * + * NOTE: this function's cost is `O(n · log(n))` in average and `O(n²)` in the worst case, with n the length of the + * array. Using it in view functions that are executed through `eth_call` is safe, but one should be very careful + * when executing this as part of a transaction. If the array being sorted is too large, the sort operation may + * consume more gas than is available in a block, leading to potential DoS. + * + * IMPORTANT: Consider memory side-effects when using custom comparator functions that access memory in an unsafe way. + */ + function sort( + bytes32[] memory array, + function(bytes32, bytes32) pure returns (bool) comp + ) internal pure returns (bytes32[] memory) { + sort(_castToUint256Array(array), _castToUint256Comp(comp)); + return array; + } + + /** + * @dev Variant of {sort} that sorts an array of bytes32 in increasing order. + */ + function sort( + bytes32[] memory array + ) internal pure returns (bytes32[] memory) { + sort(_castToUint256Array(array), Comparators.lt); + return array; + } + + /** + * @dev Performs a quick sort of a segment of memory. The segment sorted starts at `begin` (inclusive), and stops + * at end (exclusive). Sorting follows the `comp` comparator. + * + * Invariant: `begin <= end`. This is the case when initially called by {sort} and is preserved in subcalls. + * + * IMPORTANT: Memory locations between `begin` and `end` are not validated/zeroed. This function should + * be used only if the limits are within a memory array. + */ + function _quickSort( + uint256 begin, + uint256 end, + function(uint256, uint256) pure returns (bool) comp + ) private pure { + unchecked { + if (end - begin < 0x40) return; + + // Use first element as pivot + uint256 pivot = _mload(begin); + // Position where the pivot should be at the end of the loop + uint256 pos = begin; + + for (uint256 it = begin + 0x20; it < end; it += 0x20) { + if (comp(_mload(it), pivot)) { + // If the value stored at the iterator's position comes before the pivot, we increment the + // position of the pivot and move the value there. + pos += 0x20; + _swap(pos, it); + } + } + + _swap(begin, pos); // Swap pivot into place + _quickSort(begin, pos, comp); // Sort the left side of the pivot + _quickSort(pos + 0x20, end, comp); // Sort the right side of the pivot + } + } + + /** + * @dev Pointer to the memory location of the first element of `array`. + */ + function _begin( + uint256[] memory array + ) private pure returns (uint256 ptr) { + assembly ("memory-safe") { + ptr := add(array, 0x20) + } + } + + /** + * @dev Pointer to the memory location of the first memory word (32bytes) after `array`. This is the memory word + * that comes just after the last element of the array. + */ + function _end( + uint256[] memory array + ) private pure returns (uint256 ptr) { + unchecked { + return _begin(array) + array.length * 0x20; + } + } + + /** + * @dev Load memory word (as a uint256) at location `ptr`. + */ + function _mload( + uint256 ptr + ) private pure returns (uint256 value) { + assembly { + value := mload(ptr) + } + } + + /** + * @dev Swaps the elements memory location `ptr1` and `ptr2`. + */ + function _swap(uint256 ptr1, uint256 ptr2) private pure { + assembly { + let value1 := mload(ptr1) + let value2 := mload(ptr2) + mstore(ptr1, value2) + mstore(ptr2, value1) + } + } + + /// @dev Helper: low level cast address memory array to uint256 memory array + function _castToUint256Array( + address[] memory input + ) private pure returns (uint256[] memory output) { + assembly { + output := input + } + } + + /// @dev Helper: low level cast bytes32 memory array to uint256 memory array + function _castToUint256Array( + bytes32[] memory input + ) private pure returns (uint256[] memory output) { + assembly { + output := input + } + } + + /// @dev Helper: low level cast address comp function to uint256 comp function + function _castToUint256Comp( + function(address, address) pure returns (bool) input + ) private pure returns (function(uint256, uint256) pure returns (bool) output) { + assembly { + output := input + } + } + + /// @dev Helper: low level cast bytes32 comp function to uint256 comp function + function _castToUint256Comp( + function(bytes32, bytes32) pure returns (bool) input + ) private pure returns (function(uint256, uint256) pure returns (bool) output) { + assembly { + output := input + } + } + + /** + * @dev Searches a sorted `array` and returns the first index that contains + * a value greater or equal to `element`. If no such index exists (i.e. all + * values in the array are strictly less than `element`), the array length is + * returned. Time complexity O(log n). + * + * NOTE: The `array` is expected to be sorted in ascending order, and to + * contain no repeated elements. + * + * IMPORTANT: Deprecated. This implementation behaves as {lowerBound} but lacks + * support for repeated elements in the array. The {lowerBound} function should + * be used instead. + */ + function findUpperBound( + uint256[] storage array, + uint256 element + ) internal view returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeAccess(array, mid).value > element) { + high = mid; + } else { + low = mid + 1; + } + } + + // At this point `low` is the exclusive upper bound. We will return the inclusive upper bound. + if (low > 0 && unsafeAccess(array, low - 1).value == element) { + return low - 1; + } else { + return low; + } + } + + /** + * @dev Searches an `array` sorted in ascending order and returns the first + * index that contains a value greater or equal than `element`. If no such index + * exists (i.e. all values in the array are strictly less than `element`), the array + * length is returned. Time complexity O(log n). + * + * See C++'s https://en.cppreference.com/w/cpp/algorithm/lower_bound[lower_bound]. + */ + function lowerBound(uint256[] storage array, uint256 element) internal view returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeAccess(array, mid).value < element) { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } else { + high = mid; + } + } + + return low; + } + + /** + * @dev Searches an `array` sorted in ascending order and returns the first + * index that contains a value strictly greater than `element`. If no such index + * exists (i.e. all values in the array are strictly less than `element`), the array + * length is returned. Time complexity O(log n). + * + * See C++'s https://en.cppreference.com/w/cpp/algorithm/upper_bound[upper_bound]. + */ + function upperBound(uint256[] storage array, uint256 element) internal view returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeAccess(array, mid).value > element) { + high = mid; + } else { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } + } + + return low; + } + + /** + * @dev Same as {lowerBound}, but with an array in memory. + */ + function lowerBoundMemory( + uint256[] memory array, + uint256 element + ) internal pure returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeMemoryAccess(array, mid) < element) { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } else { + high = mid; + } + } + + return low; + } + + /** + * @dev Same as {upperBound}, but with an array in memory. + */ + function upperBoundMemory( + uint256[] memory array, + uint256 element + ) internal pure returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeMemoryAccess(array, mid) > element) { + high = mid; + } else { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } + } + + return low; + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeAccess( + address[] storage arr, + uint256 pos + ) internal pure returns (StorageSlot.AddressSlot storage) { + bytes32 slot; + assembly ("memory-safe") { + slot := arr.slot + } + return slot.deriveArray().offset(pos).getAddressSlot(); + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeAccess( + bytes32[] storage arr, + uint256 pos + ) internal pure returns (StorageSlot.Bytes32Slot storage) { + bytes32 slot; + assembly ("memory-safe") { + slot := arr.slot + } + return slot.deriveArray().offset(pos).getBytes32Slot(); + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeAccess( + uint256[] storage arr, + uint256 pos + ) internal pure returns (StorageSlot.Uint256Slot storage) { + bytes32 slot; + assembly ("memory-safe") { + slot := arr.slot + } + return slot.deriveArray().offset(pos).getUint256Slot(); + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeAccess( + bytes[] storage arr, + uint256 pos + ) internal pure returns (StorageSlot.BytesSlot storage) { + bytes32 slot; + assembly ("memory-safe") { + slot := arr.slot + } + return slot.deriveArray().offset(pos).getBytesSlot(); + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeAccess( + string[] storage arr, + uint256 pos + ) internal pure returns (StorageSlot.StringSlot storage) { + bytes32 slot; + assembly ("memory-safe") { + slot := arr.slot + } + return slot.deriveArray().offset(pos).getStringSlot(); + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeMemoryAccess( + address[] memory arr, + uint256 pos + ) internal pure returns (address res) { + assembly { + res := mload(add(add(arr, 0x20), mul(pos, 0x20))) + } + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeMemoryAccess( + bytes32[] memory arr, + uint256 pos + ) internal pure returns (bytes32 res) { + assembly { + res := mload(add(add(arr, 0x20), mul(pos, 0x20))) + } + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeMemoryAccess( + uint256[] memory arr, + uint256 pos + ) internal pure returns (uint256 res) { + assembly { + res := mload(add(add(arr, 0x20), mul(pos, 0x20))) + } + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeMemoryAccess( + bytes[] memory arr, + uint256 pos + ) internal pure returns (bytes memory res) { + assembly { + res := mload(add(add(arr, 0x20), mul(pos, 0x20))) + } + } + + /** + * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. + * + * WARNING: Only use if you are certain `pos` is lower than the array length. + */ + function unsafeMemoryAccess( + string[] memory arr, + uint256 pos + ) internal pure returns (string memory res) { + assembly { + res := mload(add(add(arr, 0x20), mul(pos, 0x20))) + } + } + + /** + * @dev Helper to set the length of a dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(address[] storage array, uint256 len) internal { + assembly ("memory-safe") { + sstore(array.slot, len) + } + } + + /** + * @dev Helper to set the length of a dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(bytes32[] storage array, uint256 len) internal { + assembly ("memory-safe") { + sstore(array.slot, len) + } + } + + /** + * @dev Helper to set the length of a dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(uint256[] storage array, uint256 len) internal { + assembly ("memory-safe") { + sstore(array.slot, len) + } + } + + /** + * @dev Helper to set the length of a dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(bytes[] storage array, uint256 len) internal { + assembly ("memory-safe") { + sstore(array.slot, len) + } + } + + /** + * @dev Helper to set the length of a dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(string[] storage array, uint256 len) internal { + assembly ("memory-safe") { + sstore(array.slot, len) + } + } +} + +/** + * @title ECUtils + * @notice Library containing utility functions for elliptic curve operations + */ +library BLSSigCheckUtils { + /** + * @notice Checks if a point lies on the BN254 elliptic curve + * @dev The curve equation is y^2 = x^3 + 3 (mod p) + * @param p The point to check, in G1 + * @return true if the point lies on the curve, false otherwise + */ + function isOnCurve( + BN254.G1Point memory p + ) internal pure returns (bool) { + uint256 y2 = mulmod(p.Y, p.Y, BN254.FP_MODULUS); + uint256 x2 = mulmod(p.X, p.X, BN254.FP_MODULUS); + uint256 x3 = mulmod(p.X, x2, BN254.FP_MODULUS); + uint256 rhs = addmod(x3, 3, BN254.FP_MODULUS); + return y2 == rhs; + } +} diff --git a/src/unaudited/ECUtils.sol b/src/unaudited/ECUtils.sol deleted file mode 100644 index 0b0e772a..00000000 --- a/src/unaudited/ECUtils.sol +++ /dev/null @@ -1,26 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.12; - -import {BN254} from "../libraries/BN254.sol"; - -/** - * @title ECUtils - * @notice Library containing utility functions for elliptic curve operations - */ -library ECUtils { - /** - * @notice Checks if a point lies on the BN254 elliptic curve - * @dev The curve equation is y^2 = x^3 + 3 (mod p) - * @param p The point to check, in G1 - * @return true if the point lies on the curve, false otherwise - */ - function isOnCurve( - BN254.G1Point memory p - ) internal pure returns (bool) { - uint256 y2 = mulmod(p.Y, p.Y, BN254.FP_MODULUS); - uint256 x2 = mulmod(p.X, p.X, BN254.FP_MODULUS); - uint256 x3 = mulmod(p.X, x2, BN254.FP_MODULUS); - uint256 rhs = addmod(x3, 3, BN254.FP_MODULUS); - return y2 == rhs; - } -} diff --git a/test/unit/BLSSigCheckOperatorStateRetriever.t.sol b/test/unit/BLSSigCheckOperatorStateRetriever.t.sol index ee73b467..b47c052c 100644 --- a/test/unit/BLSSigCheckOperatorStateRetriever.t.sol +++ b/test/unit/BLSSigCheckOperatorStateRetriever.t.sol @@ -657,6 +657,152 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is ); } + function test_getNonSignerStakesAndSignature_nonSignersAreSorted() public { + // Setup - register multiple operators + uint256 quorumBitmap = 1; // Quorum 0 only + cheats.roll(registrationBlockNumber); + + // Register 4 operators with different addresses + address[] memory operators = new address[](4); + operators[0] = _incrementAddress(defaultOperator, 1); // Lowest address + operators[1] = _incrementAddress(defaultOperator, 2); + operators[2] = _incrementAddress(defaultOperator, 3); + operators[3] = _incrementAddress(defaultOperator, 4); // Highest address + + // Register each operator with a different pubkey, incrementing the block each time + for (uint256 i = 0; i < operators.length; i++) { + BN254.G1Point memory pubKey = BN254.scalar_mul_tiny(BN254.generatorG1(), uint16(i + 1)); + _registerOperatorWithCoordinator(operators[i], quorumBitmap, pubKey); + cheats.roll(block.number + 1); // increment block after each registration + } + + // Create G2 points for all operators + for (uint256 i = 0; i < operators.length; i++) { + BN254.G2Point memory opG2 = _makeG2Point(i + 2); + vm.mockCall( + address(blsApkRegistry), + abi.encodeWithSelector(IBLSApkRegistry.getOperatorPubkeyG2.selector, operators[i]), + abi.encode(opG2) + ); + } + + // Create a dummy signature + BN254.G1Point memory dummySigma = BN254.scalar_mul_tiny(BN254.generatorG1(), 123); + + // Only have operators[0] and operators[2] sign + address[] memory signingOperators = new address[](2); + signingOperators[0] = operators[0]; + signingOperators[1] = operators[2]; + + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(uint8(0)); + + // Call the function under test + IBLSSignatureCheckerTypes.NonSignerStakesAndSignature memory result = + sigCheckOperatorStateRetriever.getNonSignerStakesAndSignature( + registryCoordinator, quorumNumbers, dummySigma, signingOperators, uint32(block.number) + ); + + // Verify we have 2 non-signers + assertEq(result.nonSignerQuorumBitmapIndices.length, 2, "Should have 2 non-signers"); + assertEq(result.nonSignerPubkeys.length, 2, "Should have 2 non-signer pubkeys"); + + // Verify the non-signers are operators[1] and operators[3] + assertEq( + result.nonSignerQuorumBitmapIndices[0], 0, "First non-signer should be operators[1]" + ); + assertEq( + result.nonSignerQuorumBitmapIndices[1], 0, "Second non-signer should be operators[3]" + ); + + // Verify the non-signers are sorted by pubkey hash + for (uint256 i = 1; i < result.nonSignerPubkeys.length; i++) { + bytes32 hash_i = result.nonSignerPubkeys[i].hashG1Point(); + bytes32 hash_prev = result.nonSignerPubkeys[i - 1].hashG1Point(); + assertTrue( + uint256(hash_i) > uint256(hash_prev), + "Non-signer pubkeys should be sorted by hash in ascending order" + ); + } + } + + function test_getNonSignerStakesAndSignature_nonSignersAreSorted_fuzzed( + uint8 numOperators, + uint8 numSigners + ) public { + // Bound the fuzzed values to reasonable ranges + numOperators = uint8(bound(numOperators, 5, 10)); // At least 5 operators, max 10 + numSigners = uint8(bound(numSigners, 2, numOperators - 1)); // At least 2 signers, but not all operators + + // Setup - register multiple operators + uint256 quorumBitmap = 1; // Quorum 0 only + cheats.roll(registrationBlockNumber); + + // Register operators with different addresses + address[] memory operators = new address[](numOperators); + for (uint256 i = 0; i < numOperators; i++) { + operators[i] = _incrementAddress(defaultOperator, i + 1); + } + + // Register each operator with a different pubkey + for (uint256 i = 0; i < operators.length; i++) { + BN254.G1Point memory pubKey = BN254.scalar_mul_tiny(BN254.generatorG1(), uint16(i + 1)); + _registerOperatorWithCoordinator(operators[i], quorumBitmap, pubKey); + cheats.roll(block.number + 1); // increment block after each registration + } + + // Create G2 points for all operators + for (uint256 i = 0; i < operators.length; i++) { + BN254.G2Point memory opG2 = _makeG2Point(i + 2); + vm.mockCall( + address(blsApkRegistry), + abi.encodeWithSelector(IBLSApkRegistry.getOperatorPubkeyG2.selector, operators[i]), + abi.encode(opG2) + ); + } + + // Create a dummy signature + BN254.G1Point memory dummySigma = BN254.scalar_mul_tiny(BN254.generatorG1(), 123); + + // Select random signers from the registered operators + address[] memory signingOperators = new address[](numSigners); + for (uint256 i = 0; i < numSigners; i++) { + signingOperators[i] = operators[i]; // Use first numSigners operators as signers + } + + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(uint8(0)); + + // Call the function under test + IBLSSignatureCheckerTypes.NonSignerStakesAndSignature memory result = + sigCheckOperatorStateRetriever.getNonSignerStakesAndSignature( + registryCoordinator, quorumNumbers, dummySigma, signingOperators, uint32(block.number) + ); + + // Verify we have the correct number of non-signers + uint256 expectedNonSigners = numOperators - numSigners; + assertEq( + result.nonSignerQuorumBitmapIndices.length, + expectedNonSigners, + "Should have correct number of non-signers" + ); + assertEq( + result.nonSignerPubkeys.length, + expectedNonSigners, + "Should have correct number of non-signer pubkeys" + ); + + // Verify the non-signers are sorted by pubkey hash + for (uint256 i = 1; i < result.nonSignerPubkeys.length; i++) { + bytes32 hash_i = result.nonSignerPubkeys[i].hashG1Point(); + bytes32 hash_prev = result.nonSignerPubkeys[i - 1].hashG1Point(); + assertTrue( + uint256(hash_i) > uint256(hash_prev), + "Non-signer pubkeys should be sorted by hash in ascending order" + ); + } + } + function _getApkAtBlocknumber( ISlashingRegistryCoordinator registryCoordinator, uint8 quorumNumber,