LayerTwo-Labs / bip300301_enforcer

CUSF software enforcing BIP300 and BIP301 rules.
1 stars 4 forks source link

Implement Validator Service Server #20

Closed Ash-L2L closed 3 weeks ago

torkelrogstad commented 3 weeks ago

I'd very strongly suggest that we change the proto definitions where we take in bytes, and instead to string with hex-encodings. It makes it a lot easier to work bytes data from other places. An example:

  1. I'm trying to test the GetBlockHeaderInfo endpoint
  2. I go mempool.drivechain.live and grab the most recent block hash: 0000023bc2004ed6b1cf850e1ad9e0fd880432b4628762b40c13571dfa0c7ee1
  3. I paste this into the request body with buf curl: buf curl --protocol grpc --http2-prior-knowledge -d '{"block_hash":"0000023bc2004ed6b1cf850e1ad9e0fd880432b4628762b40c13571dfa0c7ee1"}' http://localhost:50051/cusf.validator.v1.ValidatorService/GetBlockHeaderInfo
  4. I get an error, because I'm supposed to base64-encode the bytes...

Edit:

This also ties into the endianness issue. Encoding 0000023bc2004ed6b1cf850e1ad9e0fd880432b4628762b40c13571dfa0c7ee1 into base64 gives me ACO8IATtaxz4UOGtng/YgEMrRih2K0DBNXHfoMfuE=, which again gives me the error message Missing header info for block hashe17e0cfa1d57130cb4628762b4320488fde0d91a0e85cfb1d64e00c23b020000`...

Ash-L2L commented 3 weeks ago

I'd very strongly suggest that we change the proto definitions where we take in bytes, and instead to string with hex-encodings.

I have mixed opinions on this. Hex encoding is certainly more readable. However it is likely to cause confusion,, since as you noted, Bitcoin displays block hashes with the bytes reversed. The protobuf style guide also recommends against hex-encoding https://protobuf.dev/programming-guides/api/#dont-encode-data-in-a-string

If we must encode in hex strings, I would suggest using a wrapper that clearly indicates the encoding. IE.

message ConsensusHex {
  google.protobuf.StringValue hex = 1;
}
torkelrogstad commented 3 weeks ago

On a general basis, I agree with the Protobuf style guide wrt to not hex-encoding bytes. Bitcoin-related hashes is more or less the only exception I have to this rule! I've been bitten by the irritating byte order in bitcoin hashes way too many times over multiple years and projects, and just dealing with hex-encoded strings has by far been the approach that has given me the least amount of head aches. IMO it's safe to assume that if there's a hex string, it's encoded in the way it would be displayed to a human. I.e. in a wallet, or on a block explorer