Skip to content

Commit

Permalink
add try variants + use for governor proposal parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
@Amxx
Amxx committed Sep 9, 2024
1 parent 6dca3cb commit a7a6e9e
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 122 deletions.
57 changes: 9 additions & 48 deletions contracts/governance/Governor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {DoubleEndedQueue} from "../utils/structs/DoubleEndedQueue.sol";
import {Address} from "../utils/Address.sol";
import {Context} from "../utils/Context.sol";
import {Nonces} from "../utils/Nonces.sol";
import {Strings} from "../utils/Strings.sol";
import {IGovernor, IERC6372} from "./IGovernor.sol";

/**
Expand Down Expand Up @@ -760,68 +761,28 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
address proposer,
string memory description
) internal view virtual returns (bool) {
uint256 len = bytes(description).length;
uint256 length = bytes(description).length;

// Length is too short to contain a valid proposer suffix
if (len < 52) {
if (length < 52) {
return true;
}

// Extract what would be the `#proposer=0x` marker beginning the suffix
bytes12 marker;
assembly {
// - Start of the string contents in memory = description + 32
// - First character of the marker = len - 52
// - Length of "#proposer=0x0000000000000000000000000000000000000000" = 52
// - We read the memory word starting at the first character of the marker:
// - (description + 32) + (len - 52) = description + (len - 20)
// - Note: Solidity will ignore anything past the first 12 bytes
marker := mload(add(description, sub(len, 20)))
}
bytes12 marker = bytes12(Strings.unsafeReadBytesOffset(bytes(description), length - 52));

// If the marker is not found, there is no proposer suffix to check
if (marker != bytes12("#proposer=0x")) {
return true;
}

// Parse the 40 characters following the marker as uint160
uint160 recovered = 0;
for (uint256 i = len - 40; i < len; ++i) {
(bool isHex, uint8 value) = _tryHexToUint(bytes(description)[i]);
// If any of the characters is not a hex digit, ignore the suffix entirely
if (!isHex) {
return true;
}
recovered = (recovered << 4) | value;
// Parse the 40 characters following the marker as address
(bool success, uint256 value) = Strings.tryHexToUint(description, length - 40, length);
if (!success) {
return true;
}

return recovered == uint160(proposer);
}

/**
* @dev Try to parse a character from a string as a hex value. Returns `(true, value)` if the char is in
* `[0-9a-fA-F]` and `(false, 0)` otherwise. Value is guaranteed to be in the range `0 <= value < 16`
*/
function _tryHexToUint(bytes1 char) private pure returns (bool, uint8) {
uint8 c = uint8(char);
unchecked {
// Case 0-9
if (47 < c && c < 58) {
return (true, c - 48);
}
// Case A-F
else if (64 < c && c < 71) {
return (true, c - 55);
}
// Case a-f
else if (96 < c && c < 103) {
return (true, c - 87);
}
// Else: not a hex char
else {
return (false, 0);
}
}
return address(uint160(value)) == proposer;
}

/**
Expand Down
162 changes: 135 additions & 27 deletions contracts/utils/Strings.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ library Strings {
/**
* @dev The string being parsed contains characters that are not in scope of the given base.
*/
error StringsInvalidChar(bytes1 chr, uint8 base);
error StringsInvalidChar();

/**
* @dev Converts a `uint256` to its ASCII `string` decimal representation.
Expand Down Expand Up @@ -130,15 +130,49 @@ library Strings {
* - the result does not fit in a `uint256`.
*/
function toUint(string memory input) internal pure returns (uint256) {
return toUint(input, 0, bytes(input).length);
}

/**
* @dev Variant of {toUint} that parses a substring of `input` located between position `begin` (included) and
* `end` (excluded).
*/
function toUint(string memory input, uint256 begin, uint256 end) internal pure returns (uint256) {
(bool success, uint256 value) = tryToUint(input, begin, end);
if (!success) revert StringsInvalidChar();
return value;
}

/**
* @dev Variant of {toUint-string} that returns false if the parsing fails because of an invalid character.
*
* This function will still revert if the result does not fit in a `uint256`
*/
function tryToUint(string memory input) internal pure returns (bool success, uint256 value) {
return tryToUint(input, 0, bytes(input).length);
}

