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

Add oracle call for retrieving the public key associated to a contract #857

Closed
spalladino opened this issue Jun 15, 2023 · 0 comments · Fixed by #1042
Closed

Add oracle call for retrieving the public key associated to a contract #857

spalladino opened this issue Jun 15, 2023 · 0 comments · Fixed by #1042
Assignees

Comments

@spalladino
Copy link
Collaborator

Today contracts use a mix of addresses and public keys for referring to users in Noir contracts. For instance, the zk_token requires the pubkeys of sender and recipient (for decrypting/encrypting notes) but not their addresses, and its transfer function does not check that the sender is actually the msg.sender (meaning that a leaked encryption key causes loss of assets, even if the signing key is not compromised). And for the non_native token used in cross-chain tests, we're needing both the address and the pubkey as arguments.

Now, with the ongoing changes for supporting Account Abstraction, we're calculating deployment addresses as the hash of a public key, a salt, contract code, and constructor args. We can tweak this to hash(deployer_public_key, h1), where h1 := hash(salt, function_tree_root, constructor_hash), so that given public_key and h1, we can verify the resulting address.

This means we could implement an oracle call that, given an address, returns the corresponding public_key and h1, and Noir can verify that the oracle response is correct by hashing them and checking that the result is equal to the address (which Joe had originally suggested for getting the pubkey in the entrypoint of an account contract). And we could use this to stop passing public keys around in contracts, and just use addresses for referring to a user within contracts. This will require a method for sharing public_keys and h1s though (pretty much what Vitalik had identified on the Three Transitions post, but that's a longer story), but we can probably work around that for the devnet.

This'd mean we do not abstract encryption and nullifying keys for now, but if we hide this get_public_key logic behind a Noir library, it should be easy to change in the near future.

@spalladino spalladino added the S-needs-discussion Status: Still needs more discussion before work can start. label Jun 15, 2023
@spalladino spalladino added this to the 👍 Account Abstraction milestone Jun 15, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Jun 15, 2023
@spalladino spalladino self-assigned this Jul 11, 2023
@spalladino spalladino removed the S-needs-discussion Status: Still needs more discussion before work can start. label Jul 11, 2023
@spalladino spalladino moved this from Todo to In Progress in A3 Jul 11, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in A3 Jul 13, 2023
@iAmMichaelConnor iAmMichaelConnor removed this from A3 Jul 26, 2023
lucasxia01 added a commit that referenced this issue Oct 29, 2024
eccvm_recursive_verifier_test measurements (size-512 eccvm recursive
verification)

Old: 876,214
New: 678,751

The relative performance delta should be much greater for large eccvm
instances as this PR removes an nlogn algorithm.

This PR resolves issue
[#857](AztecProtocol/barretenberg#857) and
issue [#1023](AztecProtocol/barretenberg#1023)
(single batch mul in IPA)

Re: [#1023](AztecProtocol/barretenberg#1023).
The code still performs 2 batch muls, but all additional * operator
calls have been combined into the batch muls.

It is not worth combining both batch muls, as it would require a
multiplication operation on a large number of scalar multipliers. In the
recursive setting the scalars are bigfield elements - the extra
bigfield::operator* cost is not worth combining both batch_mul calls.

Additional improvements:

removed unneccessary uses of `pow` operator in ipa - in the recursive
setting these were stdlib::bigfield::pow calls and very expensive

removed the number of distinct multiplication calls in
ipa::reduce_verify_internal

cycle_scalar::cycle_scalar(stdlib::bigfield) constructor now more
optimally constructs a cycle_scalar out of a bigfield element. New
method leverages the fact that `scalar.lo` and `scalar.hi` are
implicitly range-constrained to remove reundant bigfield constructor
calls and arithmetic calls, and the process of performing a scalar
multiplication applies a modular reduction to the imput, which makes the
explicit call to `validate_scalar_is_in_field` unneccessary

---------
Co-authored-by: lucasxia01 <[email protected]>
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 a pull request may close this issue.

3 participants