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: decoded return values #5762

Merged
merged 5 commits into from
Apr 17, 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 @@ -115,10 +115,13 @@ contract DocsExample {
// and returns the response.
// Used to test that we can retrieve values through calls and
// correctly return them in the simulation
context.call_private_function_no_args(
let mut leader: Leader = context.call_private_function_no_args(
context.this_address(),
FunctionSelector::from_signature("get_shared_immutable_constrained_private()")
).unpack_into()
).unpack_into();

leader.points += 1;
leader
}

#[aztec(public)]
Expand All @@ -127,17 +130,26 @@ contract DocsExample {
// and returns the response.
// Used to test that we can retrieve values through calls and
// correctly return them in the simulation
context.call_public_function_no_args(
let mut leader: Leader = context.call_public_function_no_args(
context.this_address(),
FunctionSelector::from_signature("get_shared_immutable_constrained_public()")
).deserialize_into()
).deserialize_into();

leader.points += 1;
leader
}

#[aztec(public)]
fn get_shared_immutable_constrained_public() -> pub Leader {
storage.shared_immutable.read_public()
}

#[aztec(public)]
fn get_shared_immutable_constrained_public_multiple() -> pub [Leader; 5] {
let a = storage.shared_immutable.read_public();
[a, a, a, a, a]
}

#[aztec(private)]
fn get_shared_immutable_constrained_private() -> pub Leader {
storage.shared_immutable.read_private()
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ export class AztecNodeService implements AztecNode {
throw reverted[0].revertReason;
}
this.log.info(`Simulated tx ${tx.getTxHash()} succeeds`);
return returns;
return returns[0];
}

public setConfig(config: Partial<SequencerConfig>): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { type FunctionCall, PackedValues, TxExecutionRequest } from '@aztec/circuit-types';
import { type AztecAddress, FunctionData, GasSettings, TxContext } from '@aztec/circuits.js';
import { type FunctionAbi, FunctionType, encodeArguments } from '@aztec/foundation/abi';
import { type FunctionAbi, FunctionType, decodeReturnValues, encodeArguments } from '@aztec/foundation/abi';

import { type Wallet } from '../account/wallet.js';
import { BaseContractInteraction, type SendMethodOptions } from './base_contract_interaction.js';
Expand Down Expand Up @@ -73,7 +73,6 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
* 2. It supports `unconstrained`, `private` and `public` functions
* 3. For `private` execution it:
* 3.a SKIPS the entrypoint and starts directly at the function
* 3.b SKIPS public execution entirely
* 4. For `public` execution it:
* 4.a Removes the `txRequest` value after ended simulation
* 4.b Ignores the `from` in the options
Expand All @@ -86,10 +85,6 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
return this.wallet.viewTx(this.functionDao.name, this.args, this.contractAddress, options.from);
}

// TODO: If not unconstrained, we return a size 4 array of fields.
// TODO: It should instead return the correctly decoded value
// TODO: The return type here needs to be fixed! @LHerskind

