Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

somthing wrong in bitvec codec #5886

Open
mrtnetwork opened this issue May 20, 2024 · 2 comments
Open

somthing wrong in bitvec codec #5886

mrtnetwork opened this issue May 20, 2024 · 2 comments

Comments

@mrtnetwork
Copy link

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
Copy link
Member

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

@mrtnetwork
Copy link
Author

mrtnetwork commented May 27, 2024

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;
  }


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants