From bd5c8793c91214ee2e85ded245e336953fe7abdf Mon Sep 17 00:00:00 2001 From: Gregorio Juliana Date: Tue, 5 Mar 2024 14:22:39 +0100 Subject: [PATCH] chore: fixed call nesting, tests and docs (#4932) Added missing tests and fixed context propagation for nested static calls. Also included docs and migration notes! @AztecProtocol/devrel --- .../writing_contracts/functions/main.md | 2 +- docs/docs/developers/limitations/main.md | 2 - docs/docs/learn/concepts/accounts/authwit.md | 4 -- docs/docs/misc/migration_notes.md | 10 +++ .../aztec/src/context/private_context.nr | 2 + .../contracts/parent_contract/src/main.nr | 68 +++++++++++++++++++ .../end-to-end/src/e2e_static_calls.test.ts | 60 +++++++++++++++- .../src/client/client_execution_context.ts | 4 ++ .../src/public/public_execution_context.ts | 2 + yellow-paper/docs/calls/static-calls.md | 9 +-- 10 files changed, 147 insertions(+), 16 deletions(-) diff --git a/docs/docs/developers/contracts/writing_contracts/functions/main.md b/docs/docs/developers/contracts/writing_contracts/functions/main.md index 8164067e8cb..91b0fea0ca6 100644 --- a/docs/docs/developers/contracts/writing_contracts/functions/main.md +++ b/docs/docs/developers/contracts/writing_contracts/functions/main.md @@ -6,7 +6,7 @@ Functions serve as the building blocks of smart contracts. Functions can be eith For a more practical guide of using multiple types of functions, follow the [token tutorial](../../../tutorials/writing_token_contract.md). -Currently, any function is "mutable" in the sense that it might alter state. In the future, we will support static calls, similarly to EVM. A static call is essentially a call that does not alter state (it keeps state static). +Currently, any function is "mutable" in the sense that it might alter state. However, we also support support static calls, similarly to EVM. A static call is essentially a call that does not alter state (it keeps state static). ## Constructors diff --git a/docs/docs/developers/limitations/main.md b/docs/docs/developers/limitations/main.md index 1e6619333e3..baf3e719021 100644 --- a/docs/docs/developers/limitations/main.md +++ b/docs/docs/developers/limitations/main.md @@ -30,8 +30,6 @@ Help shape and define: - It is a testing environment, it is insecure, unaudited and does not generate any proofs, its only for testing purposes; - Constructors can not call nor alter public state - The constructor is executed exclusively in private domain, WITHOUT the ability to call public functions or alter public state. This means to set initial storage values, you need to follow a pattern similar to [proxies in Ethereum](https://blog.openzeppelin.com/proxy-patterns), where you `initialize` the contract with values after it have been deployed, see [constructor](../contracts/writing_contracts/functions/write_constructor.md). -- No static nor delegate calls (see [mutability](../contracts/writing_contracts/functions/main.md)). - - These values are unused in the call-context. - Beware that what you think of as a `view` could alter state ATM! Notably the account could alter state or re-enter whenever the account contract's `is_valid` function is called. - `msg_sender` is currently leaking when doing private -> public calls - The `msg_sender` will always be set, if you call a public function from the private world, the `msg_sender` will be set to the private caller's address. See [function context](../contracts/writing_contracts/functions/context.md). diff --git a/docs/docs/learn/concepts/accounts/authwit.md b/docs/docs/learn/concepts/accounts/authwit.md index 31fa51e1efa..b8412741746 100644 --- a/docs/docs/learn/concepts/accounts/authwit.md +++ b/docs/docs/learn/concepts/accounts/authwit.md @@ -132,10 +132,6 @@ sequenceDiagram The call to the account contract for checking authentication should be a static call, meaning that it cannot change state or make calls that change state. If this call is not static, it could be used to re-enter the flow and change the state of the contract. ::: -:::danger Static call currently unsupported -The current execution layer does not implement static call. So currently you will be passing along the control flow :grimacing:. -::: - :::danger Re-entries The above flow could be re-entered at token transfer. It is mainly for show to illustrate a logic outline. ::: diff --git a/docs/docs/misc/migration_notes.md b/docs/docs/misc/migration_notes.md index a222b39ad88..fa49a944eb3 100644 --- a/docs/docs/misc/migration_notes.md +++ b/docs/docs/misc/migration_notes.md @@ -8,6 +8,16 @@ Aztec is in full-speed development. Literally every version breaks compatibility ## 0.25.0 +### [Aztec.nr] Static calls + +It is now possible to perform static calls from both public and private functions. Static calls forbid any modification to the state, including L2->L1 messages or log generation. Once a static context is set through a static all, every subsequent call will also be treated as static via context propagation. + +```rust +context.static_call_private_function(targetContractAddress, targetSelector, args); + +context.static_call_public_function(targetContractAddress, targetSelector, args); +``` + ### [Aztec.nr] Introduction to `prelude` A new `prelude` module to include common Aztec modules and types. diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr index 61746bdd239..30c6d140273 100644 --- a/noir-projects/aztec-nr/aztec/src/context/private_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/private_context.nr @@ -328,6 +328,7 @@ impl PrivateContext { is_static_call: bool, is_delegate_call: bool ) -> [Field; RETURN_VALUES_LENGTH] { + let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; let item = call_private_function_internal( contract_address, function_selector, @@ -434,6 +435,7 @@ impl PrivateContext { is_static_call: bool, is_delegate_call: bool ) { + let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call; let fields = enqueue_public_function_call_internal( contract_address, function_selector, diff --git a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr index 056ba803ec7..dccf19cd04c 100644 --- a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr @@ -149,6 +149,39 @@ contract Parent { return_values[0] } + #[aztec(private)] + fn privateCall( + targetContract: AztecAddress, + targetSelector: FunctionSelector, + args: [Field; 2] + ) -> Field { + // Call the target private function + let return_values = context.call_private_function(targetContract, targetSelector, args); + + // Copy the return value from the call to this function's return values + return_values[0] + } + + // Private function to set a static context and verify correct propagation for nested private calls + #[aztec(private)] + fn privateStaticCallNested( + targetContract: AztecAddress, + targetSelector: FunctionSelector, + args: [Field; 2] + ) -> Field { + // Call the target private function statically + let privateCallSelector = FunctionSelector::from_signature("privateCall((Field),(u32),[Field;2])"); + let thisAddress = context.this_address(); + let return_values = context.static_call_private_function( + thisAddress, + privateCallSelector, + [targetContract.to_field(), targetSelector.to_field(), args[0], args[1]] + ); + + // Copy the return value from the call to this function's return values + return_values[0] + } + // Public function to directly call another public function to the targetContract using the selector and value provided #[aztec(public)] fn publicStaticCall( @@ -161,6 +194,41 @@ contract Parent { return_values[0] } + // Public function to set a static context and verify correct propagation for nested public calls + #[aztec(public)] + fn publicNestedStaticCall( + targetContract: AztecAddress, + targetSelector: FunctionSelector, + args: [Field; 1] + ) -> Field { + // Call the target public function through the pub entrypoint statically + let pubEntryPointSelector = FunctionSelector::from_signature("pubEntryPoint((Field),(u32),Field)"); + let thisAddress = context.this_address(); + let return_values = context.static_call_public_function( + thisAddress, + pubEntryPointSelector, + [targetContract.to_field(), targetSelector.to_field(), args[0]] + ); + return_values[0] + } + + // Private function to enqueue a static call to the pubEntryPoint function of another contract, passing the target arguments provided + #[aztec(private)] + fn enqueueStaticNestedCallToPubFunction( + targetContract: AztecAddress, + targetSelector: FunctionSelector, + args: [Field; 1] + ) { + // Call the target public function through the pub entrypoint statically + let pubEntryPointSelector = FunctionSelector::from_signature("pubEntryPoint((Field),(u32),Field)"); + let thisAddress = context.this_address(); + context.static_call_public_function( + thisAddress, + pubEntryPointSelector, + [targetContract.to_field(), targetSelector.to_field(), args[0]] + ); + } + // Private function to enqueue a static call to the pubEntryPoint function of another contract, passing the target arguments provided #[aztec(private)] fn enqueueStaticCallToPubFunction( diff --git a/yarn-project/end-to-end/src/e2e_static_calls.test.ts b/yarn-project/end-to-end/src/e2e_static_calls.test.ts index 1d635f51f8d..974d3ce3f45 100644 --- a/yarn-project/end-to-end/src/e2e_static_calls.test.ts +++ b/yarn-project/end-to-end/src/e2e_static_calls.test.ts @@ -31,16 +31,40 @@ describe('e2e_static_calls', () => { .wait(); }, 100_000); + it('performs legal (nested) private to private static calls', async () => { + await parentContract.methods + .privateStaticCallNested(childContract.address, childContract.methods.privateGetValue.selector, [ + 42n, + wallet.getCompleteAddress().address, + ]) + .send() + .wait(); + }, 100_000); + it('performs legal public to public static calls', async () => { await parentContract.methods - .enqueueStaticCallToPubFunction(childContract.address, childContract.methods.pubGetValue.selector, [42n]) + .publicStaticCall(childContract.address, childContract.methods.pubGetValue.selector, [42n]) + .send() + .wait(); + }, 100_000); + + it('performs legal (nested) public to public static calls', async () => { + await parentContract.methods + .publicNestedStaticCall(childContract.address, childContract.methods.pubGetValue.selector, [42n]) .send() .wait(); }, 100_000); it('performs legal enqueued public static calls', async () => { await parentContract.methods - .publicStaticCall(childContract.address, childContract.methods.pubGetValue.selector, [42n]) + .enqueueStaticCallToPubFunction(childContract.address, childContract.methods.pubGetValue.selector, [42n]) + .send() + .wait(); + }, 100_000); + + it('performs legal (nested) enqueued public static calls', async () => { + await parentContract.methods + .enqueueStaticNestedCallToPubFunction(childContract.address, childContract.methods.pubGetValue.selector, [42n]) .send() .wait(); }, 100_000); @@ -57,6 +81,18 @@ describe('e2e_static_calls', () => { ).rejects.toThrow('Static call cannot create new notes, emit L2->L1 messages or generate logs'); }, 100_000); + it('fails when performing illegal (nested) private to private static calls', async () => { + await expect( + parentContract.methods + .privateStaticCallNested(childContract.address, childContract.methods.privateSetValue.selector, [ + 42n, + wallet.getCompleteAddress().address, + ]) + .send() + .wait(), + ).rejects.toThrow('Static call cannot create new notes, emit L2->L1 messages or generate logs'); + }, 100_000); + it('fails when performing illegal public to public static calls', async () => { await expect( parentContract.methods @@ -66,6 +102,15 @@ describe('e2e_static_calls', () => { ).rejects.toThrow('Static call cannot update the state, emit L2->L1 messages or generate logs'); }, 100_000); + it('fails when performing illegal (nested) public to public static calls', async () => { + await expect( + parentContract.methods + .publicNestedStaticCall(childContract.address, childContract.methods.pubSetValue.selector, [42n]) + .send() + .wait(), + ).rejects.toThrow('Static call cannot update the state, emit L2->L1 messages or generate logs'); + }, 100_000); + it('fails when performing illegal enqueued public static calls', async () => { await expect( parentContract.methods @@ -74,5 +119,16 @@ describe('e2e_static_calls', () => { .wait(), ).rejects.toThrow('Static call cannot update the state, emit L2->L1 messages or generate logs'); }, 100_000); + + it('fails when performing illegal (nested) enqueued public static calls', async () => { + await expect( + parentContract.methods + .enqueueStaticNestedCallToPubFunction(childContract.address, childContract.methods.pubSetValue.selector, [ + 42n, + ]) + .send() + .wait(), + ).rejects.toThrow('Static call cannot update the state, emit L2->L1 messages or generate logs'); + }, 100_000); }); }); diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index ebeb17c3936..cc756380c20 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -343,6 +343,8 @@ export class ClientExecutionContext extends ViewDataOracle { `Calling private function ${this.contractAddress}:${functionSelector} from ${this.callContext.storageContractAddress}`, ); + isStaticCall = isStaticCall || this.callContext.isStaticCall; + const targetArtifact = await this.db.getFunctionArtifact(targetContractAddress, functionSelector); const targetFunctionData = FunctionData.fromAbi(targetArtifact); @@ -412,6 +414,8 @@ export class ClientExecutionContext extends ViewDataOracle { isStaticCall: boolean, isDelegateCall: boolean, ): Promise { + isStaticCall = isStaticCall || this.callContext.isStaticCall; + const targetArtifact = await this.db.getFunctionArtifact(targetContractAddress, functionSelector); const derivedCallContext = await this.deriveCallContext( targetContractAddress, diff --git a/yarn-project/simulator/src/public/public_execution_context.ts b/yarn-project/simulator/src/public/public_execution_context.ts index 70a8f6ceafd..3b53fd2490b 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -163,6 +163,8 @@ export class PublicExecutionContext extends TypedOracle { isStaticCall: boolean, isDelegateCall: boolean, ) { + isStaticCall = isStaticCall || this.execution.callContext.isStaticCall; + const args = this.packedArgsCache.unpack(argsHash); this.log(`Public function call: addr=${targetContractAddress} selector=${functionSelector} args=${args.join(',')}`); diff --git a/yellow-paper/docs/calls/static-calls.md b/yellow-paper/docs/calls/static-calls.md index cb1b2c789c6..8df2dd7c87a 100644 --- a/yellow-paper/docs/calls/static-calls.md +++ b/yellow-paper/docs/calls/static-calls.md @@ -5,13 +5,6 @@ In particular, the following fields of the returned `CallStackItem` must be zero or empty in a static call: - - `new_note_hashes` - `new_nullifiers` @@ -22,6 +15,8 @@ Thoughts? Ethereum does the latter. We should write about whichever we choose in - `encrypted_log_preimages_length` - `unencrypted_log_preimages_length` +From the moment a static call is made, every subsequent nested call is forced to be static by setting a flag in the derived `CallContext`, which propagates through the call stack. + At the protocol level, a static call is identified by a `is_static_call` flag in the `CircuitPublicInputs` of the `CallStackItem`. The kernel is responsible for asserting that the call and all nested calls do not emit any forbidden side effects. At the contract level, a caller can initiate a static call via a `staticCallPrivateFunction` or `staticCallPublicFunction` oracle call. The caller is responsible for asserting that the returned `CallStackItem` has the `is_static_call` flag correctly set.