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

Conversation

benesjan
Copy link
Contributor

@benesjan benesjan commented May 13, 2024

Fixes #5664
+ sneaked in various minor cleanups --> not bothering with having a separate PR due to having enough fun with CI and low complexity of this one.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @benesjan and the rest of your teammates on Graphite Graphite

@benesjan benesjan force-pushed the 05-13-feat_demonstrating_use_of_nsk_app_to_check_nullification branch 2 times, most recently from 40230e0 to 4ed82ad Compare May 14, 2024 08:17
@@ -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.

@@ -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.

@@ -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.

@@ -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.

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.

@@ -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.

@@ -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.

@benesjan benesjan marked this pull request as ready for review May 14, 2024 08:35
@benesjan benesjan requested a review from LHerskind May 14, 2024 09:54
Copy link
Contributor

@LHerskind LHerskind left a comment

Choose a reason for hiding this comment

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

Not obvious to me if the change broke the authwit test, or if that is unrelated and just ci.

The signing key should still be similar, so strange to me 🤔

@benesjan benesjan force-pushed the 05-13-feat_demonstrating_use_of_nsk_app_to_check_nullification branch from 7c7901f to 20e7b2b Compare May 14, 2024 11:10
@benesjan benesjan requested a review from LHerskind May 14, 2024 12:09
@benesjan benesjan enabled auto-merge (squash) May 14, 2024 12:10
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 👀

@benesjan benesjan merged commit ddf4461 into master May 14, 2024
90 of 111 checks passed
@benesjan benesjan deleted the 05-13-feat_demonstrating_use_of_nsk_app_to_check_nullification branch May 14, 2024 14:30
ludamad pushed a commit that referenced this pull request May 14, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.40.0</summary>

##
[0.40.0](aztec-package-v0.39.0...aztec-package-v0.40.0)
(2024-05-14)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.40.0</summary>

##
[0.40.0](barretenberg.js-v0.39.0...barretenberg.js-v0.40.0)
(2024-05-14)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.40.0</summary>

##
[0.40.0](aztec-packages-v0.39.0...aztec-packages-v0.40.0)
(2024-05-14)


### ⚠ BREAKING CHANGES

