Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[audit] #1: Add gas optimization to Sha3 #57

Merged
merged 4 commits into from
Jul 9, 2024
Merged

[audit] #1: Add gas optimization to Sha3 #57

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c24df9c
Add gas optimization to
stevieraykatz Jun 25, 2024
049e0ef
Add for loop optimization
stevieraykatz Jun 25, 2024
8c75097
Add header natspec to Sha3
stevieraykatz Jun 26, 2024
9915bad
Further natspec improvements
stevieraykatz Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions src/lib/Sha3.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,31 @@
//SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;

/// @title Sha3 Hex Encoding
///
/// @notice This method is copied from the ENS `ReverseRegistrar` contract. It's been moved to its own
/// lib for readability and testing purposes.
/// See: https://github.com/ensdomains/ens-contracts/blob/545a0104d0fbdd10865743e25729a921a76fd950/contracts/reverseRegistrar/ReverseRegistrar.sol#L164-L181
///
/// @author ENS (https://github.com/ensdomains/ens-contracts)
library Sha3 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a lib we inherited?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally; ENS had this method included in the body of the reverse registrar and i moved it out to this lib.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, then just noting that if we wanted to try to avoid writing assembly, we could have maybe used this from Solady https://github.com/Vectorized/solady/blob/main/src/utils/LibString.sol#L963

Also isn't keccak256 a variant of sha3 but not exactly the same?

Copy link
Collaborator Author

@stevieraykatz stevieraykatz Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that keccak256 and sha3 aren't the same though perhaps their results are equivalent in this context... TBH I didn't dig too much into the functionality here since it's leveraged in the wild by ENS exactly like this. I'm a little reticent to introduce our own methodology leveraging the solady toLowerCase function.

Perhaps I should add a natspec to the top of this file referencing the ENS implementation so that it's clear we're just inheriting this logic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added natspec header

// Hex encoding of "0123456789abcdef"
bytes32 constant ALPHABET = 0x3031323334353637383961626364656600000000000000000000000000000000;
/// @notice Hex encoding of "0123456789abcdef"
bytes32 constant ALPHABET = 0x30_31_32_33_34_35_36_37_38_39_61_62_63_64_65_66_00000000000000000000000000000000;

/**
* @dev An optimised function to compute the sha3 of the lower-case
* hexadecimal representation of an Ethereum address.
* @param addr The address to hash
* @return ret The SHA3 hash of the lower-case hexadecimal encoding of the
* input address.
*/
/// @notice Calculates the hash of a lower-case Ethereum address
///
/// @param addr The address to hash
///
/// @return ret The SHA3 hash of the lower-case hexadecimal encoding of the input address.
function hexAddress(address addr) internal pure returns (bytes32 ret) {
assembly {
for { let i := 40 } gt(i, 0) {} {
for { let i := 40 } i {} {
i := sub(i, 1)
mstore8(i, byte(and(addr, 0xf), ALPHABET))
addr := div(addr, 0x10)
addr := shr(4, addr)
i := sub(i, 1)
mstore8(i, byte(and(addr, 0xf), ALPHABET))
addr := div(addr, 0x10)
addr := shr(4, addr)
}

ret := keccak256(0, 40)
Expand Down