vbuterin / pybitcointools

SImple, common-sense Bitcoin-themed Python ECC library
1.28k stars 857 forks source link

changebase: to base58 (from base 256 or 16) fails test cases with leading null bytes #105

Closed wizardofozzie closed 8 years ago

wizardofozzie commented 8 years ago

Base58 Test vectors failure for 2 test cases:

  1. ["00eb15231dfceb60925886b67d065299925915aeb172c06647", "1NS17iag9jJgTHD1VXjvLCEnZuQ3rJDE9L"]
  2. ["00000000000000000000", "1111111111"]

Test Vector 5:

changebase("00eb15231dfceb60925886b67d065299925915aeb172c06647", 16, 58) returns "NS17iag9jJgTHD1VXjvLCEnZuQ3rJDE9L" (leading 1 missing) The correct result is returned by specifying minlen=34, ie changebase("00eb15231dfceb60925886b67d065299925915aeb172c06647", 16, 58, minlen=34)

Test Vector 12:

changebase("00000000000000000000", 16, 58) returns "" (instead of "1111111111"). Again, specifiying minlen=10 fixes this, changebase("00000000000000000000", 16, 58, 10).

wizardofozzie commented 8 years ago

This code works for the json tests, however b58check functions are now failing.

def changebase(string, frm, to, minlen=0):
    if frm == to:
        return lpad(string, get_code_string(frm)[0], minlen)
    elif frm in (16, 256) and to == 58:
        if frm == 16:
            nullbytes = re.match('^(00)*', string).group(0)
            nblen = len(nullbytes)/2
        else:
            nullbytes = re.match('^(\x00)*', string).group(0)
            nblen = len(nullbytes)
        return lpad('', '1', nblen) + encode(decode(string, frm), to)
    elif frm == 58 and to in (16, 256):
        nullbytes = re.match('^1*', string).group(0)
        if to == 16:
            padding = lpad('', '00', len(nullbytes))
        else:
            padding = lpad('', '\x00', len(nullbytes))
        return padding + encode(decode(string, frm), to)
    return encode(decode(string, frm), to, minlen)
fcracker79 commented 8 years ago

This is necessary just for Base58 check and not for normal Base58 encoding. Please refer to this, point 5 of "Creating a Base58Check string".

wizardofozzie commented 8 years ago

@fcracker79 Ahhh, yes, you're right.