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

test: Ephemeral tree hashPreimage #10232

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

IlyasRidhuan
Copy link
Contributor

The benchmark run actually also doubles as a useful test vector since it updates a "pre-filled" leaf.

This test is also useful in that it shows that we don't need the condition on the hashPreimage . The only time the getKey() === 0n condition is used in the worldstate (which wasnt right in the ephemeral trees anyways) is when we are using the forced null inclusion on public data tree updates - which the avm trees handle separately (and isn't long for this world anyways).

tl;dr
Old version

hashPreimage<T extends TreeLeafPreimage>(preimage: T): Fr {
      // Watch for this edge-case, we are hashing the key=0 leaf to 0.
      // This is for backward compatibility with the world state implementation
      if (preimage.getKey() === 0n) {
        return Fr.zero();
      }
      const input = preimage.toHashInputs().map(x => Fr.fromBuffer(x));
      return poseidon2Hash(input);
    }

New Version

hashPreimage<T extends TreeLeafPreimage>(preimage: T): Fr {
     const input = preimage.toHashInputs().map(x => Fr.fromBuffer(x));
     return poseidon2Hash(input);
   }

@IlyasRidhuan IlyasRidhuan removed the request for review from fcarreiro November 26, 2024 23:49
@dbanks12
Copy link
Collaborator

dbanks12 commented Nov 27, 2024

Approved, but it looks like it's failing 🤔
Edit: saw your msg after I reviewed. Makes sense. I'll make sure the fix works before merging this.

@dbanks12 dbanks12 merged commit f04e540 into db/insert-private-nullifiers-avm Nov 27, 2024
3 checks passed
@dbanks12 dbanks12 deleted the ir/avm_tree_no_zero_hash branch November 27, 2024 14:14
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

Successfully merging this pull request may close these issues.

2 participants