[audit] #1: Add gas optimization to Sha3

[stevieraykatz]

From Spearbit:

Description
hexAddress can be optimised by replacing:

div(addr, 0x10)

with

shr(4, addr)

and

gt(i, 0)

with

I

Also underscores can be added to ALPHABET to make it more readable.

[!Note] The current implementation copies the same implementation from ens-contracts

Recommendation
Apply the following patch to save around 40 gas:

diff --git a/src/lib/Sha3.sol b/src/lib/Sha3.sol
index 12cec00..5877a36 100644
--- a/src/lib/Sha3.sol
+++ b/src/lib/Sha3.sol
@@ -3,7 +3,7 @@ pragma solidity ^0.8.23;
 
 library Sha3 {
     // Hex encoding of "0123456789abcdef"
-    bytes32 constant ALPHABET = 0x3031323334353637383961626364656600000000000000000000000000000000;
+    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
@@ -14,13 +14,13 @@ library Sha3 {
      */
     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)

[wilsoncusack]

is this a lib we inherited?

[stevieraykatz]

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

[wilsoncusack]

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?

[stevieraykatz]

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?

[stevieraykatz]

Added natspec header