jimmysong / programmingbitcoin

Repository for the book
Other
1.75k stars 656 forks source link

ch06: Script raw_serialize method missing '=' in the condition #222

Open inauman opened 3 years ago

inauman commented 3 years ago
 1    def raw_serialize(self):
 2        result = b''
 3        for cmd in self.cmds:
 4            if type(cmd) == int:  # <1>
 5                result += int_to_little_endian(cmd, 1)
 6            else:
 7                length = len(cmd)
 8                if length < 75:  # <2>
 9                    result += int_to_little_endian(length, 1)
10                elif length > 75 and length < 0x100:  # <3>
11                   result += int_to_little_endian(76, 1)
12                    result += int_to_little_endian(length, 1)
13                elif length >= 0x100 and length <= 520:  # <4>
14                    result += int_to_little_endian(77, 1)
15                    result += int_to_little_endian(length, 2)
16                else:  # <5>
17                    raise ValueError('too long an cmd')
18                result += cmd
19        return result

According to ch06 in the book, "For a number between 1 and 75 inclusive, we know the next n bytes are an element". So, the condition in line 8 in the above code of raw_serialize(self) function should have an equality test as well. I think it should be:

 if length <= 75:

I am still learning bitcoin programming so please let me know if I am not thinking correctly. I would be happy to do a PR to fix this minor issue.

MrRifRaf commented 1 year ago

Hi, I agree, it's a typo. Btw the parse operation correctly has equality sign.

salmonberry7 commented 1 month ago

Yes it should be if length <= 75: since in Bitcoin Wiki the single byte opcodes 1-75 (inclusive) are the single byte push opcodes. As it stands the raw_serialize function will raise a ValueError exception if a data element byte vector of length 75 is encountered in the commands list of a Script object. The 2 elif's set up the push opcodes 76 and 77, which are required for lengths > 75.