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)
- 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. [^]
[...] 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