eth.rb Update encoder.rb to limit hex false positives

Looking at Util.prefixed? arg to determine whether to treat something as hex makes it impossible to encode anything that starts with the bytes [48,120], which is not that infrequent.

Also, I don't think we get any benefit from this part of the conditional anyway since if the string isn't both hex chars and 2x the input size things will fail anyway.

Longer term it might be beneficial to force the user to specify the encoding.

Thanks!

Jul 15 '24 06:07 RogerPodacter

Longer term it might be beneficial to force the user to specify the encoding.

Or we will implement actual types in Ruby 🤡

Will run the tests once I'm back in the office.

Aug 28 '24 12:08 q9f

Thanks!! And would types really be so crazy here? The hex v. bytes ambiguity is just so brutal for our use-case!

Should we force everyone to use a special class? Definitely annoying but maybe worth it to have a decent night's sleep?

Here's what the AI spit out. Thoughts?

class ByteString
  attr_reader :bytes

  # Factory methods
  def self.from_hex(hex_string)
    # Remove leading '0x' if present
    hex_string = hex_string[2..] if hex_string.start_with?('0x')

    # Ensure it's a valid hex string and has an even number of characters
    raise ArgumentError, "Invalid hex string" unless hex_string.match?(/\A[\h]*\z/)
    raise ArgumentError, "Hex string length must be even" unless hex_string.length.even?

    new([hex_string].pack('H*'))
  end

  def self.from_bin(binary_string)
    raise ArgumentError, "Binary string expected with ASCII-8BIT encoding" unless binary_string.is_a?(String) && binary_string.encoding == Encoding::ASCII_8BIT
    new(binary_string)
  end

  # Convert back to hex
  def to_hex
    @bytes.unpack1('H*')
  end

  # Convert to binary
  def to_bin
    @bytes
  end

  # Prevent calling to_s to avoid ambiguity
  def to_s
    raise NoMethodError, "Ambiguous conversion: Use `to_bin` for bytes or `to_hex` for hex representation"
  end

  # For debugging or string representation
  def inspect
    "#<ByteString hex=#{to_hex} length=#{@bytes.bytesize}>"
  end

  # Equality comparison
  def ==(other)
    other.is_a?(ByteString) && @bytes == other.bytes
  end

  # More strict equality comparison (used by Hash)
  def eql?(other)
    self == other
  end

  # Hash method to allow use in sets or as hash keys
  def hash
    @bytes.hash
  end

  private_class_method :new

  # Initialize with binary string (bytes)
  def initialize(binary_string)
    raise ArgumentError, "Binary string must have ASCII-8BIT encoding" unless binary_string.encoding == Encoding::ASCII_8BIT
    @bytes = binary_string.dup.freeze
  end
end

Sep 04 '24 20:09 RogerPodacter

Unfortuately, this breaks 6 tests:

