Skip to content

Commit

Permalink
feat!: call stack validation optimisation. (#3387)
Browse files Browse the repository at this point in the history
Closes #3060 

### Before
We passed **call stack preimages** to the kernel. The kernel then hashed
the preimages and check them against the hashes in the app circuits'
public inputs. The public kernel also checked the addresses and contexts
in the preimages were valid for the types of calls they were making
(private kernel should've done the same check, but was not present in
the cpp code). The hashes would be aggregated to the accumulated data.
A nested execution would pop the hash from the accumulated data, and
checked that it's the same as its own hash.

### Now
We pass **call requests** to the kernel. The kernel checks that the
addresses and contexts in those requests match the current call. The
requests are aggregated to the accumulated data.
A nested execution pops the call request from the accumulated data,
checks that the hash is the same as its own hash, and validates the
addresses and contexts are correct for the type of call.

### Other changes

- Remove unnecessary checks in the protocol circuits: 
  - Array lengths when they were already fixed sizes.
  - Format of the arrays from previous kernel.
- Check the function exist in the tree using contract address instead of
storage contract address.
- Removing `yarn:remake-bindings`: Stop generating types and utils from
cpp.
- Deleting some types and utils generated from cpp that are not used
anywhere. (A thorough cleanup will be in another PR.)

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [ ] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [ ] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [ ] Every change is related to the PR description.
- [ ] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).
  • Loading branch information
LeilaWang authored Nov 27, 2023
1 parent 4a1e9c3 commit d06d5db
Show file tree
Hide file tree
Showing 96 changed files with 16,506 additions and 12,192 deletions.
4 changes: 2 additions & 2 deletions yarn-project/acir-simulator/src/acvm/serialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ export function toACVMPublicInputs(publicInputs: PrivateCircuitPublicInputs): AC
...publicInputs.newCommitments.map(toACVMField),
...publicInputs.newNullifiers.map(toACVMField),
...publicInputs.nullifiedCommitments.map(toACVMField),
...publicInputs.privateCallStack.map(toACVMField),
...publicInputs.publicCallStack.map(toACVMField),
...publicInputs.privateCallStackHashes.map(toACVMField),
...publicInputs.publicCallStackHashes.map(toACVMField),
...publicInputs.newL2ToL1Msgs.map(toACVMField),
...publicInputs.encryptedLogsHash.map(toACVMField),
...publicInputs.unencryptedLogsHash.map(toACVMField),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
TxContext,
} from '@aztec/circuits.js';
import {
computeCallStackItemHash,
computeCommitmentNonce,
computeSecretMessageHash,
computeVarArgsHash,
Expand Down Expand Up @@ -371,8 +370,8 @@ describe('Private Execution test suite', () => {
expect(result.nestedExecutions[0].callStackItem.publicInputs.returnValues[0]).toEqual(new Fr(privateIncrement));

// check that Aztec.nr calculated the call stack item hash like cpp does
const expectedCallStackItemHash = computeCallStackItemHash(result.nestedExecutions[0].callStackItem);
expect(result.callStackItem.publicInputs.privateCallStack[0]).toEqual(expectedCallStackItemHash);
const expectedCallStackItemHash = result.nestedExecutions[0].callStackItem.hash();
expect(result.callStackItem.publicInputs.privateCallStackHashes[0]).toEqual(expectedCallStackItemHash);
});
});

Expand Down Expand Up @@ -557,11 +556,11 @@ describe('Private Execution test suite', () => {
sideEffectCounter: 0,
});

const publicCallRequestHash = computeCallStackItemHash(publicCallRequest.toPublicCallStackItem());
const publicCallRequestHash = publicCallRequest.toPublicCallStackItem().hash();

expect(result.enqueuedPublicFunctionCalls).toHaveLength(1);
expect(result.enqueuedPublicFunctionCalls[0]).toEqual(publicCallRequest);
expect(result.callStackItem.publicInputs.publicCallStack[0]).toEqual(publicCallRequestHash);
expect(result.callStackItem.publicInputs.publicCallStackHashes[0]).toEqual(publicCallRequestHash);
});
});

Expand Down
3 changes: 1 addition & 2 deletions yarn-project/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ for package in "@aztec/foundation" "@aztec/noir-compiler"; do
yarn workspace $package build
done

