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

feat(docs): Testing guide and getPrivateStorage method #1992

Merged
merged 14 commits into from
Sep 6, 2023

Conversation

spalladino
Copy link
Collaborator

@spalladino spalladino commented Sep 4, 2023

Adds a guide on testing dapps. While writing the guide, I had to add a getPrivateStorageAt method; but while reviewing other issues I noted that this method already existed and was deleted on purpose, due to lack of authentication.

I'd like to reincorporate the method, since it's useful for testing, and we already lack authentication for accessing other private information (like just calling view on a function that loads private notes) so we are not opening a new avenue for attack.

Depends on #1991

* @param storageSlot - The Fr representing the storage slot to be fetched.
* @returns A set of note preimages for the owner in that contract and slot.
*/
getPrivateStorageAt(owner: AztecAddress, contract: AztecAddress, storageSlot: Fr): Promise<NotePreimage[]>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It all looks fine, just boils down to your thoughts @iAmMichaelConnor on re-introducing this method on the RPC server. We used to have it, then we removed it as you expressed concerns about making private state freely accessible. There is a feeling that being able to read private state directly aids in the dev/testing process.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup, when reading old conversations I saw that that removal came out of a suggestion of mine. But I changed my mind after going through this testing guide.

@spalladino spalladino marked this pull request as ready for review September 5, 2023 13:57
@spalladino spalladino enabled auto-merge (squash) September 6, 2023 12:29

### Running Sandbox in the nodejs process

Instead of connecting to a local running Sandbox instance, you can also start your own Sandbox within the nodejs process running your tests, for an easier setup. To do this, import the `@aztec/aztec-sandbox` package in your project, and run `createSandbox` during setup. Note that this will still require you to run a local Ethereum node like Anvil locally.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Instead of connecting to a local running Sandbox instance, you can also start your own Sandbox within the nodejs process running your tests, for an easier setup. To do this, import the `@aztec/aztec-sandbox` package in your project, and run `createSandbox` during setup. Note that this will still require you to run a local Ethereum node like Anvil locally.
Instead of connecting to a local running Sandbox instance, you can also start your own Sandbox within the nodejs process running your tests, for an easier setup. To do this, import the `@aztec/aztec-sandbox` package in your project, and run `createSandbox` during setup. Note that this will still require you to run an Ethereum node like Anvil locally.

@spalladino spalladino merged commit 5a8c571 into master Sep 6, 2023
@spalladino spalladino deleted the palla/testing-guide branch September 6, 2023 13:07
spalladino added a commit that referenced this pull request Sep 6, 2023
Also fixes duplicated "local" usage spotted by @PhilWindle
[here](#1992 (comment)).
AztecBot pushed a commit to AztecProtocol/docs that referenced this pull request Sep 6, 2023
PhilWindle pushed a commit that referenced this pull request Sep 6, 2023
🤖 I have created a new Aztec Packages release
---


##
[0.1.0-alpha62](v0.1.0-alpha61...v0.1.0-alpha62)
(2023-09-06)


### Features

* **circuits:** hints nullifier transient commitments
([#2056](#2056))
([725b550](725b550))
* **docs:** Testing guide and getPrivateStorage method
([#1992](#1992))
([5a8c571](5a8c571))


### Bug Fixes

* **build:** Use semver version in docker version tag
([#2065](#2065))
([b3db0d0](b3db0d0))


### Documentation

* Link to local ethereum nodes in testing guide
([#2061](#2061))
([e29148b](e29148b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants