somthing wrong in bitvec codec

[mrtnetwork]

In the BitVec codec, I see the following comment:

 * @name BitVec
 * @description
 * A BitVec that represents an array of bits. The bits are however stored encoded. The difference between this
 * and a normal Bytes would be that the length prefix indicates the number of bits encoded, not the bytes
 */

However, in the decoding section, we have:

function decodeBitVecU8a (value?: Uint8Array): [number, Uint8Array] {
  if (!value?.length) {
    return [0, new Uint8Array()];
  }

  // handle all other Uint8Array inputs, these do have a length prefix which is the number of bits encoded
  const [offset, length] = compactFromU8aLim(value);
  const total = offset + Math.ceil(length / 8);

  if (total > value.length) {
    throw new Error(`BitVec: required length less than remainder, expected at least ${total}, found ${value.length}`);
  }

  return [length, value.subarray(offset, total)];
}

Can you please double-check the encoding and decoding logic for BitVec? Thanks!

[TarikGul]

Do you mind explaining what exactly is wrong with this implementation?

[mrtnetwork]

Do you mind explaining what exactly is wrong with this implementation?

The current implementation of the BitVec class contains a discrepancy in how the length is encoded. According to the class comment, the length prefix should indicate the number of bits encoded, not the number of bytes.

/**
 * A BitVec that represents an array of bits. The bits are however stored encoded. The difference between this
 * and a normal Bytes would be that the length prefix indicates the number of bits encoded, not the bytes.
 */

The method responsible for calculating the encoded length currently returns:

return this.length + compactToU8a(this.#decodedLength).length;

The length should be encoded correctly as the number of bits. Therefore, the correct implementation should be:

return this.length + compactToU8a(this.#decodedLength * 8).length;

decode section

function decodeBitVecU8a (value?: Uint8Array): [number, Uint8Array] {
  if (!value?.length) {
    return [0, new Uint8Array()];
  }

  // handle all other Uint8Array inputs, these do have a length prefix which is the number of bits encoded
  const [offset, length] = compactFromU8aLim(value);
  const total = offset + Math.ceil(length / 8);

  if (total > value.length) {
    throw new Error(`BitVec: required length less than remainder, expected at least ${total}, found ${value.length}`);
  }

  return [length, value.subarray(offset, total)];
}

/** @internal */
function decodeBitVec (value?: AnyU8a): [number, Uint8Array] {
  if (Array.isArray(value) || isString(value)) {
    const u8a = u8aToU8a(value);

    return [u8a.length / 8, u8a];
  }

  return decodeBitVecU8a(value);
}

constructor (registry: Registry, value?: AnyU8a, isMsb = false) {
    const [decodedLength, u8a] = decodeBitVec(value);

    super(registry, u8a);

    this.#decodedLength = decodedLength;
    this.#isMsb = isMsb;
  }

[rajk93]

@mrtnetwork based on my understanding and research the current implementation is correct:

public override get encodedLength (): number {
  return this.length + compactToU8a(this.#decodedLength).length;
}

Why it's correct:

The SCALE BitVec length prefix must be the number of bits.

• this.#decodedLength already stores this exact number of bits.
• compactToU8a(this.#decodedLength) correctly encodes this bit count for the prefix.
• The formula then sums the byte length of this prefix with the byte length of the data (this.length).

Using this.#decodedLength * 8 for the value passed to compactToU8a() would make the prefix represent "eight times the number of bits," which is not compliant with the BitVec SCALE specification.

Example:

A BitVec with 10 bits:

• this.#decodedLength = 10.
• Data part (this.length) = Math.ceil(10/8) = 2 bytes.
• compactToU8a(10) produces new Uint8Array([40]) (since 10 << 2 = 40)) , hence length of it will be = 1 byte.
• encodedLength = 2 + 1 = 3 bytes.

If the prefix used compactToU8a(10 * 8 = 80), it would incorrectly represent "80 bits".

Hope this concisely clarifies it.

[valentinfernandez1]

Amazing analysis @rajk93

[mrtnetwork]

@rajk93

this.#decodedLength already stores this exact number of bits.

But in this code, this.#decodedLength stores the byte length

[rajk93]

Yes, @mrtnetwork —

It looks like there's an issue in the decodeBitVec function, which is used in the constructor to set #decodedLength.

Currently, the length calculation appears to be:

return [u8a.length / 8, u8a];

However, this should be:

return [u8a.length * 8, u8a];

to correctly represent the length in bits, not bytes.