# Run remake bindings before building Aztec.nr contracts or l1 contracts as they depend on files created by it.
yarn workspace @aztec/circuits.js remake-bindings
# Run remake constants before building Aztec.nr contracts or l1 contracts as they depend on files created by it.
yarn workspace @aztec/circuits.js remake-constants

(cd noir-contracts && ./bootstrap.sh)
Expand Down
1 change: 0 additions & 1 deletion yarn-project/circuits.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"remake-bindings": "DEBUG=wasm ts-node-esm src/cbind/circuits.in.ts && prettier -w src/cbind/circuits.gen.ts",
"remake-constants": "ts-node-esm src/cbind/constants.in.ts && prettier -w src/cbind/constants.gen.ts",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests"
},
Expand Down
7 changes: 4 additions & 3 deletions yarn-project/circuits.js/src/abis/abis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
} from '../tests/factories.js';
import {
computeBlockHashWithGlobals,
computeCallStackItemHash,
computeCommitmentNonce,
computeCompleteAddress,
computeContractAddressFromPartial,
Expand All @@ -29,6 +28,8 @@ import {
computeFunctionSelector,
computeFunctionTreeRoot,
computeGlobalsHash,
computePrivateCallStackItemHash,
computePublicCallStackItemHash,
computePublicDataTreeIndex,
computePublicDataTreeValue,
computeSecretMessageHash,
Expand Down Expand Up @@ -209,13 +210,13 @@ describe('abis wasm bindings', () => {

it('compute private call stack item hash', () => {
const item = makePrivateCallStackItem();
const hash = computeCallStackItemHash(item);
const hash = computePrivateCallStackItemHash(item);
expect(hash).toMatchSnapshot();
});

it('compute public call stack item hash', () => {
const item = makePublicCallStackItem();
const hash = computeCallStackItemHash(item);
const hash = computePublicCallStackItemHash(item);
expect(hash).toMatchSnapshot();
});

Expand Down
21 changes: 3 additions & 18 deletions yarn-project/circuits.js/src/abis/abis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,21 +490,6 @@ function computeContractDeploymentDataHash(data: ContractDeploymentData): Fr {
);
}

/**
* Computes a call stack item hash.
* @param callStackItem - The call stack item.
* @returns The call stack item hash.
*/
export function computeCallStackItemHash(callStackItem: PrivateCallStackItem | PublicCallStackItem): Fr {
if (callStackItem instanceof PrivateCallStackItem) {
return computePrivateCallStackItemHash(callStackItem);
} else if (callStackItem instanceof PublicCallStackItem) {
return computePublicCallStackItemHash(callStackItem);
} else {
throw new Error(`Unexpected call stack item type`);
}
}

/**
*
*/
Expand Down Expand Up @@ -536,8 +521,8 @@ function computePrivateInputsHash(input: PrivateCircuitPublicInputs) {
...input.newCommitments.map(fr => fr.toBuffer()),
...input.newNullifiers.map(fr => fr.toBuffer()),
...input.nullifiedCommitments.map(fr => fr.toBuffer()),
...input.privateCallStack.map(fr => fr.toBuffer()),
...input.publicCallStack.map(fr => fr.toBuffer()),
...input.privateCallStackHashes.map(fr => fr.toBuffer()),
...input.publicCallStackHashes.map(fr => fr.toBuffer()),
...input.newL2ToL1Msgs.map(fr => fr.toBuffer()),
...input.encryptedLogsHash.map(fr => fr.toBuffer()),
...input.unencryptedLogsHash.map(fr => fr.toBuffer()),
Expand Down Expand Up @@ -605,7 +590,7 @@ function computePublicInputsHash(input: PublicCircuitPublicInputs) {
...input.returnValues.map(fr => fr.toBuffer()),
...input.contractStorageUpdateRequests.map(computeContractStorageUpdateRequestHash),
...input.contractStorageReads.map(computeContractStorageReadsHash),
...input.publicCallStack.map(fr => fr.toBuffer()),
...input.publicCallStackHashes.map(fr => fr.toBuffer()),
...input.newCommitments.map(fr => fr.toBuffer()),
...input.newNullifiers.map(fr => fr.toBuffer()),
...input.newL2ToL1Msgs.map(fr => fr.toBuffer()),
Expand Down
Loading

0 comments on commit d06d5db

Please sign in to comment.