From 4725f75a2d5c4db9c68c40de4355a901368ee14e Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 26 Sep 2024 09:00:53 +0000 Subject: [PATCH 1/4] the return of ec add unsafe --- .../dsl/acir_format/ec_operations.cpp | 41 ++++++++++++++++++- .../dsl/acir_format/witness_constant.cpp | 11 ++++- .../dsl/acir_format/witness_constant.hpp | 8 ++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp index 0da820c8d258..8255074daf6c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp @@ -7,6 +7,11 @@ namespace acir_format { +// This functions assumes that: +// 1. none of the points are infinity +// 2. the points are on the curve +// 3a. the points have not the same abssissa, OR +// 3b. the points are identical (same witness index or same value) template void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_valid_witness_assignments) { @@ -17,9 +22,39 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val input.input1_x, input.input1_y, input.input1_infinite, has_valid_witness_assignments, builder); auto input2_point = to_grumpkin_point( input.input2_x, input.input2_y, input.input2_infinite, has_valid_witness_assignments, builder); + // Runtime checks that the inputs are not the point at infinity, as assumed by the function. + ASSERT(get_value(input.input1_infinite, builder) == 0); + ASSERT(get_value(input.input2_infinite, builder) == 0); // Addition - cycle_group_ct result = input1_point + input2_point; + // Check if operands are the same + bool x_match = false; + if (!input1_point.x.is_constant() && !input2_point.x.is_constant()) { + x_match = (input1_point.x.get_witness_index() == input2_point.x.get_witness_index()); + } else { + if (input1_point.x.is_constant() && input2_point.x.is_constant()) { + x_match = (input1_point.x.get_value() == input2_point.x.get_value()); + } + } + bool y_match = false; + if (!input1_point.y.is_constant() && !input2_point.y.is_constant()) { + y_match = (input1_point.y.get_witness_index() == input2_point.y.get_witness_index()); + } else { + if (input1_point.y.is_constant() && input2_point.y.is_constant()) { + y_match = (input1_point.y.get_value() == input2_point.y.get_value()); + } + } + + cycle_group_ct result; + // If operands are the same, we double. + if (x_match && y_match) { + result = input1_point.dbl(); + } else { + // Runtime checks that the inputs have not the same x coordinate, as assumed by the function. + ASSERT(input1_point.x.get_value() != input2_point.x.get_value()); + result = input1_point.unconditional_add(input2_point); + } + cycle_group_ct standard_result = result.get_standard_form(); auto x_normalized = standard_result.x.normalize(); auto y_normalized = standard_result.y.normalize(); @@ -35,6 +70,10 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val } else { builder.assert_equal(y_normalized.witness_index, input.result_y); } + // Runtime check that the result is not the point at infinity, as assumed by the function. + ASSERT(infinite.get_value() == 0); + // TODO: remove the infinite result, because the function should always return a non-zero point. + // But this requires an ACIR serialisation change and it will be done in a subsequent PR. if (infinite.is_constant()) { builder.fix_witness(input.result_infinite, infinite.get_value()); } else { diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.cpp index d199eeb62f8b..fbf9588aaf99 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.cpp @@ -12,10 +12,17 @@ bb::stdlib::cycle_group to_grumpkin_point(const WitnessOrConstant& bool has_valid_witness_assignments, Builder& builder) { - using bool_ct = bb::stdlib::bool_t; auto point_x = to_field_ct(input_x, builder); auto point_y = to_field_ct(input_y, builder); - auto infinite = bool_ct(to_field_ct(input_infinite, builder)); + // We assume input_infinite is boolean, so we do not add a boolean gate + bool_t infinite(&builder); + if (!input_infinite.is_constant) { + infinite.witness_index = input_infinite.index; + infinite.witness_bool = get_value(input_infinite, builder) == FF::one(); + } else { + infinite.witness_index = IS_CONSTANT; + infinite.witness_bool = input_infinite.value == FF::one(); + } // When we do not have the witness assignments, we set is_infinite value to true if it is not constant // else default values would give a point which is not on the curve and this will fail verification diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.hpp index bb60a754d591..2be25275c858 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/witness_constant.hpp @@ -39,4 +39,12 @@ bb::stdlib::cycle_group to_grumpkin_point(const WitnessOrConstant& bool has_valid_witness_assignments, Builder& builder); +template FF get_value(const WitnessOrConstant& input, Builder& builder) +{ + if (input.is_constant) { + return input.value; + } + return builder.variables[input.index]; +} + } // namespace acir_format \ No newline at end of file From 2c35a92d58174af84c34eae94b00e7f0d560f8a3 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 26 Sep 2024 12:46:16 +0000 Subject: [PATCH 2/4] fix test case --- .../dsl/acir_format/ec_operations.test.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp index f12c39e8f8f5..73cf0aadb6b1 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp @@ -24,19 +24,18 @@ class EcOperations : public ::testing::Test { size_t generate_ec_add_constraint(EcAdd& ec_add_constraint, WitnessVector& witness_values) { - using cycle_group_ct = bb::stdlib::cycle_group; witness_values.push_back(0); auto g1 = grumpkin::g1::affine_one; - cycle_group_ct input_point(g1); - // Doubling - cycle_group_ct result = input_point.dbl(); + auto g2 = g1 + g1; + auto affine_result = g1 + g2; + // add: x,y,x2,y2 witness_values.push_back(g1.x); witness_values.push_back(g1.y); - witness_values.push_back(g1.x); - witness_values.push_back(g1.y); - witness_values.push_back(result.x.get_value()); - witness_values.push_back(result.y.get_value()); + witness_values.push_back(g2.x); + witness_values.push_back(g2.y); + witness_values.push_back(affine_result.x); + witness_values.push_back(affine_result.y); witness_values.push_back(fr(0)); witness_values.push_back(fr(0)); ec_add_constraint = EcAdd{ From 32de43a78e8dd2ee66a709d3dd47aec160885c6a Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 1 Oct 2024 16:34:50 +0000 Subject: [PATCH 3/4] force CI to run e2e --- yarn-project/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/yarn-project/README.md b/yarn-project/README.md index 9c4c50108120..600b7802b456 100644 --- a/yarn-project/README.md +++ b/yarn-project/README.md @@ -72,7 +72,7 @@ You may also need to modify the [Dockerfile](yarn-project/yarn-project-base/Dock `deploy-npm` script handles the releases of npm packages within yarn-project. But the initial release is a manual process: 1. Ensure relevant folders are copied in by docker in `yarn-project/yarn-project-base/Dockerfile` and `yarn-project/Dockerfile` -2. SSH into the CI +2. SSH into the CI. 3. Run the following: ```sh cd project @@ -86,6 +86,6 @@ COMMIT_TAG= - Extract `VERSION` as the script shows (in the eg it should be 0.8.8) - Skip the version existing checks like `if [ "$VERSION" == "$PUBLISHED_VERSION" ]` and `if [ "$VERSION" != "$HIGHER_VERSION" ]`. Since this is our first time deploying the package, `PUBLISHED_VERSION` and `HIGHER_VERSION` will be empty and hence these checks would fail. These checks are necessary in the CI for continual releases. - Locally update the package version in package.json using `jq` as shown in the script. - - Do a dry-run + - Do a dry-run - If dry run succeeds, publish the package! 5. Create a PR by adding your package into the `deploy-npm` script so next release onwards, CI can cut releases for your package. From 3c60dcbc3bcd69efad37c604242baf1eceb83d25 Mon Sep 17 00:00:00 2001 From: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:13:31 +0100 Subject: [PATCH 4/4] chore: remove unused header in public executor (#8990) --- .../src/avm/avm_execution_environment.ts | 4 +--- .../simulator/src/avm/avm_simulator.test.ts | 1 - yarn-project/simulator/src/avm/fixtures/index.ts | 3 +-- yarn-project/simulator/src/public/executor.ts | 13 +++---------- .../simulator/src/public/public_processor.ts | 2 +- yarn-project/txe/src/oracle/txe_oracle.ts | 15 +++------------ 6 files changed, 9 insertions(+), 29 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_execution_environment.ts b/yarn-project/simulator/src/avm/avm_execution_environment.ts index 9e067710bdad..36592fd7a036 100644 --- a/yarn-project/simulator/src/avm/avm_execution_environment.ts +++ b/yarn-project/simulator/src/avm/avm_execution_environment.ts @@ -1,4 +1,4 @@ -import { FunctionSelector, type GlobalVariables, type Header } from '@aztec/circuits.js'; +import { FunctionSelector, type GlobalVariables } from '@aztec/circuits.js'; import { type AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr } from '@aztec/foundation/fields'; @@ -14,7 +14,6 @@ export class AvmExecutionEnvironment { public readonly functionSelector: FunctionSelector, // may be temporary (#7224) public readonly contractCallDepth: Fr, public readonly transactionFee: Fr, - public readonly header: Header, public readonly globals: GlobalVariables, public readonly isStaticCall: boolean, public readonly isDelegateCall: boolean, @@ -35,7 +34,6 @@ export class AvmExecutionEnvironment { functionSelector, this.contractCallDepth.add(Fr.ONE), this.transactionFee, - this.header, this.globals, isStaticCall, isDelegateCall, diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index a5dac0b30356..03078a1b713a 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -819,7 +819,6 @@ describe('AVM simulator: transpiled Noir contracts', () => { /*nestedEnvironment=*/ expect.objectContaining({ sender: environment.address, // sender is top-level call contractCallDepth: new Fr(1), // top call is depth 0, nested is depth 1 - header: environment.header, // just confirming that nested env looks roughly right globals: environment.globals, // just confirming that nested env looks roughly right isStaticCall: isStaticCall, // TODO(7121): can't check calldata like this since it is modified on environment construction diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index 9d8e2183322d..084767f03170 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -1,5 +1,5 @@ import { isNoirCallStackUnresolved } from '@aztec/circuit-types'; -import { GasFees, GlobalVariables, Header } from '@aztec/circuits.js'; +import { GasFees, GlobalVariables } from '@aztec/circuits.js'; import { FunctionSelector, getFunctionDebugMetadata } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { EthAddress } from '@aztec/foundation/eth-address'; @@ -63,7 +63,6 @@ export function initExecutionEnvironment(overrides?: Partial n.value), ); - const avmExecutionEnv = createAvmExecutionEnvironment( - executionRequest, - this.header, - globalVariables, - transactionFee, - ); + const avmExecutionEnv = createAvmExecutionEnvironment(executionRequest, globalVariables, transactionFee); const avmMachineState = new AvmMachineState(availableGas); const avmContext = new AvmContext(avmPersistableState, avmExecutionEnv, avmMachineState); @@ -120,7 +115,6 @@ export class PublicExecutor { */ function createAvmExecutionEnvironment( executionRequest: PublicExecutionRequest, - header: Header, globalVariables: GlobalVariables, transactionFee: Fr, ): AvmExecutionEnvironment { @@ -131,7 +125,6 @@ function createAvmExecutionEnvironment( executionRequest.callContext.functionSelector, /*contractCallDepth=*/ Fr.zero(), transactionFee, - header, globalVariables, executionRequest.callContext.isStaticCall, executionRequest.callContext.isDelegateCall, diff --git a/yarn-project/simulator/src/public/public_processor.ts b/yarn-project/simulator/src/public/public_processor.ts index 7739c0a501de..486747c7f925 100644 --- a/yarn-project/simulator/src/public/public_processor.ts +++ b/yarn-project/simulator/src/public/public_processor.ts @@ -57,7 +57,7 @@ export class PublicProcessorFactory { const historicalHeader = maybeHistoricalHeader ?? merkleTree.getInitialHeader(); const worldStateDB = new WorldStateDB(merkleTree, this.contractDataSource); - const publicExecutor = new PublicExecutor(worldStateDB, historicalHeader, telemetryClient); + const publicExecutor = new PublicExecutor(worldStateDB, telemetryClient); const publicKernelSimulator = new RealPublicKernelCircuitSimulator(this.simulator); return PublicProcessor.create( diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 6bd3aef063ed..32791a806b88 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -679,24 +679,14 @@ export class TXE implements TypedOracle { return `${artifact.name}:${f.name}`; } - async executePublicFunction( - targetContractAddress: AztecAddress, - args: Fr[], - callContext: CallContext, - counter: number, - ) { - const header = Header.empty(); - header.state = await this.trees.getStateReference(true); - header.globalVariables.blockNumber = new Fr(await this.getBlockNumber()); - + executePublicFunction(targetContractAddress: AztecAddress, args: Fr[], callContext: CallContext, counter: number) { const executor = new PublicExecutor( new TXEWorldStateDB(this.trees.asLatest(), new TXEPublicContractDataSource(this)), - header, new NoopTelemetryClient(), ); const execution = new PublicExecutionRequest(targetContractAddress, callContext, args); - return executor.simulate( + const executionResult = executor.simulate( execution, GlobalVariables.empty(), Gas.test(), @@ -705,6 +695,7 @@ export class TXE implements TypedOracle { /* transactionFee */ Fr.ONE, counter, ); + return Promise.resolve(executionResult); } async avmOpcodeCall(