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

Broadcast the 'public key' and 'partial address' as L1 calldata, when deploying an account contract #1778

Closed
iAmMichaelConnor opened this issue Aug 23, 2023 · 8 comments · Fixed by #1801 or #1804
Assignees
Labels
T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature).

Comments

@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented Aug 23, 2023

Currently, recipients have to be manually registered via the aztec rpc server. This approach doesn't scale, because if you call some function of some smart contract, you might need to encrypt a note for some counterparty that you've never interacted with before (the counterparty's address might only be derived at runtime!).

So (at least until we implement a better keys scheme) we need an aztec node to passively 'gobble up' the address, public key and partial address of every deployed account contract, so that the simulator can grab that information whenever a function wishes to perform an encryption.

Note:

  • The act of querying this address data from a public node is leaky, if the user is not running their own public node. Research is underway to explore private retrieval.

Definition of "Done"

  • An address, public key, and partial address are broadcast as calldata to L1
  • All new such address data is gobbled up and stored in a db (I'm not sure by which module of the stack, but you'll know :) )
  • Whenever a noir contract needs to generate an encrypted log for a recipient, it can call an oracle, passing the recipient's address. The simulator can (somehow) query the corresponding public key and partial address. The oracle returns this information to the circuit.
  • All existing example Noir contracts are updated to use this new approach to encrypting notes (rather than registering a recipient).
  • e2e tests are updated to use this approach (rather than registering a recipient).

Possibly blocked by #1139 (although if encryption is currently being correctly performed by typescript (as a hack vs encrypting within Noir), this issue is not blocked, because we can update the typescript code).

@iAmMichaelConnor iAmMichaelConnor added the T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature). label Aug 23, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Aug 23, 2023
@iAmMichaelConnor iAmMichaelConnor mentioned this issue Aug 23, 2023
4 tasks
@benesjan
Copy link
Contributor

I would approach this by renaming our ContractDeploymentEmitter back to UnverifiedDataEmitter and emit NewRecipient event in that. Does that sound good to everyone?

@iAmMichaelConnor
Copy link
Contributor Author

I'm not sure. I suppose ContractDeploymentEmitter is still an accurate description. It's data which is emitted when an (account) contract is deployed. I think some people had confusion with the word "unverified", because the hash of the data is checked on L1 (so it is 'verified' in a way).

NewReceipient might not quite be the right name. It's simply "new address data", and all address data should be gobbled up into a db. ("Recipient" might not be the correct term. At the point the contract is deployed the data doesn't represent a "recipient", it simply represents an "address").

@iAmMichaelConnor
Copy link
Contributor Author

I just added a 'defn of done'.

@PhilWindle
Copy link
Collaborator

@iAmMichaelConnor Are we ok publishing this data for all contract deployments? I don't really know how we can differentiate between an account contract and a non-account contract.

@iAmMichaelConnor
Copy link
Contributor Author

iAmMichaelConnor commented Aug 24, 2023

Good question. What's the public key for a non-account contract? Is it 0?

Aside: It's tricky. The act of encrypting data for someone only makes sense for EoAs (externally owned accounts, like users, or bots). Encrypting data for a non-account contract address is kind-of pointless, because a non-account smart contract isn't capable of decrypting something (it's incapable of owning a secret that only it knows).
So in Aztec, it seems there's a fundamental difference between an account contract and a non-account contract. Maybe that fundamental difference is just "is the public key equal to 0?". Maybe if the public key of a non-account contract is always 0, the noir code to generate an encrypted log can conditionally not output a log.

@benesjan
Copy link
Contributor

NewReceipient might not quite be the right name. It's simply "new address data", and all address data should be gobbled up into a db. ("Recipient" might not be the correct term. At the point the contract is deployed the data doesn't represent a "recipient", it simply represents an "address").

