Add BytesParsing lib

[nik-suri]

No description provided.

[kcsongor]

can we check that ret == 0 || ret == 1 here? or equivalently ret == ret & 1 which might be marginally cheaper

[djb15]

Wouldn't ret == ret & 1 return true for all odd numbers though, so it only half does what you're looking for. Agree though that it would be nice to enforce 0/1, good call

[kcsongor]

hmm, I think it does what we want 🤔 are you thinking of ret | 1?

[djb15]

Unless I'm being stupid I think that's right since it's a bitwise AND. If a is always 1, this little Remix test return true for b = 1,3,5,etc

pragma solidity 0.8.0;

contract Test {
    function testAnd(uint8 a, uint8 b) public pure returns (bool) {
        uint8 x = a & b;
        return (x != 0);
    }
}

[djb15]

Ohhh I see what you're saying now. You're suggesting we return ret == ret & 1 and forget about the !=0 part? That would work although then anything >1 returns false, but wouldn't we rather it reverted instead?

[nonergodic]

assert is the wrong choice since it should indicate situations that should never happen (implementation errors) and consumes all remaining gas. So it should be either a custom error or require.

[nonergodic]

I propose the following implementation:

function asBoolUnchecked(
  bytes memory encoded,
  uint offset
) internal pure returns (bool, uint) {
  (uint8 val, uint nextOffset) = asUint8(encoded, offset);
  if (val & 0xfe != 0)
    revert InvalidBoolVal(val);
   
  bool ret;
  assembly ("memory-safe") {
    ret := val
  }
  return (ret, nextOffset);
}

this should translates to:

• 1 PUSH for the 0xfe constant
• 1 AND for &
• 2 ISZERO for != comparison
• 1 JUMPI

and the assembly section skips the 2 ISZERO opcodes that the solidity compiler would otherwise throw in there to cleanly convert a uint8 into a bool

[nonergodic]

exactly, though thinking about it more, assert(ret == 0 || ret == 1) is a lot more readable and costs marginally more gas 😅

I think in this part of the code we should strongly prioritize gas efficiency over readability. solc will turn the || here into another conditional jump because of short-circuiting which makes the entire thing a lot less gas and code efficient.

[nonergodic]

fixing my sloppy oversight of using asUint8 instead of asUint8Unchecked here

[nonergodic]

And as it turns out the inline assembly is not safe because solc is actually no cleaning up the upper 31 bytes of ret (as I expected it would), so there's no way to avoid the cost of the two ISZERO opcodes after all.

Also fixed in the above PR.

My bad 😿

[nonergodic]

lgtm!