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: demonstrating use of nsk_app to check nullification #6362

Merged
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
5 changes: 2 additions & 3 deletions yarn-project/accounts/src/single_key/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
import { AccountManager, type Salt } from '@aztec/aztec.js/account';
import { type AccountWallet, getWallet } from '@aztec/aztec.js/wallet';
import { type GrumpkinPrivateKey, type PXE } from '@aztec/circuit-types';
import { type AztecAddress, type Fr, GeneratorIndex } from '@aztec/circuits.js';
import { sha512ToGrumpkinScalar } from '@aztec/foundation/crypto';
import { type AztecAddress, type Fr, deriveMasterIncomingViewingSecretKey } from '@aztec/circuits.js';

import { SingleKeyAccountContract } from './account_contract.js';

Expand All @@ -23,7 +22,7 @@ export { SchnorrSingleKeyAccountContractArtifact as SingleKeyAccountContractArti
* @param salt - Deployment salt.
*/
export function getSingleKeyAccount(pxe: PXE, secretKey: Fr, salt?: Salt): AccountManager {
const encryptionPrivateKey = sha512ToGrumpkinScalar([secretKey, GeneratorIndex.IVSK_M]);
const encryptionPrivateKey = deriveMasterIncomingViewingSecretKey(secretKey);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaked this change in - it's better to use 1 function for this.

return new AccountManager(pxe, secretKey, new SingleKeyAccountContract(encryptionPrivateKey), salt);
}

Expand Down
9 changes: 4 additions & 5 deletions yarn-project/accounts/src/testing/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { generatePublicKey } from '@aztec/aztec.js';
import { type AccountWalletWithSecretKey } from '@aztec/aztec.js/wallet';
import { type PXE } from '@aztec/circuit-types';
import { GeneratorIndex } from '@aztec/circuits.js/constants';
import { sha512ToGrumpkinScalar } from '@aztec/foundation/crypto';
import { deriveMasterIncomingViewingSecretKey, deriveSigningKey } from '@aztec/circuits.js/keys';
import { Fr } from '@aztec/foundation/fields';

import { getSchnorrAccount } from '../schnorr/index.js';
Expand All @@ -14,7 +13,7 @@ export const INITIAL_TEST_SECRET_KEYS = [
];

export const INITIAL_TEST_ENCRYPTION_KEYS = INITIAL_TEST_SECRET_KEYS.map(secretKey =>
sha512ToGrumpkinScalar([secretKey, GeneratorIndex.IVSK_M]),
deriveMasterIncomingViewingSecretKey(secretKey),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaked this change in - it's better to use 1 function for this.

);
// TODO(#5837): come up with a standard signing key derivation scheme instead of using ivsk_m as signing keys here
export const INITIAL_TEST_SIGNING_KEYS = INITIAL_TEST_ENCRYPTION_KEYS;
Expand Down Expand Up @@ -43,14 +42,14 @@ export async function getDeployedTestAccountsWallets(pxe: PXE): Promise<AccountW
const registeredAccounts = await pxe.getRegisteredAccounts();
return Promise.all(
INITIAL_TEST_SECRET_KEYS.filter(initialSecretKey => {
const initialEncryptionKey = sha512ToGrumpkinScalar([initialSecretKey, GeneratorIndex.IVSK_M]);
const initialEncryptionKey = deriveMasterIncomingViewingSecretKey(initialSecretKey);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaked this change in - it's better to use 1 function for this.

const publicKey = generatePublicKey(initialEncryptionKey);
return (
registeredAccounts.find(registered => registered.publicKeys.masterIncomingViewingPublicKey.equals(publicKey)) !=
undefined
);
}).map(secretKey => {
const signingKey = sha512ToGrumpkinScalar([secretKey, GeneratorIndex.IVSK_M]);
const signingKey = deriveSigningKey(secretKey);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaked this change in - it's better to use 1 function for this.

// TODO(#5726): use actual salt here instead of hardcoding Fr.ZERO
return getSchnorrAccount(pxe, secretKey, signingKey, Fr.ZERO).getWallet();
}),
Expand Down
26 changes: 18 additions & 8 deletions yarn-project/accounts/src/testing/create_account.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { type AccountWalletWithSecretKey } from '@aztec/aztec.js/wallet';
import { type PXE } from '@aztec/circuit-types';
import { Fr, GeneratorIndex } from '@aztec/circuits.js';
import { sha512ToGrumpkinScalar } from '@aztec/foundation/crypto';
import { Fr, deriveSigningKey } from '@aztec/circuits.js';

import { getSchnorrAccount } from '../schnorr/index.js';

Expand All @@ -12,24 +11,35 @@ import { getSchnorrAccount } from '../schnorr/index.js';
*/
export function createAccount(pxe: PXE): Promise<AccountWalletWithSecretKey> {
const secretKey = Fr.random();
const signingKey = sha512ToGrumpkinScalar([secretKey, GeneratorIndex.IVSK_M]);
const signingKey = deriveSigningKey(secretKey);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaked this change in - it's better to use 1 function for this.

return getSchnorrAccount(pxe, secretKey, signingKey).waitSetup();
}

/**
* Creates a given number of random accounts using the Schnorr account wallet.
* @param pxe - PXE.
* @param numberOfAccounts - How many accounts to create.
* @param secrets - Optional array of secrets to use for the accounts. If empty, random secrets will be generated.
* @throws If the secrets array is not empty and does not have the same length as the number of accounts.
* @returns The created account wallets.
*/
export async function createAccounts(pxe: PXE, numberOfAccounts = 1): Promise<AccountWalletWithSecretKey[]> {
export async function createAccounts(
pxe: PXE,
numberOfAccounts = 1,
secrets: Fr[] = [],
): Promise<AccountWalletWithSecretKey[]> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added secrets param here to be able to generate nsk_app for the account (I need the secret for that). There is a getNullifierKeys API on PXE database which is used by oracles but it didn't seem to make sense to expose it on PXE as I don't think it will be needed by anything else but this 1 test case.

const accounts = [];

if (secrets.length == 0) {
secrets = Array.from({ length: numberOfAccounts }, () => Fr.random());
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👀

} else if (secrets.length > 0 && secrets.length !== numberOfAccounts) {
throw new Error('Secrets array must be empty or have the same length as the number of accounts');
}

// Prepare deployments
for (let i = 0; i < numberOfAccounts; ++i) {
const secretKey = Fr.random();
const signingKey = sha512ToGrumpkinScalar([secretKey, GeneratorIndex.IVSK_M]);
const account = getSchnorrAccount(pxe, secretKey, signingKey);
for (const secret of secrets) {
const signingKey = deriveSigningKey(secret);
const account = getSchnorrAccount(pxe, secret, signingKey);
// Unfortunately the function below is not stateless and we call it here because it takes a long time to run and
// the results get stored within the account object. By calling it here we increase the probability of all the
// accounts being deployed in the same block because it makes the deploy() method basically instant.
Expand Down
67 changes: 64 additions & 3 deletions yarn-project/end-to-end/src/e2e_key_registry.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import { type AccountWallet, AztecAddress, Fr, type PXE } from '@aztec/aztec.js';
import { CompleteAddress, Point, PublicKeys } from '@aztec/circuits.js';
import { createAccounts } from '@aztec/accounts/testing';
import { type AccountWallet, AztecAddress, type AztecNode, Fr, type L2Block, type PXE } from '@aztec/aztec.js';
import {
CompleteAddress,
GeneratorIndex,
INITIAL_L2_BLOCK_NUM,
Point,
PublicKeys,
computeAppNullifierSecretKey,
deriveMasterNullifierSecretKey,
} from '@aztec/circuits.js';
import { siloNullifier } from '@aztec/circuits.js/hash';
import { poseidon2Hash } from '@aztec/foundation/crypto';
import { KeyRegistryContract, TestContract } from '@aztec/noir-contracts.js';
import { getCanonicalKeyRegistryAddress } from '@aztec/protocol-contracts/key-registry';

Expand All @@ -15,6 +26,7 @@ describe('Key Registry', () => {
let keyRegistry: KeyRegistryContract;

let pxe: PXE;
let aztecNode: AztecNode;
let testContract: TestContract;
jest.setTimeout(TIMEOUT);

Expand All @@ -25,7 +37,7 @@ describe('Key Registry', () => {
const account = CompleteAddress.random();

beforeAll(async () => {
({ teardown, pxe, wallets } = await setup(3));
({ aztecNode, teardown, pxe, wallets } = await setup(2));
keyRegistry = await KeyRegistryContract.at(getCanonicalKeyRegistryAddress(), wallets[0]);

testContract = await TestContract.deploy(wallets[0]).send().deployed();
Expand Down Expand Up @@ -208,4 +220,53 @@ describe('Key Registry', () => {
.wait();
});
});

describe('using nsk_app to detect nullification', () => {
// This test checks that it possible to detect that a note has been nullified just by using nsk_app. Note that this
// only works for non-transient note as transient notes never emit a note hash which makes it impossible to brute
// force their nullifier. This makes this scheme a bit useless in practice.
it('nsk_app and contract address are enough to detect note nullification', async () => {
const secret = Fr.random();
const [account] = await createAccounts(pxe, 1, [secret]);

const masterNullifierSecretKey = deriveMasterNullifierSecretKey(secret);
const nskApp = computeAppNullifierSecretKey(masterNullifierSecretKey, testContract.address);

const noteValue = 5;
const noteOwner = account.getAddress();
const noteStorageSlot = 12;

await testContract.methods.call_create_note(noteValue, noteOwner, noteStorageSlot).send().wait();

expect(await getNumNullifiedNotes(nskApp, testContract.address)).toEqual(0);

await testContract.methods.call_destroy_note(noteStorageSlot).send().wait();

expect(await getNumNullifiedNotes(nskApp, testContract.address)).toEqual(1);
});

const getNumNullifiedNotes = async (nskApp: Fr, contractAddress: AztecAddress) => {
// 1. Get all the note hashes
const blocks = await aztecNode.getBlocks(INITIAL_L2_BLOCK_NUM, 1000);
const noteHashes = blocks.flatMap((block: L2Block) =>
block.body.txEffects.flatMap(txEffect => txEffect.noteHashes),
);
// 2. Get all the seen nullifiers
const nullifiers = blocks.flatMap((block: L2Block) =>
block.body.txEffects.flatMap(txEffect => txEffect.nullifiers),
);
// 3. Derive all the possible nullifiers using nskApp
const derivedNullifiers = noteHashes.map(noteHash => {
const innerNullifier = poseidon2Hash([noteHash, nskApp, GeneratorIndex.NOTE_NULLIFIER]);
return siloNullifier(contractAddress, innerNullifier);
});
// 4. Count the number of derived nullifiers that are in the nullifiers array
return derivedNullifiers.reduce((count, derived) => {
if (nullifiers.some(nullifier => nullifier.equals(derived))) {
count++;
}
return count;
}, 0);
};
});
});
5 changes: 1 addition & 4 deletions yarn-project/end-to-end/src/shared/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ export const browserTestSuite = (
createPXEClient,
getSchnorrAccount,
Contract,
deriveKeys,
Fr,
ExtendedNote,
Note,
Expand Down Expand Up @@ -248,11 +247,9 @@ export const browserTestSuite = (
knownAccounts.push(newAccount);
}
const owner = knownAccounts[0];
// TODO(#5726): this is messy, maybe we should expose publicKeysHash on account
const publicKeysHash = deriveKeys(INITIAL_TEST_SECRET_KEYS[0]).publicKeys.hash();
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 was no longer necessary after I cleaned up the complete address.

const ownerAddress = owner.getAddress();
const tx = new DeployMethod(
publicKeysHash,
owner.getCompleteAddress().publicKeys.hash(),
owner,
TokenContractArtifact,
(a: AztecJs.AztecAddress) => Contract.at(a, TokenContractArtifact, owner),
Expand Down
4 changes: 0 additions & 4 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -804,10 +804,6 @@ export class PXEService implements PXE {
return Promise.resolve(this.synchronizer.getSyncStatus());
}

public getKeyStore() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nuked this as it seemed like something we don't want exposed and it was not used.

return this.keyStore;
}

public async isContractClassPubliclyRegistered(id: Fr): Promise<boolean> {
return !!(await this.node.getContractClass(id));
}
Expand Down
Loading