* debug logs for all
([#6392](#6392))

### Features

* Add wasmtime
([#6314](#6314))
([ea6ccdd](ea6ccdd))
* Debug logs for all
([#6392](#6392))
([10afa13](10afa13))
* Demonstrating use of nsk_app to check nullification
([#6362](#6362))
([ddf4461](ddf4461))
* Use gas estimation in aztecjs contract function interactions
([#6260](#6260))
([18192ac](18192ac))


### Miscellaneous

* Add more serialisation traits to protocol circuits
([#6385](#6385))
([97d5422](97d5422))
* **ci:** Bump timeout of prover-client-test
([#6394](#6394))
([d05cd07](d05cd07))
* Reenable bench summary
([#6211](#6211))
([713b243](713b243))
* **token-contract-tests:** Change intrinsic assertion messages
([#6386](#6386))
([aca81ae](aca81ae))
</details>

<details><summary>barretenberg: 0.40.0</summary>

##
[0.40.0](barretenberg-v0.39.0...barretenberg-v0.40.0)
(2024-05-14)


### ⚠ BREAKING CHANGES

* debug logs for all
([#6392](#6392))

### Features

* Debug logs for all
([#6392](#6392))
([10afa13](10afa13))


### Miscellaneous

* Add more serialisation traits to protocol circuits
([#6385](#6385))
([97d5422](97d5422))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request May 15, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.40.0</summary>

##
[0.40.0](AztecProtocol/aztec-packages@aztec-package-v0.39.0...aztec-package-v0.40.0)
(2024-05-14)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.40.0</summary>

##
[0.40.0](AztecProtocol/aztec-packages@barretenberg.js-v0.39.0...barretenberg.js-v0.40.0)
(2024-05-14)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.40.0</summary>

##
[0.40.0](AztecProtocol/aztec-packages@aztec-packages-v0.39.0...aztec-packages-v0.40.0)
(2024-05-14)


### ⚠ BREAKING CHANGES

* debug logs for all
([#6392](AztecProtocol/aztec-packages#6392))

### Features

* Add wasmtime
([#6314](AztecProtocol/aztec-packages#6314))
([ea6ccdd](AztecProtocol/aztec-packages@ea6ccdd))
* Debug logs for all
([#6392](AztecProtocol/aztec-packages#6392))
([10afa13](AztecProtocol/aztec-packages@10afa13))
* Demonstrating use of nsk_app to check nullification
([#6362](AztecProtocol/aztec-packages#6362))
([ddf4461](AztecProtocol/aztec-packages@ddf4461))
* Use gas estimation in aztecjs contract function interactions
([#6260](AztecProtocol/aztec-packages#6260))
([18192ac](AztecProtocol/aztec-packages@18192ac))


### Miscellaneous

* Add more serialisation traits to protocol circuits
([#6385](AztecProtocol/aztec-packages#6385))
([97d5422](AztecProtocol/aztec-packages@97d5422))
* **ci:** Bump timeout of prover-client-test
([#6394](AztecProtocol/aztec-packages#6394))
([d05cd07](AztecProtocol/aztec-packages@d05cd07))
* Reenable bench summary
([#6211](AztecProtocol/aztec-packages#6211))
([713b243](AztecProtocol/aztec-packages@713b243))
* **token-contract-tests:** Change intrinsic assertion messages
([#6386](AztecProtocol/aztec-packages#6386))
([aca81ae](AztecProtocol/aztec-packages@aca81ae))
</details>

<details><summary>barretenberg: 0.40.0</summary>

##
[0.40.0](AztecProtocol/aztec-packages@barretenberg-v0.39.0...barretenberg-v0.40.0)
(2024-05-14)


### ⚠ BREAKING CHANGES

* debug logs for all
([#6392](AztecProtocol/aztec-packages#6392))

### Features

* Debug logs for all
([#6392](AztecProtocol/aztec-packages#6392))
([10afa13](AztecProtocol/aztec-packages@10afa13))


### Miscellaneous

* Add more serialisation traits to protocol circuits
([#6385](AztecProtocol/aztec-packages#6385))
([97d5422](AztecProtocol/aztec-packages@97d5422))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
iakovenkos pushed a commit that referenced this pull request May 15, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.40.0</summary>

##
[0.40.0](aztec-package-v0.39.0...aztec-package-v0.40.0)
(2024-05-14)


### Miscellaneous

* **aztec-package:** Synchronize aztec-packages versions
</details>

<details><summary>barretenberg.js: 0.40.0</summary>

##
[0.40.0](barretenberg.js-v0.39.0...barretenberg.js-v0.40.0)
(2024-05-14)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.40.0</summary>

##
[0.40.0](aztec-packages-v0.39.0...aztec-packages-v0.40.0)
(2024-05-14)


### ⚠ BREAKING CHANGES

* debug logs for all
([#6392](#6392))

### Features

* Add wasmtime
([#6314](#6314))
([ea6ccdd](ea6ccdd))
* Debug logs for all
([#6392](#6392))
([10afa13](10afa13))
* Demonstrating use of nsk_app to check nullification
([#6362](#6362))
([ddf4461](ddf4461))
* Use gas estimation in aztecjs contract function interactions
([#6260](#6260))
([18192ac](18192ac))


### Miscellaneous

* Add more serialisation traits to protocol circuits
([#6385](#6385))
([97d5422](97d5422))
* **ci:** Bump timeout of prover-client-test
([#6394](#6394))
([d05cd07](d05cd07))
* Reenable bench summary
([#6211](#6211))
([713b243](713b243))
* **token-contract-tests:** Change intrinsic assertion messages
([#6386](#6386))
([aca81ae](aca81ae))
</details>

<details><summary>barretenberg: 0.40.0</summary>

##
[0.40.0](barretenberg-v0.39.0...barretenberg-v0.40.0)
(2024-05-14)


### ⚠ BREAKING CHANGES

* debug logs for all
([#6392](#6392))

### Features

* Debug logs for all
([#6392](#6392))
([10afa13](10afa13))


### Miscellaneous

* Add more serialisation traits to protocol circuits
([#6385](#6385))
([97d5422](97d5422))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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 this pull request may close these issues.

test(Keys_Nullifier): Have 3rd party watch note be nullified
2 participants