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

logic bug in verifyProof #7

Open
Hachikoi-the-creator opened this issue May 22, 2023 · 2 comments
Open

logic bug in verifyProof #7

Hachikoi-the-creator opened this issue May 22, 2023 · 2 comments

Comments

@Hachikoi-the-creator
Copy link

Inside the for the concat func expects uint8Array but proof[i].data is type string
Error visible for me since I'm using TS to try and understand what is happening a little bit better
Fixed code:

  for (let i = 0; i < proof.length; i++) {
    if (proof[i].left) {
      data = concat(utf8ToBytes(proof[i].data), data);
    } else {
      data = concat(data, utf8ToBytes(proof[i].data));
    }
  }
@Hachikoi-the-creator
Copy link
Author

nvm I broke it

@Hachikoi-the-creator
Copy link
Author

Hachikoi-the-creator commented May 23, 2023

Found a what I think could be a logic bug tho.

function verifyProof(proof, leaf, root) {
  proof = proof.map(({ data, left }) => ({
    left,
    data: hexToBytes(data),
  }));
  let data = keccak256(Buffer.from(leaf));

  for (let i = 0; i < proof.length; i++) {
    if (proof[i].left) {
      data = concat(proof[i].data, data);
    } else {
      data = concat(data, proof[i].data);
    }
  }

  return bytesToHex(data) === root;
}

since verify proof will be used in the serve and we cannot transfer uint8Arrays tru http, I modified it so the inputs use strings (hex) and in the concat we do the hextToBytes

import { keccak256 } from "ethereum-cryptography/keccak";
import { bytesToHex, hexToBytes } from "ethereum-cryptography/utils";
import { ProofVer } from "./merkleTree";

const concat = (left: Uint8Array, right: Uint8Array) =>
  keccak256(Buffer.concat([left, right]));

export function verifyProof(proof: ProofVer[], leaf: string, root: string) {
  let data: Uint8Array = keccak256(Buffer.from(leaf));

  for (let i = 0; i < proof.length; i++) {
    if (proof[i].left) {
      data = concat(hexToBytes(proof[i].data), data);
    } else {
      data = concat(data, hexToBytes(proof[i].data));
    }
  }

  return bytesToHex(data) === root;
}

that is the ts version where I was able to spot the error

Adding extra type for context

export type ProofVer = {
  left: boolean;
  data: string;
};

@Hachikoi-the-creator Hachikoi-the-creator changed the title unmatching types in verifyProof logic bug in verifyProof May 23, 2023
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

1 participant