Preliminary report on the bitcoind hex conversion mess

Filed under: Bitcoin, Politikos, Software — Jacob Welsh @ 16:46

As I previously noted about my getrawtransaction patch,

It does accept invalid hex strings, perhaps a flaw in that SetHex method.

Indeed, SetHex in the base_uint class is completely permissive: it accepts leading whitespace, optional "0x" or "0X", then however many hex digits it finds, up to the length of the template specialization (160 or 256 bits), zero-filling if too short and ignoring excess if too long. It also includes some pointer arithmetic I believe has technically undefined behavior: comparing after decrementing past the start of the object. And it's one source of the obnoxious byte order reversal in bitcoind hash I/O, with GetHex being its counterpart.

The users of the function are the RPC commands listsinceblock and gettransaction. There's another SetHex function in CBigNum, similar but taking arbitrary length, entirely unused.

On the other hand, ParseHex, the subject of mod6_phexdigit_fix.vpatch, is oriented toward "dumps" in that it allows space between digit pairs and doesn't reverse byte order. It's used in some tests, in the hard-coded genesis block output in main.cpp, in the mining RPC commands getwork and getmemorypool, and in sendrawtransaction as seen in polarbeard_add_sendrawtransaction.vpatch.

My conclusion is that getrawtransaction is doing the right thing here, in that it shouldn't be made a special case, but the hex parsing should be cleaned up generally. If it were just me, I'd rip it all out and use a single, strict, non-reversing hex decoder. But there's no telling how much outside code or data has built up around the old rules. Parties interested in the matter, in the sense of having a meaningful amount of coin on the line, are encouraged to write in.(i)

  1. Not that I expect this to do much on its own, as politics involves involves actively going out to find people where they are and talk to them. [^]

1 Comment »

  1. [...] I've discussed previously and don't see as a problem with this [...]

    Pingback by New and improved raw transaction support for bitcoind « Fixpoint — 2020-04-22 @ 05:26

RSS feed for comments on this post. TrackBack URL

Leave a comment

Powered by MP-WP. Copyright Jacob Welsh.