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

fix: registering PublicDataWitness in JsonRpcServer #6243

Merged

Conversation

benesjan
Copy link
Contributor

@benesjan benesjan commented May 7, 2024

PublicDataWitness was not correctly registered on the JsonRpcServer and it made getPublicDataTreeWitness endpoint on aztec node unusable when connecting to it via RPC. This PR fixes it + fixes some bad naming and moves witness out of interfaces because it's not an interface.

Stumbled upon this when working on the keys stuff where the endpoint is now used way more extensively due to the key registry inclusion proofs.

Copy link
Contributor Author

benesjan commented May 7, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @benesjan and the rest of your teammates on Graphite Graphite

@benesjan benesjan force-pushed the 05-07-fix_registering_publicdatawitness_in_jsonrpcserver branch from 091aa29 to 3fda5de Compare May 7, 2024 14:58
@benesjan benesjan force-pushed the 05-07-fix_registering_publicdatawitness_in_jsonrpcserver branch from 3fda5de to dac515d Compare May 7, 2024 15:26
return new PublicDataWitness(
toBigIntBE(reader.readBytes(32)),
reader.readObject(PublicDataTreeLeafPreimage),
SiblingPath.fromBuffer(reader.readBytes(4 + 32 * PUBLIC_DATA_TREE_HEIGHT)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the fixed num bytes here ^ is shit but SiblingPath serialization seems to be incompatible with the methods on reader and I've added a test which serializes and deserializes the witness so if there is some serialization change it should catch it.

@benesjan benesjan marked this pull request as ready for review May 7, 2024 15:59
Copy link
Contributor

@sklppy88 sklppy88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@benesjan benesjan enabled auto-merge (squash) May 7, 2024 16:15
@benesjan benesjan merged commit e8c4455 into master May 7, 2024
65 of 88 checks passed
@benesjan benesjan deleted the 05-07-fix_registering_publicdatawitness_in_jsonrpcserver branch May 7, 2024 16:15
TomAFrench added a commit that referenced this pull request May 8, 2024
* master: (25 commits)
  fix: Enable client proof tests (#6249)
  chore: update cspell for abi demonomorphizer (#6258)
  feat(aztec-nr): add 'with_gas()' function to avm call interface (#6256)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  fix: Pw/update merge check (#6201)
  chore(master): Release 0.37.1 (#6148)
  fix: Cl/split out e2e tests (#6242)
  feat: Typings generator with generics (#6235)
  chore(ci): fix restarts with fresh spot, acir test fixes, non-mandatory benches (#6226)
  chore: misc AVM migration prep changes (#6253)
  feat!: AES blackbox (#6016)
  chore(docs): Fix some typos in specs of private kernel initial (#6224)
  chore(aztec-macros): avm function return types are auto tagged as `pub` (#6250)
  chore(aztec-nr): create a 'with_selector' version of `emit_unencrypted_log` in avm context (#6248)
  fix: registering PublicDataWitness in JsonRpcServer (#6243)
  feat: Sync from noir (#6234)
  feat(avm-simulator): consider previous pending nullifiers across enqueued calls (#6188)
  ...
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