nanocurrency / protocol

Implementation independent content related to the Nano protocol
BSD 2-Clause "Simplified" License
36 stars 10 forks source link

Bug on vote_by_hash kaitai specification #28

Closed marcosmmb closed 2 years ago

marcosmmb commented 2 years ago

Currently, this is the kaitai specification for vote_by_hash:

  vote_by_hash:
    doc: A sequence of hashes, where count is read from header.
    seq:
      - id: hashes
        size: 32
        repeat: until
        repeat-until: _index == _root.header.item_count_int or _io.eof
        if: not _io.eof

which generates the following Python code upon compilation:

    class VoteByHash(KaitaiStruct):
        """A sequence of hashes, where count is read from header."""
        def __init__(self, _io, _parent=None, _root=None):
            self._io = _io
            self._parent = _parent
            self._root = _root if _root else self
            self._read()

        def _read(self):
            if not (self._io.is_eof()):
                self.hashes = []
                i = 0
                while True:
                    _ = self._io.read_bytes(32)
                    self.hashes.append(_)
                    if  ((i == self._root.header.item_count_int) or (self._io.is_eof())) :
                        break
                    i += 1

Notice that the hash list self.hashes will end up with a size of item_count_int + 1, which will "leak" and consider the first 32 bytes of the next byte string as a block hash, while it is usually the start of a new message.

From my tests, it seems that updating the line to

repeat-until: _index == (_root.header.item_count_int - 1) or _io.eof

will fix the issue.

marcosmmb commented 2 years ago

@zhyatt @dsiganos After some tests I've realized that maybe the issue is not in the Nano protocol definition file, but actually with Kaitai's code generation.

Note that for Python, the i variable starts with value 0, while in Go it starts with value 1. This means that in Go the code actually works as expected, but in Python the result will be different from what's expected.

Generated Python code:

def _read(self):
  if not (self._io.is_eof()):
      self.hashes = []
      i = 0
      while True:
          _ = self._io.read_bytes(32)
          self.hashes.append(_)
          if  ((i == self._root.header.item_count_int) or (self._io.is_eof())) :
              break
          i += 1

Generated Golang code:

func (this *Nano_VoteByHash) Read(io *kaitai.Stream, parent *Nano_MsgConfirmAck, root *Nano) (err error) {
    this._io = io
    this._parent = parent
    this._root = root

    tmp114, err := this._io.EOF()
    if err != nil {
        return err
    }
    if !(tmp114) {
        for i := 1; ; i++ {
            tmp115, err := this._io.ReadBytes(int(32))
            if err != nil {
                return err
            }
            tmp115 = tmp115
            _it := tmp115
            this.Hashes = append(this.Hashes, _it)
            tmp116, err := this._root.Header.ItemCountInt()
            if err != nil {
                return err
            }
            tmp117, err := this._io.EOF()
            if err != nil {
                return err
            }
            if (i == tmp116) || (tmp117) {
                break
            }
        }
    }
    return err
}
cryptocode commented 2 years ago

@marcosmmb if you've found a Kaitai codgen bug, and if you have time, please consider adding a testcase issue at https://github.com/kaitai-io/kaitai_struct so they can fix it.