if (this.functionDao.functionType == FunctionType.SECRET) {
const nodeInfo = await this.wallet.getNodeInfo();
const packedArgs = PackedValues.fromValues(encodeArguments(this.functionDao, this.args));
Expand All @@ -103,13 +98,15 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
authWitnesses: [],
gasSettings: options.gasSettings ?? GasSettings.simulation(),
});
const simulatedTx = await this.pxe.simulateTx(txRequest, false, options.from ?? this.wallet.getAddress());
return simulatedTx.privateReturnValues?.[0];
const simulatedTx = await this.pxe.simulateTx(txRequest, true, options.from ?? this.wallet.getAddress());
const flattened = simulatedTx.privateReturnValues;
return flattened ? decodeReturnValues(this.functionDao, flattened) : [];
} else {
const txRequest = await this.create();
const simulatedTx = await this.pxe.simulateTx(txRequest, true);
this.txRequest = undefined;
return simulatedTx.publicReturnValues?.[0];
const flattened = simulatedTx.publicReturnValues;
return flattened ? decodeReturnValues(this.functionDao, flattened) : [];
}
}
}
5 changes: 2 additions & 3 deletions yarn-project/circuit-types/src/interfaces/aztec-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
type PUBLIC_DATA_TREE_HEIGHT,
} from '@aztec/circuits.js';
import { type L1ContractAddresses } from '@aztec/ethereum';
import { type ProcessReturnValues } from '@aztec/foundation/abi';
import { type AztecAddress } from '@aztec/foundation/aztec-address';
import { type Fr } from '@aztec/foundation/fields';
import { type ContractClassPublic, type ContractInstanceWithAddress } from '@aztec/types/contracts';
Expand All @@ -22,7 +21,7 @@ import {
} from '../logs/index.js';
import { type MerkleTreeId } from '../merkle_tree_id.js';
import { type SiblingPath } from '../sibling_path/index.js';
import { type Tx, type TxHash, type TxReceipt } from '../tx/index.js';
import { type ProcessReturnValues, type Tx, type TxHash, type TxReceipt } from '../tx/index.js';
import { type TxEffect } from '../tx_effect.js';
import { type SequencerConfig } from './configs.js';
import { type L2BlockNumber } from './l2_block_number.js';
Expand Down Expand Up @@ -283,7 +282,7 @@ export interface AztecNode {
* This currently just checks that the transaction execution succeeds.
* @param tx - The transaction to simulate.
**/
simulatePublicCalls(tx: Tx): Promise<ProcessReturnValues[]>;
simulatePublicCalls(tx: Tx): Promise<ProcessReturnValues>;

/**
* Updates the configuration of this node.
Expand Down
6 changes: 3 additions & 3 deletions yarn-project/circuit-types/src/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
getContractClassFromArtifact,
} from '@aztec/circuits.js';
import { makePublicCallRequest } from '@aztec/circuits.js/testing';
import { type ContractArtifact, type DecodedReturn } from '@aztec/foundation/abi';
import { type ContractArtifact } from '@aztec/foundation/abi';
import { makeTuple } from '@aztec/foundation/array';
import { times } from '@aztec/foundation/collection';
import { randomBytes } from '@aztec/foundation/crypto';
Expand All @@ -21,7 +21,7 @@ import { type ContractInstanceWithAddress, SerializableContractInstance } from '
import { EncryptedL2Log } from './logs/encrypted_l2_log.js';
import { EncryptedFunctionL2Logs, EncryptedTxL2Logs, Note, UnencryptedTxL2Logs } from './logs/index.js';
import { ExtendedNote } from './notes/index.js';
import { SimulatedTx, Tx, TxHash } from './tx/index.js';
import { type ProcessReturnValues, SimulatedTx, Tx, TxHash } from './tx/index.js';

/**
* Testing utility to create empty logs composed from a single empty log.
Expand Down Expand Up @@ -94,7 +94,7 @@ export const mockTxForRollup = (seed = 1, { hasLogs = false }: { hasLogs?: boole

export const mockSimulatedTx = (seed = 1, hasLogs = true) => {
const tx = mockTx(seed, { hasLogs });
const dec: DecodedReturn = [1n, 2n, 3n, 4n];
const dec: ProcessReturnValues = [new Fr(1n), new Fr(2n), new Fr(3n), new Fr(4n)];
return new SimulatedTx(tx, dec, dec);
};

Expand Down
7 changes: 7 additions & 0 deletions yarn-project/circuit-types/src/tx/simulated_tx.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,11 @@ describe('simulated_tx', () => {
const simulatedTx = mockSimulatedTx();
expect(SimulatedTx.fromJSON(simulatedTx.toJSON())).toEqual(simulatedTx);
});

it('convert undefined effects to and from json', () => {
const simulatedTx = mockSimulatedTx();
simulatedTx.privateReturnValues = undefined;
simulatedTx.publicReturnValues = undefined;
expect(SimulatedTx.fromJSON(simulatedTx.toJSON())).toEqual(simulatedTx);
});
});
38 changes: 11 additions & 27 deletions yarn-project/circuit-types/src/tx/simulated_tx.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { AztecAddress } from '@aztec/circuits.js';
import { type ProcessReturnValues } from '@aztec/foundation/abi';
import { Fr } from '@aztec/circuits.js';

import { Tx } from './tx.js';

export type ProcessReturnValues = Fr[] | undefined;

export class SimulatedTx {
constructor(
public tx: Tx,
Expand All @@ -15,17 +16,11 @@ export class SimulatedTx {
* @returns A plain object with SimulatedTx properties.
*/
public toJSON() {
const returnToJson = (data: ProcessReturnValues): string => {
const replacer = (key: string, value: any): any => {
if (typeof value === 'bigint') {
return value.toString() + 'n'; // Indicate bigint with "n"
} else if (value instanceof AztecAddress) {
return value.toString();
} else {
return value;
}
};
return JSON.stringify(data, replacer);
const returnToJson = (data: ProcessReturnValues | undefined): string => {
if (data === undefined) {
return JSON.stringify(data);
}
return JSON.stringify(data.map(fr => fr.toString()));
};

return {
Expand All @@ -41,22 +36,11 @@ export class SimulatedTx {
* @returns A Tx class object.
*/
public static fromJSON(obj: any) {
const returnFromJson = (json: string): ProcessReturnValues => {
if (json == undefined) {
const returnFromJson = (json: string): ProcessReturnValues | undefined => {
if (json === undefined) {
return json;
}
const reviver = (key: string, value: any): any => {
if (typeof value === 'string') {
if (value.match(/\d+n$/)) {
// Detect bigint serialization
return BigInt(value.slice(0, -1));
} else if (value.match(/^0x[a-fA-F0-9]{64}$/)) {
return AztecAddress.fromString(value);
}
}
return value;
};
return JSON.parse(json, reviver);
return JSON.parse(json).map(Fr.fromString);
};

const tx = Tx.fromJSON(obj.tx);
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ describe('e2e_avm_simulator', () => {
}, 50_000);

it('Can execute ACVM function among AVM functions', async () => {
expect(await avmContract.methods.constant_field_acvm().simulate()).toEqual([123456n]);
expect(await avmContract.methods.constant_field_acvm().simulate()).toEqual(123456n);
});

it('Can call AVM function from ACVM', async () => {
expect(await avmContract.methods.call_avm_from_acvm().simulate()).toEqual([123456n]);
expect(await avmContract.methods.call_avm_from_acvm().simulate()).toEqual(123456n);
});

it('Can call ACVM function from AVM', async () => {
expect(await avmContract.methods.call_acvm_from_avm().simulate()).toEqual([123456n]);
expect(await avmContract.methods.call_acvm_from_avm().simulate()).toEqual(123456n);
});

it('AVM sees settled nullifiers by ACVM', async () => {
Expand Down Expand Up @@ -146,7 +146,7 @@ describe('e2e_avm_simulator', () => {

describe('Storage', () => {
it('Read immutable (initialized) storage (Field)', async () => {
expect(await avmContract.methods.read_storage_immutable().simulate()).toEqual([42n]);
expect(await avmContract.methods.read_storage_immutable().simulate()).toEqual(42n);
});
});
});
Expand Down
27 changes: 16 additions & 11 deletions yarn-project/end-to-end/src/e2e_state_vars.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,40 +38,45 @@ describe('e2e_state_vars', () => {
// checking the return values with:
// 1. A constrained private function that reads it directly
// 2. A constrained private function that calls another private function that reads.
// The indirect, adds 1 to the point to ensure that we are returning the correct value.

await contract.methods.initialize_shared_immutable(1).send().wait();

const a = await contract.methods.get_shared_immutable_constrained_private().simulate();
const b = await contract.methods.get_shared_immutable_constrained_private_indirect().simulate();
const c = await contract.methods.get_shared_immutable().simulate();

expect((a as any)[0]).toEqual((c as any)['account'].toBigInt());
expect((a as any)[1]).toEqual((c as any)['points']);
expect((b as any)[0]).toEqual((c as any)['account'].toBigInt());
expect((b as any)[1]).toEqual((c as any)['points']);

expect(a).toEqual(b);
expect(a).toEqual(c);
expect(b).toEqual({ account: c.account, points: c.points + 1n });
await contract.methods.match_shared_immutable(c.account, c.points).send().wait();
});

it('public read of SharedImmutable', async () => {
// Reads the value using an unconstrained function checking the return values with:
// 1. A constrained public function that reads it directly
// 2. A constrained public function that calls another public function that reads.
// The indirect, adds 1 to the point to ensure that we are returning the correct value.

const a = await contract.methods.get_shared_immutable_constrained_public().simulate();
const b = await contract.methods.get_shared_immutable_constrained_public_indirect().simulate();
const c = await contract.methods.get_shared_immutable().simulate();

expect((a as any)[0]).toEqual((c as any)['account'].toBigInt());
expect((a as any)[1]).toEqual((c as any)['points']);
expect((b as any)[0]).toEqual((c as any)['account'].toBigInt());
expect((b as any)[1]).toEqual((c as any)['points']);
expect(a).toEqual(c);
expect(b).toEqual({ account: c.account, points: c.points + 1n });

expect(a).toEqual(b);
await contract.methods.match_shared_immutable(c.account, c.points).send().wait();
});

it('public multiread of SharedImmutable', async () => {
// Reads the value using an unconstrained function checking the return values with:
// 1. A constrained public function that reads 5 times directly (going beyond the previous 4 Field return value)

const a = await contract.methods.get_shared_immutable_constrained_public_multiple().simulate();
const c = await contract.methods.get_shared_immutable().simulate();

expect(a).toEqual([c, c, c, c, c]);
});

it('initializing SharedImmutable the second time should fail', async () => {
// Jest executes the tests sequentially and the first call to initialize_shared_immutable was executed
// in the previous test, so the call bellow should fail.
Expand Down
7 changes: 3 additions & 4 deletions yarn-project/foundation/src/abi/decoder.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import { AztecAddress } from '../aztec-address/index.js';
import { type Fr } from '../fields/index.js';
import { type ABIParameter, type ABIVariable, type AbiType, type FunctionArtifact } from './abi.js';
import { type ABIParameter, type ABIVariable, type AbiType, type FunctionAbi } from './abi.js';
import { isAztecAddressStruct } from './utils.js';

/**
* The type of our decoded ABI.
*/
export type DecodedReturn = bigint | boolean | AztecAddress | DecodedReturn[] | { [key: string]: DecodedReturn };
export type ProcessReturnValues = (DecodedReturn | undefined)[] | undefined;

/**
* Decodes return values from a function call.
* Missing support for integer and string.
*/
class ReturnValuesDecoder {
constructor(private artifact: FunctionArtifact, private flattened: Fr[]) {}
constructor(private artifact: FunctionAbi, private flattened: Fr[]) {}

/**
* Decodes a single return value from field to the given type.
Expand Down Expand Up @@ -97,7 +96,7 @@ class ReturnValuesDecoder {
* @param returnValues - The decoded return values.
* @returns
*/
export function decodeReturnValues(abi: FunctionArtifact, returnValues: Fr[]) {
export function decodeReturnValues(abi: FunctionAbi, returnValues: Fr[]) {
return new ReturnValuesDecoder(abi, returnValues.slice()).decode();
}

Expand Down
5 changes: 2 additions & 3 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,7 @@ export class PXEService implements PXE {
}

if (simulatePublic) {
// Only one transaction, so we can take index 0.
simulatedTx.publicReturnValues = (await this.#simulatePublicCalls(simulatedTx.tx))[0];
simulatedTx.publicReturnValues = await this.#simulatePublicCalls(simulatedTx.tx);
}

if (!msgSender) {
Expand Down Expand Up @@ -646,7 +645,7 @@ export class PXEService implements PXE {
await this.patchPublicCallStackOrdering(publicInputs, enqueuedPublicFunctions);

const tx = new Tx(publicInputs, proof, encryptedLogs, unencryptedLogs, enqueuedPublicFunctions);
return new SimulatedTx(tx, [executionResult.returnValues]);
return new SimulatedTx(tx, executionResult.returnValues);
}

/**
Expand Down
5 changes: 2 additions & 3 deletions yarn-project/simulator/src/client/execution_result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
type PrivateCallStackItem,
type PublicCallRequest,
} from '@aztec/circuits.js';
import { type DecodedReturn } from '@aztec/foundation/abi';
import { type Fr } from '@aztec/foundation/fields';

import { type ACVMField } from '../acvm/index.js';
Expand Down Expand Up @@ -40,8 +39,8 @@ export interface ExecutionResult {
// Needed when we enable chained txs. The new notes can be cached and used in a later transaction.
/** The notes created in the executed function. */
newNotes: NoteAndSlot[];
/** The decoded return values of the executed function. */
returnValues: DecodedReturn;
/** The raw return values of the executed function. */
returnValues: Fr[];
/** The nested executions. */
nestedExecutions: this[];
/** Enqueued public function execution requests to be picked up by the sequencer. */
Expand Down
Loading
Loading