/**
* @dev Variant of {toUint-string-uint256-uint256} that returns false if the parsing fails because of an invalid
* character.
*
* This function will still revert if the result does not fit in a `uint256`
*/
function tryToUint(
string memory input,
uint256 begin,
uint256 end
) internal pure returns (bool success, uint256 value) {
bytes memory buffer = bytes(input);

uint256 result = 0;
uint256 bufferLength = buffer.length;
for (uint256 i = 0; i < bufferLength; ++i) {
result *= 10; // will revert if overflow
result += _parseChr(buffer[i], 10);
for (uint256 i = begin; i < end; ++i) {
uint8 chr = _tryParseChr(buffer[i]);
if (chr > 9) return (false, 0);
result *= 10;
result += chr;
}
return result;
return (true, result);
}

/**
Expand All @@ -149,19 +183,54 @@ library Strings {
* - the result does not fit in a `int256`.
*/
function toInt(string memory input) internal pure returns (int256) {
return toInt(input, 0, bytes(input).length);
}

/**
* @dev Variant of {toInt-string} that parses a substring of `input` located between position `begin` (included) and
* `end` (excluded).
*/
function toInt(string memory input, uint256 begin, uint256 end) internal pure returns (int256) {
(bool success, int256 value) = tryToInt(input, begin, end);
if (!success) revert StringsInvalidChar();
return value;
}

/**
* @dev Variant of {toInt-string} that returns false if the parsing fails because of an invalid character.
*
* This function will still revert if the result does not fit in a `int256`
*/
function tryToInt(string memory input) internal pure returns (bool success, int256 value) {
return tryToInt(input, 0, bytes(input).length);
}

/**
* @dev Variant of {toInt-string-uint256-uint256} that returns false if the parsing fails because of an invalid
* character.
*
* This function will still revert if the result does not fit in a `int256`
*/
function tryToInt(
string memory input,
uint256 begin,
uint256 end
) internal pure returns (bool success, int256 value) {
bytes memory buffer = bytes(input);

// check presence of a negative sign.
bool isNegative = bytes1(buffer) == 0x2d;
bool isNegative = bytes1(unsafeReadBytesOffset(buffer, begin)) == 0x2d;
int8 factor = isNegative ? int8(-1) : int8(1);
uint256 offset = SafeCast.toUint(isNegative);

int256 result = 0;
uint256 bufferLength = buffer.length;
for (uint256 i = SafeCast.toUint(isNegative); i < bufferLength; ++i) {
result *= 10; // will revert if overflow
result += factor * int8(_parseChr(buffer[i], 10)); // parseChr is at most 9, it fits into an int8
for (uint256 i = begin + offset; i < end; ++i) {
uint8 chr = _tryParseChr(buffer[i]);
if (chr > 9) return (false, 0);
result *= 10;
result += factor * int8(chr);
}
return result;
return (true, result);
}

/**
Expand All @@ -172,38 +241,77 @@ library Strings {
* - the result does not fit in a `uint256`.
*/
function hexToUint(string memory input) internal pure returns (uint256) {
return hexToUint(input, 0, bytes(input).length);
}

/**
* @dev Variant of {hexToUint} that parses a substring of `input` located between position `begin` (included) and
* `end` (excluded).
*/
function hexToUint(string memory input, uint256 begin, uint256 end) internal pure returns (uint256) {
(bool success, uint256 value) = tryHexToUint(input, begin, end);
if (!success) revert StringsInvalidChar();
return value;
}

/**
* @dev Variant of {hexToUint-string} that returns false if the parsing fails because of an invalid character.
*
* This function will still revert if the result does not fit in a `uint256`
*/
function tryHexToUint(string memory input) internal pure returns (bool success, uint256 value) {
return tryHexToUint(input, 0, bytes(input).length);
}

/**
* @dev Variant of {hexToUint-string-uint256-uint256} that returns false if the parsing fails because of an
* invalid character.
*
* This function will still revert if the result does not fit in a `uint256`
*/
function tryHexToUint(
string memory input,
uint256 begin,
uint256 end
) internal pure returns (bool success, uint256 value) {
bytes memory buffer = bytes(input);

// skip 0x prefix if present
uint256 offset = Math.ternary(bytes2(buffer) == 0x3078, 2, 0);
bool hasPrefix = bytes2(unsafeReadBytesOffset(buffer, begin)) == 0x3078;
uint256 offset = SafeCast.toUint(hasPrefix) * 2;

uint256 result = 0;
uint256 bufferLength = buffer.length;
for (uint256 i = offset; i < bufferLength; ++i) {
result *= 16; // will revert if overflow
result += _parseChr(buffer[i], 16);
for (uint256 i = begin + offset; i < end; ++i) {
uint8 chr = _tryParseChr(buffer[i]);
if (chr > 15) return (false, 0);
result *= 16;
result += chr;
}
return (true, result);
}

// TODO: documentation.
function unsafeReadBytesOffset(bytes memory buffer, uint256 offset) internal pure returns (bytes32 value) {
assembly ("memory-safe") {
value := mload(add(buffer, add(0x20, offset)))
}
return result;
}

function _parseChr(bytes1 chr, uint8 base) private pure returns (uint8) {
function _tryParseChr(bytes1 chr) private pure returns (uint8) {
uint8 value = uint8(chr);

// Try to parse `chr`:
// - Case 1: [0-9]
// - Case 2: [a-z]
// - Case 2: [A-Z]
// - Case 2: [a-f]
// - Case 2: [A-F]
// - otherwise not supported
unchecked {
if (value > 47 && value < 58) value -= 48;
else if (value > 96 && value < 123) value -= 87;
else if (value > 64 && value < 91) value -= 55;
else revert StringsInvalidChar(chr, base);
else if (value > 96 && value < 103) value -= 87;
else if (value > 64 && value < 71) value -= 55;
else return type(uint8).max;
}

// check base
if (value >= base) revert StringsInvalidChar(chr, base);

return value;
}
}
Loading

0 comments on commit a7a6e9e

Please sign in to comment.