Failures:

  1) Eth::Abi.encode .decode can handle hex-strings for bytes types
     Failure/Error: raise ValueOutOfBounds, arg unless arg.size <= type.sub_type.to_i
     
     Eth::Abi::ValueOutOfBounds:
       0x80ac58cd
     # ./lib/eth/abi/encoder.rb:179:in `bytes'
     # ./lib/eth/abi/encoder.rb:110:in `primitive_type'
     # ./lib/eth/abi/encoder.rb:80:in `type'
     # ./lib/eth/abi.rb:59:in `block in encode'
     # ./lib/eth/abi.rb:54:in `each'
     # ./lib/eth/abi.rb:54:in `each_with_index'
     # ./lib/eth/abi.rb:54:in `encode'
     # ./spec/eth/abi_spec.rb:305:in `block (3 levels) in <top (required)>'

  2) Eth::Client ens can resolve an ens record
     Failure/Error: raise ValueOutOfBounds, arg unless arg.size <= type.sub_type.to_i
     
     Eth::Abi::ValueOutOfBounds:
       0xfab68bf82e5750a73a16c3c157598b4be30ed6b7e048f8e29e11572119713eaa
     # ./lib/eth/abi/encoder.rb:179:in `bytes'
     # ./lib/eth/abi/encoder.rb:110:in `primitive_type'
     # ./lib/eth/abi/encoder.rb:80:in `type'
     # ./lib/eth/abi.rb:59:in `block in encode'
     # ./lib/eth/abi.rb:54:in `each'
     # ./lib/eth/abi.rb:54:in `each_with_index'
     # ./lib/eth/abi.rb:54:in `encode'
     # ./lib/eth/client.rb:463:in `call_payload'
     # ./lib/eth/client.rb:449:in `call_raw'
     # ./lib/eth/client.rb:263:in `call'
     # ./lib/eth/ens/resolver.rb:60:in `resolver'
     # ./lib/eth/ens/resolver.rb:74:in `resolve'
     # ./lib/eth/client.rb:110:in `resolve_ens'
     # ./spec/eth/client_spec.rb:91:in `block (3 levels) in <top (required)>'

  3) Eth::Ens::Resolver text gets text records for different keys
     Failure/Error: raise ValueOutOfBounds, arg unless arg.size <= type.sub_type.to_i
     
     Eth::Abi::ValueOutOfBounds:
       0xfab68bf82e5750a73a16c3c157598b4be30ed6b7e048f8e29e11572119713eaa
     # ./lib/eth/abi/encoder.rb:179:in `bytes'
     # ./lib/eth/abi/encoder.rb:110:in `primitive_type'
     # ./lib/eth/abi/encoder.rb:80:in `type'
     # ./lib/eth/abi.rb:59:in `block in encode'
     # ./lib/eth/abi.rb:54:in `each'
     # ./lib/eth/abi.rb:54:in `each_with_index'
     # ./lib/eth/abi.rb:54:in `encode'
     # ./lib/eth/client.rb:463:in `call_payload'
     # ./lib/eth/client.rb:449:in `call_raw'
     # ./lib/eth/client.rb:263:in `call'
     # ./lib/eth/ens/resolver.rb:60:in `resolver'
     # ./lib/eth/ens/resolver.rb:89:in `text'
     # ./spec/eth/ens/resolver_spec.rb:40:in `block (3 levels) in <top (required)>'

  4) Eth::Ens::Resolver resolve gets resolver and owner from chain
     Failure/Error: raise ValueOutOfBounds, arg unless arg.size <= type.sub_type.to_i
     
     Eth::Abi::ValueOutOfBounds:
       0xfab68bf82e5750a73a16c3c157598b4be30ed6b7e048f8e29e11572119713eaa
     # ./lib/eth/abi/encoder.rb:179:in `bytes'
     # ./lib/eth/abi/encoder.rb:110:in `primitive_type'
     # ./lib/eth/abi/encoder.rb:80:in `type'
     # ./lib/eth/abi.rb:59:in `block in encode'
     # ./lib/eth/abi.rb:54:in `each'
     # ./lib/eth/abi.rb:54:in `each_with_index'
     # ./lib/eth/abi.rb:54:in `encode'
     # ./lib/eth/client.rb:463:in `call_payload'
     # ./lib/eth/client.rb:449:in `call_raw'
     # ./lib/eth/client.rb:263:in `call'
     # ./lib/eth/ens/resolver.rb:51:in `owner'
     # ./spec/eth/ens/resolver_spec.rb:46:in `block (3 levels) in <top (required)>'

  5) Eth::Ens::Resolver resolve resolves ens names for Ethereum
     Failure/Error: raise ValueOutOfBounds, arg unless arg.size <= type.sub_type.to_i
     
     Eth::Abi::ValueOutOfBounds:
       0xfab68bf82e5750a73a16c3c157598b4be30ed6b7e048f8e29e11572119713eaa
     # ./lib/eth/abi/encoder.rb:179:in `bytes'
     # ./lib/eth/abi/encoder.rb:110:in `primitive_type'
     # ./lib/eth/abi/encoder.rb:80:in `type'
     # ./lib/eth/abi.rb:59:in `block in encode'
     # ./lib/eth/abi.rb:54:in `each'
     # ./lib/eth/abi.rb:54:in `each_with_index'
     # ./lib/eth/abi.rb:54:in `encode'
     # ./lib/eth/client.rb:463:in `call_payload'
     # ./lib/eth/client.rb:449:in `call_raw'
     # ./lib/eth/client.rb:263:in `call'
     # ./lib/eth/ens/resolver.rb:60:in `resolver'
     # ./lib/eth/ens/resolver.rb:74:in `resolve'
     # ./spec/eth/ens/resolver_spec.rb:51:in `block (3 levels) in <top (required)>'

  6) Eth::Ens::Resolver resolve resolves ens names for other coin types
     Failure/Error: raise ValueOutOfBounds, arg unless arg.size <= type.sub_type.to_i
     
     Eth::Abi::ValueOutOfBounds:
       0xfab68bf82e5750a73a16c3c157598b4be30ed6b7e048f8e29e11572119713eaa
     # ./lib/eth/abi/encoder.rb:179:in `bytes'
     # ./lib/eth/abi/encoder.rb:110:in `primitive_type'
     # ./lib/eth/abi/encoder.rb:80:in `type'
     # ./lib/eth/abi.rb:59:in `block in encode'
     # ./lib/eth/abi.rb:54:in `each'
     # ./lib/eth/abi.rb:54:in `each_with_index'
     # ./lib/eth/abi.rb:54:in `encode'
     # ./lib/eth/client.rb:463:in `call_payload'
     # ./lib/eth/client.rb:449:in `call_raw'
     # ./lib/eth/client.rb:263:in `call'
     # ./lib/eth/ens/resolver.rb:60:in `resolver'
     # ./lib/eth/ens/resolver.rb:74:in `resolve'
     # ./spec/eth/ens/resolver_spec.rb:55:in `block (3 levels) in <top (required)>'

Finished in 12.61 seconds (files took 0.6163 seconds to load)
295 examples, 6 failures, 3 pending

Failed examples:

rspec ./spec/eth/abi_spec.rb:304 # Eth::Abi.encode .decode can handle hex-strings for bytes types
rspec ./spec/eth/client_spec.rb:90 # Eth::Client ens can resolve an ens record
rspec ./spec/eth/ens/resolver_spec.rb:39 # Eth::Ens::Resolver text gets text records for different keys
rspec ./spec/eth/ens/resolver_spec.rb:45 # Eth::Ens::Resolver resolve gets resolver and owner from chain
rspec ./spec/eth/ens/resolver_spec.rb:50 # Eth::Ens::Resolver resolve resolves ens names for Ethereum
rspec ./spec/eth/ens/resolver_spec.rb:54 # Eth::Ens::Resolver resolve resolves ens names for other coin types

Could you give me an example that I can implement as test case? What are you trying to do that fails with the current library?

Dec 17 '24 13:12 q9f

I'm willing to consider alternatives but we need to make sure it passes all unit tests, otherwise, we'll break many aspects of the library. So, closing for now unless you have an idea what we could do instead.

Feb 11 '25 08:02 q9f

I'll try to take another look. I really thought this couldn't possibly break anything so my understanding isn't complete!

Feb 11 '25 13:02 RogerPodacter