@iAmMichaelConnor But the only reason of publishing the new address data (we've created a CompleteAddress type exactly for that) is that the given smart contract can receive a note, no?

But anyway, I am ok with calling the event something like NewCompleteAddress or just CompleteAddress.

@iAmMichaelConnor
Copy link
Contributor Author

@iAmMichaelConnor But the only reason of publishing the new address data (we've created a CompleteAddress type exactly for that) is that the given smart contract can receive a note, no?

But anyway, I am ok with calling the event something like NewCompleteAddress or just CompleteAddress.

Fair point! I'll let you decide :)

@PhilWindle
Copy link
Collaborator

@iAmMichaelConnor Our API is pretty unrestrictive as it stands. Ultimately, the public key used in the deployment of any contract (account or otherwise) is whatever the client of aztec.js decides to give it.

@benesjan benesjan self-assigned this Aug 24, 2023
@iAmMichaelConnor iAmMichaelConnor added this to the 📢 Initial Public Sandbox Release milestone Aug 25, 2023
@spalladino spalladino moved this from Todo to In Review in A3 Aug 29, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in A3 Aug 29, 2023
PhilWindle pushed a commit that referenced this issue Aug 30, 2023
🤖 I have created a new Aztec Packages release
---


##
[0.1.0-alpha48](v0.1.0-alpha47...v0.1.0-alpha48)
(2023-08-30)


### Features

* Add ARM build for Mac + cleanup artifacts
([#1837](#1837))
([270a4ae](270a4ae))
* broadcasting 'public key' and 'partial address' as L1 calldata
([#1801](#1801))
([78d6444](78d6444)),
closes
[#1778](#1778)
* Check sandbox version matches CLI's
([#1849](#1849))
([7279730](7279730))
* **docs:** adding some nitpick suggestions before sandbox release
([#1859](#1859))
([c1144f7](c1144f7))
* More reliable getTxReceipt api.
([#1793](#1793))
([ad16b22](ad16b22))
* **noir:** use `#[aztec(private)]` and `#[aztec(public)` attributes
([#1735](#1735))
([89756fa](89756fa))
* Recursive fn calls to spend more notes.
([#1779](#1779))
([94053e4](94053e4))
* Simulate enqueued public functions and locate failing constraints on
them
([#1853](#1853))
([a065fd5](a065fd5))
* Update safe_math and move to libraries
([#1803](#1803))
([b10656d](b10656d))
* Write debug-level log to local file in Sandbox
([#1846](#1846))
([0317e93](0317e93)),
closes
[#1605](#1605)


### Bug Fixes

* Conditionally compile base64 command for bb binary
([#1851](#1851))
([be97185](be97185))
* default color to light mode
([#1847](#1847))
([4fc8d39](4fc8d39))
* Disallow unregistered classes in JSON RPC interface and match by name
([#1820](#1820))
([35b8170](35b8170))
* Set side effect counter on contract reads
([#1870](#1870))
([1d8881e](1d8881e)),
closes
[#1588](#1588)
* Truncate SRS size to the amount of points that we have downloaded
([#1862](#1862))
([0a7058c](0a7058c))


### Miscellaneous

* add browser test to canary flow
([#1808](#1808))
([7f4fa43](7f4fa43))
* **ci:** fix output name in release please workflow
([#1858](#1858))
([857821f](857821f))
* CLI tests
([#1786](#1786))
([2987065](2987065)),
closes
[#1450](#1450)
* compile minimal WASM binary needed for blackbox functions
([#1824](#1824))
([76a30b8](76a30b8))
* fixed linter errors for `ecc`, `numeric` and `common` modules
([#1714](#1714))
([026273b](026273b))
* Refactor Cli interface to be more unix-like
([#1833](#1833))
([28d722e](28d722e))
* sync bb master
([#1842](#1842))
([2c1ff72](2c1ff72))
* sync bb master
([#1852](#1852))
([f979878](f979878))
* sync bb master
([#1866](#1866))
([e681a49](e681a49))
* typescript script names should be consistent
([#1843](#1843))
([eff8fe7](eff8fe7))
* use 2^19 as `MAX_CIRCUIT_SIZE` for NodeJS CLI
([#1834](#1834))
([c573282](c573282))


### Documentation

* Account contract tutorial
([#1772](#1772))
([0faefba](0faefba))
* Wallet dev docs
([#1746](#1746))
([9b4281d](9b4281d)),
closes
[#1744](#1744)

---
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
T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature).
Projects
Archived in project
3 participants