Skip to content

Commit

Permalink
fix: CombinedConstantData not registered for serialization (#6292)
Browse files Browse the repository at this point in the history
Fixes:
<img width="634" alt="image"
src="https://github.com/AztecProtocol/aztec-packages/assets/13470840/e4707b61-1e26-4b3c-b880-9f5b1bf85e4e">

I decided to not register `CombinedConstantData` directly on JsonRpc
server and client since that would just make it all much less readable
because CombinedConstantData is not a return value itself on any of
AztecNode methods. And since I am a fan of nice readable encapsulated
classes instead of the `Pick` type typescript freestyle I refactored
ProcessOutput such that we can register that directly on the Json RPC
server.
  • Loading branch information
benesjan authored May 9, 2024
1 parent ef9cdde commit 89ab8ee
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 67 deletions.
10 changes: 9 additions & 1 deletion yarn-project/aztec-node/src/aztec-node/http_rpc_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
LogId,
NullifierMembershipWitness,
PublicDataWitness,
PublicSimulationOutput,
SiblingPath,
Tx,
TxEffect,
Expand Down Expand Up @@ -41,7 +42,14 @@ export function createAztecNodeRpcServer(node: AztecNode) {
PublicDataWitness,
SiblingPath,
},
{ Tx, TxReceipt, EncryptedL2BlockL2Logs, UnencryptedL2BlockL2Logs, NullifierMembershipWitness },
{
PublicSimulationOutput,
Tx,
TxReceipt,
EncryptedL2BlockL2Logs,
UnencryptedL2BlockL2Logs,
NullifierMembershipWitness,
},
// disable methods not part of the AztecNode interface
['start', 'stop'],
);
Expand Down
22 changes: 11 additions & 11 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import {
LogType,
MerkleTreeId,
NullifierMembershipWitness,
type ProcessOutput,
type ProverClient,
type ProverConfig,
PublicDataWitness,
PublicSimulationOutput,
type SequencerConfig,
type SiblingPath,
type Tx,
Expand Down Expand Up @@ -634,7 +634,7 @@ export class AztecNodeService implements AztecNode {
* Simulates the public part of a transaction with the current state.
* @param tx - The transaction to simulate.
**/
public async simulatePublicCalls(tx: Tx): Promise<ProcessOutput> {
public async simulatePublicCalls(tx: Tx): Promise<PublicSimulationOutput> {
this.log.info(`Simulating tx ${tx.getTxHash()}`);
const blockNumber = (await this.blockSource.getBlockNumber()) + 1;

Expand Down Expand Up @@ -674,15 +674,15 @@ export class AztecNodeService implements AztecNode {
}
this.log.debug(`Simulated tx ${tx.getTxHash()} succeeds`);
const [processedTx] = processedTxs;
return {
constants: processedTx.data.constants,
encryptedLogs: processedTx.encryptedLogs,
unencryptedLogs: processedTx.unencryptedLogs,
end: processedTx.data.end,
revertReason: processedTx.revertReason,
publicReturnValues: returns[0],
gasUsed: processedTx.gasUsed,
};
return new PublicSimulationOutput(
processedTx.encryptedLogs,
processedTx.unencryptedLogs,
processedTx.revertReason,
processedTx.data.constants,
processedTx.data.end,
returns[0],
processedTx.gasUsed,
);
}

public async setConfig(config: Partial<SequencerConfig & ProverConfig>): Promise<void> {
Expand Down
13 changes: 11 additions & 2 deletions yarn-project/circuit-types/src/aztec_node/rpc/aztec_node_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import { type AztecNode } from '../../interfaces/aztec-node.js';
import { NullifierMembershipWitness } from '../../interfaces/nullifier_tree.js';
import { L2Block } from '../../l2_block.js';
import { EncryptedL2BlockL2Logs, ExtendedUnencryptedL2Log, LogId, UnencryptedL2BlockL2Logs } from '../../logs/index.js';
import { PublicDataWitness } from '../../public_data_witness.js';
import { SiblingPath } from '../../sibling_path/index.js';
import { Tx, TxHash, TxReceipt } from '../../tx/index.js';
import { PublicSimulationOutput, Tx, TxHash, TxReceipt } from '../../tx/index.js';
import { TxEffect } from '../../tx_effect.js';

/**
Expand All @@ -34,9 +35,17 @@ export function createAztecNodeClient(url: string, fetch = defaultFetch): AztecN
TxEffect,
LogId,
TxHash,
PublicDataWitness,
SiblingPath,
},
{ Tx, TxReceipt, EncryptedL2BlockL2Logs, UnencryptedL2BlockL2Logs, NullifierMembershipWitness },
{
PublicSimulationOutput,
Tx,
TxReceipt,
EncryptedL2BlockL2Logs,
UnencryptedL2BlockL2Logs,
NullifierMembershipWitness,
},
false,
'node',
fetch,
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/circuit-types/src/interfaces/aztec-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { type MerkleTreeId } from '../merkle_tree_id.js';
import { type PublicDataWitness } from '../public_data_witness.js';
import { type SiblingPath } from '../sibling_path/index.js';
import { type ProcessOutput, type Tx, type TxHash, type TxReceipt } from '../tx/index.js';
import { type PublicSimulationOutput, 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 +283,7 @@ export interface AztecNode {
* This currently just checks that the transaction execution succeeds.
* @param tx - The transaction to simulate.
**/
simulatePublicCalls(tx: Tx): Promise<ProcessOutput>;
simulatePublicCalls(tx: Tx): Promise<PublicSimulationOutput>;

/**
* Updates the configuration of this node.
Expand Down
20 changes: 10 additions & 10 deletions yarn-project/circuit-types/src/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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 { type ProcessOutput, type ProcessReturnValues, SimulatedTx, Tx, TxHash } from './tx/index.js';
import { type ProcessReturnValues, PublicSimulationOutput, SimulatedTx, Tx, TxHash } from './tx/index.js';

/**
* Testing utility to create empty logs composed from a single empty log.
Expand Down Expand Up @@ -129,15 +129,15 @@ export const mockTxForRollup = (seed = 1, { hasLogs = false }: { hasLogs?: boole
export const mockSimulatedTx = (seed = 1, hasLogs = true) => {
const tx = mockTx(seed, { hasLogs });
const dec: ProcessReturnValues = [new Fr(1n), new Fr(2n), new Fr(3n), new Fr(4n)];
const output: ProcessOutput = {
constants: makeCombinedConstantData(),
encryptedLogs: tx.encryptedLogs,
unencryptedLogs: tx.unencryptedLogs,
end: makeCombinedAccumulatedData(),
revertReason: undefined,
publicReturnValues: dec,
gasUsed: {},
};
const output = new PublicSimulationOutput(
tx.encryptedLogs,
tx.unencryptedLogs,
undefined,
makeCombinedConstantData(),
makeCombinedAccumulatedData(),
dec,
{},
);
return new SimulatedTx(tx, dec, output);
};

Expand Down
1 change: 1 addition & 0 deletions yarn-project/circuit-types/src/tx/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export * from './simulated_tx.js';
export * from './tx_hash.js';
export * from './tx_receipt.js';
export * from './processed_tx.js';
export * from './public_simulation_output.js';
export * from './tx_validator.js';
48 changes: 48 additions & 0 deletions yarn-project/circuit-types/src/tx/public_simulation_output.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { CombinedAccumulatedData, CombinedConstantData, Fr, Gas } from '@aztec/circuits.js';
import { mapValues } from '@aztec/foundation/collection';

import { EncryptedTxL2Logs, UnencryptedTxL2Logs } from '../logs/tx_l2_logs.js';
import { type SimulationError } from '../simulation_error.js';
import { type PublicKernelType } from './processed_tx.js';

/** Return values of simulating a circuit. */
export type ProcessReturnValues = Fr[] | undefined;

/**
* Outputs of processing the public component of a transaction.
*/
export class PublicSimulationOutput {
constructor(
public encryptedLogs: EncryptedTxL2Logs,
public unencryptedLogs: UnencryptedTxL2Logs,
public revertReason: SimulationError | undefined,
public constants: CombinedConstantData,
public end: CombinedAccumulatedData,
public publicReturnValues: ProcessReturnValues,
public gasUsed: Partial<Record<PublicKernelType, Gas>>,
) {}

toJSON() {
return {
encryptedLogs: this.encryptedLogs.toJSON(),
unencryptedLogs: this.unencryptedLogs.toJSON(),
revertReason: this.revertReason,
constants: this.constants.toBuffer().toString('hex'),
end: this.end.toBuffer().toString('hex'),
publicReturnValues: this.publicReturnValues?.map(fr => fr.toString()),
gasUsed: mapValues(this.gasUsed, gas => gas?.toJSON()),
};
}

static fromJSON(json: any): PublicSimulationOutput {
return new PublicSimulationOutput(
EncryptedTxL2Logs.fromJSON(json.encryptedLogs),
UnencryptedTxL2Logs.fromJSON(json.unencryptedLogs),
json.revertReason,
CombinedConstantData.fromBuffer(Buffer.from(json.constants, 'hex')),
CombinedAccumulatedData.fromBuffer(Buffer.from(json.end, 'hex')),
json.publicReturnValues?.map(Fr.fromString),
mapValues(json.gasUsed, gas => (gas ? Gas.fromJSON(gas) : undefined)),
);
}
}
51 changes: 10 additions & 41 deletions yarn-project/circuit-types/src/tx/simulated_tx.ts
Original file line number Diff line number Diff line change
@@ -1,52 +1,21 @@
import { CombinedAccumulatedData, CombinedConstantData, Fr, Gas } from '@aztec/circuits.js';
import { mapValues } from '@aztec/foundation/collection';
import { Fr, Gas } from '@aztec/circuits.js';

import { EncryptedTxL2Logs, UnencryptedTxL2Logs } from '../logs/index.js';
import { type ProcessedTx, PublicKernelType } from './processed_tx.js';
import { PublicKernelType } from './processed_tx.js';
import { type ProcessReturnValues, PublicSimulationOutput } from './public_simulation_output.js';
import { Tx } from './tx.js';

/** Return values of simulating a circuit. */
export type ProcessReturnValues = Fr[] | undefined;

/**
* Outputs of processing the public component of a transaction.
* REFACTOR: Rename.
*/
export type ProcessOutput = Pick<ProcessedTx, 'encryptedLogs' | 'unencryptedLogs' | 'revertReason' | 'gasUsed'> &
Pick<ProcessedTx['data'], 'constants' | 'end'> & { publicReturnValues: ProcessReturnValues };

function processOutputToJSON(output: ProcessOutput) {
return {
encryptedLogs: output.encryptedLogs.toJSON(),
unencryptedLogs: output.unencryptedLogs.toJSON(),
revertReason: output.revertReason,
constants: output.constants.toBuffer().toString('hex'),
end: output.end.toBuffer().toString('hex'),
publicReturnValues: output.publicReturnValues?.map(fr => fr.toString()),
gasUsed: mapValues(output.gasUsed, gas => gas?.toJSON()),
};
}

function processOutputFromJSON(json: any): ProcessOutput {
return {
encryptedLogs: EncryptedTxL2Logs.fromJSON(json.encryptedLogs),
unencryptedLogs: UnencryptedTxL2Logs.fromJSON(json.unencryptedLogs),
revertReason: json.revertReason,
constants: CombinedConstantData.fromBuffer(Buffer.from(json.constants, 'hex')),
end: CombinedAccumulatedData.fromBuffer(Buffer.from(json.end, 'hex')),
publicReturnValues: json.publicReturnValues?.map(Fr.fromString),
gasUsed: mapValues(json.gasUsed, gas => (gas ? Gas.fromJSON(gas) : undefined)),
};
}

// REFACTOR: Review what we need to expose to the user when running a simulation.
// Eg tx already has encrypted and unencrypted logs, but those cover only the ones
// emitted during private. We need the ones from ProcessOutput to include the public
// ones as well. However, those would only be present if the user chooses to simulate
// the public side of things. This also points at this class needing to be split into
// two: one with just private simulation, and one that also includes public simulation.
export class SimulatedTx {
constructor(public tx: Tx, public privateReturnValues?: ProcessReturnValues, public publicOutput?: ProcessOutput) {}
constructor(
public tx: Tx,
public privateReturnValues?: ProcessReturnValues,
public publicOutput?: PublicSimulationOutput,
) {}

/**
* Returns suggested total and teardown gas limits for the simulated tx.
Expand Down Expand Up @@ -79,7 +48,7 @@ export class SimulatedTx {
return {
tx: this.tx.toJSON(),
privateReturnValues: this.privateReturnValues?.map(fr => fr.toString()),
publicOutput: this.publicOutput && processOutputToJSON(this.publicOutput),
publicOutput: this.publicOutput && this.publicOutput.toJSON(),
};
}

Expand All @@ -90,7 +59,7 @@ export class SimulatedTx {
*/
public static fromJSON(obj: any) {
const tx = Tx.fromJSON(obj.tx);
const publicOutput = obj.publicOutput ? processOutputFromJSON(obj.publicOutput) : undefined;
const publicOutput = obj.publicOutput ? PublicSimulationOutput.fromJSON(obj.publicOutput) : undefined;
const privateReturnValues = obj.privateReturnValues?.map(Fr.fromString);

return new SimulatedTx(tx, privateReturnValues, publicOutput);
Expand Down

0 comments on commit 89ab8ee

Please sign in to comment.