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: re-enabling authwit constraint #6323

Merged
merged 4 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,29 @@ use dep::authwit::auth_witness;
use dep::aztec::protocol_types::{address::PartialAddress, grumpkin_point::GrumpkinPoint};

struct AuthWitness {
owner: GrumpkinPoint,
npk_m: GrumpkinPoint, ivpk_m: GrumpkinPoint, ovpk_m: GrumpkinPoint, tpk_m: GrumpkinPoint,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be made nicer in my PR up the stack.

signature: [u8; 64],
partial_address: PartialAddress,
}

impl AuthWitness {
fn deserialize(values: [Field; 67]) -> Self {
fn deserialize(values: [Field; 73]) -> Self {
let mut signature = [0; 64];
for i in 0..64 {
signature[i] = values[i + 2] as u8;
signature[i] = values[i + 8] as u8;
}
Self {
owner: GrumpkinPoint::new(values[0], values[1]),
npk_m: GrumpkinPoint::new(values[0], values[1]),
ivpk_m: GrumpkinPoint::new(values[2], values[3]),
ovpk_m: GrumpkinPoint::new(values[4], values[5]),
tpk_m: GrumpkinPoint::new(values[6], values[7]),
signature,
partial_address: PartialAddress::from_field(values[66])
partial_address: PartialAddress::from_field(values[72])
}
}
}

unconstrained pub fn get_auth_witness(message_hash: Field) -> AuthWitness {
let witness: [Field; 67] = auth_witness::get_auth_witness(message_hash);
let witness: [Field; 73] = auth_witness::get_auth_witness(message_hash);
AuthWitness::deserialize(witness)
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ contract SchnorrSingleKeyAccount {

use dep::authwit::{entrypoint::{app::AppPayload, fee::FeePayload}, account::AccountActions};

// use crate::{util::recover_address, auth_oracle::get_auth_witness};
use crate::auth_oracle::get_auth_witness;
use crate::{util::recover_address, auth_oracle::get_auth_witness};

global ACCOUNT_ACTIONS_STORAGE_SLOT = 1;

Expand Down Expand Up @@ -46,9 +45,7 @@ contract SchnorrSingleKeyAccount {
#[contract_library_method]
fn is_valid_impl(context: &mut PrivateContext, outer_hash: Field) -> bool {
let witness = get_auth_witness(outer_hash);
// TODO(#5830): the following is currently broken because we are no longer able to compute public keys hash
// in recover_address(...) just from user public key.
// assert(recover_address(outer_hash, witness).eq(context.this_address()));
assert(recover_address(outer_hash, witness).eq(context.this_address()));
true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ use dep::aztec::protocol_types::address::PublicKeysHash;
use dep::std::{schnorr::verify_signature_slice};
use crate::auth_oracle::AuthWitness;

// TODO(#5830): the following is currently broken because we are no longer able to compute public keys hash
// pub fn recover_address(message_hash: Field, witness: AuthWitness) -> AztecAddress {
// let message_bytes = message_hash.to_be_bytes(32);
// let verification = verify_signature_slice(
// witness.owner.x,
// witness.owner.y,
// witness.signature,
// message_bytes
// );
// assert(verification == true);
pub fn recover_address(message_hash: Field, witness: AuthWitness) -> AztecAddress {
let message_bytes = message_hash.to_be_bytes(32);
// In a single key account contract we re-used ivpk_m as signing key
let verification = verify_signature_slice(
witness.ivpk_m.x,
witness.ivpk_m.y,
witness.signature,
message_bytes
);
assert(verification == true);

// AztecAddress::compute(
// PublicKeysHash::compute(witness.owner),
// witness.partial_address
// )
// }
AztecAddress::compute(
PublicKeysHash::compute(witness.npk_m, witness.ivpk_m, witness.ovpk_m, witness.tpk_m),
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a bit weird to me here if the viewing key is used as the signing key? But might just be the confusion over the "single_key" when the signing key is one of the other keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree that it's weird. I created an issue for this a while ago and it's linked here.

It's like that currently because I wanted to minimize the number of changes when refactoring according to the new keys scheme and this was the most backwards compatible.

witness.partial_address
)
}
15 changes: 8 additions & 7 deletions yarn-project/accounts/src/single_key/account_contract.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { generatePublicKey } from '@aztec/aztec.js';
import { type AuthWitnessProvider } from '@aztec/aztec.js/account';
import { AuthWitness, type CompleteAddress, type GrumpkinPrivateKey } from '@aztec/circuit-types';
import { type PartialAddress } from '@aztec/circuits.js';
import { Schnorr } from '@aztec/circuits.js/barretenberg';
import { type ContractArtifact } from '@aztec/foundation/abi';
import { type Fr } from '@aztec/foundation/fields';
Expand All @@ -22,8 +20,8 @@ export class SingleKeyAccountContract extends DefaultAccountContract {
return undefined;
}

getAuthWitnessProvider({ partialAddress }: CompleteAddress): AuthWitnessProvider {
return new SingleKeyAuthWitnessProvider(this.encryptionPrivateKey, partialAddress);
getAuthWitnessProvider(account: CompleteAddress): AuthWitnessProvider {
return new SingleKeyAuthWitnessProvider(this.encryptionPrivateKey, account);
}
}

Expand All @@ -33,13 +31,16 @@ export class SingleKeyAccountContract extends DefaultAccountContract {
* by reconstructing the current address.
*/
class SingleKeyAuthWitnessProvider implements AuthWitnessProvider {
constructor(private privateKey: GrumpkinPrivateKey, private partialAddress: PartialAddress) {}
constructor(private privateKey: GrumpkinPrivateKey, private account: CompleteAddress) {}

createAuthWit(messageHash: Fr): Promise<AuthWitness> {
const schnorr = new Schnorr();
const signature = schnorr.constructSignature(messageHash.toBuffer(), this.privateKey);
const publicKey = generatePublicKey(this.privateKey);
const witness = [...publicKey.toFields(), ...signature.toBuffer(), this.partialAddress];
const witness = [
...this.account.publicKeys.flatMap(pk => pk.toFields()),
...signature.toBuffer(),
this.account.partialAddress,
];
return Promise.resolve(new AuthWitness(messageHash, witness));
}
}
3 changes: 1 addition & 2 deletions yarn-project/aztec.js/src/account_manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ export class AccountManager {
);
}
await this.#register();
const encryptionPublicKey = this.getPublicKeysHash();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an issue unrelated stale naming fix.

const { chainId, protocolVersion } = await this.pxe.getNodeInfo();
const deployWallet = new SignerlessWallet(this.pxe, new DefaultMultiCallEntrypoint(chainId, protocolVersion));

Expand All @@ -135,7 +134,7 @@ export class AccountManager {
const args = this.accountContract.getDeploymentArgs() ?? [];
this.deployMethod = new DeployAccountMethod(
this.accountContract.getAuthWitnessProvider(this.getCompleteAddress()),
encryptionPublicKey,
this.getPublicKeysHash(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an issue unrelated stale naming fix.

deployWallet,
this.accountContract.getContractArtifact(),
args,
Expand Down
12 changes: 8 additions & 4 deletions yarn-project/circuits.js/src/structs/complete_address.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AztecAddress } from '@aztec/foundation/aztec-address';
import { Fr, Point } from '@aztec/foundation/fields';
import { BufferReader } from '@aztec/foundation/serialize';
import { BufferReader, type Tuple } from '@aztec/foundation/serialize';

import { computePartialAddress } from '../contract/contract_address.js';
import { computeAddress, computePublicKeysHash, deriveKeys } from '../keys/index.js';
Expand Down Expand Up @@ -195,12 +195,16 @@ export class CompleteAddress {
return `0x${this.toBuffer().toString('hex')}`;
}

get publicKeysHash(): Fr {
return computePublicKeysHash(
get publicKeys(): Tuple<PublicKey, 4> {
return [
this.masterNullifierPublicKey,
this.masterIncomingViewingPublicKey,
this.masterOutgoingViewingPublicKey,
this.masterTaggingPublicKey,
);
];
}

get publicKeysHash(): Fr {
return computePublicKeysHash(...this.publicKeys);
}
}
7 changes: 3 additions & 4 deletions yarn-project/end-to-end/src/e2e_account_contracts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
type PXE,
type Wallet,
} from '@aztec/aztec.js';
import { deriveSigningKey } from '@aztec/circuits.js/keys';
import { randomBytes } from '@aztec/foundation/crypto';
import { ChildContract } from '@aztec/noir-contracts.js/Child';

Expand All @@ -27,7 +28,6 @@ function itShouldBehaveLikeAnAccountContract(
let child: ChildContract;
let wallet: Wallet;
let secretKey: Fr;
let signingKey: GrumpkinPrivateKey;

let pxe: PXE;
let logger: DebugLogger;
Expand All @@ -36,7 +36,7 @@ function itShouldBehaveLikeAnAccountContract(
beforeEach(async () => {
({ logger, pxe, teardown } = await setup(0));
secretKey = Fr.random();
signingKey = GrumpkinScalar.random();
const signingKey = deriveSigningKey(secretKey);

wallet = await walletSetup(pxe, secretKey, getAccountContract(signingKey));
child = await ChildContract.deploy(wallet).send().deployed();
Expand All @@ -56,8 +56,7 @@ function itShouldBehaveLikeAnAccountContract(
expect(storedValue).toEqual(new Fr(42n));
});

// TODO(#5830): re-enable this test
it.skip('fails to call a function using an invalid signature', async () => {
it('fails to call a function using an invalid signature', async () => {
const accountAddress = wallet.getCompleteAddress();
const invalidWallet = await walletAt(pxe, getAccountContract(GrumpkinScalar.random()), accountAddress);
const childWithInvalidWallet = await ChildContract.at(child.address, invalidWallet);
Expand Down
Loading