Skip to content

Commit

Permalink
feat: TXE fixes to avm opcodes and missing oracles, forced ci failure (
Browse files Browse the repository at this point in the history
…#7252)

Makes CI fail in case TXE test fail. Noticed they were broken, but
master was still green and removed some unnecessary commands in the
docker build.

Also reverted some of the changes from
#7237 since we need
the length of the data we're about to read in `avmOpcodeStorageRead`
(the oracle cannot obtain this information from the assigned output
variable.
  • Loading branch information
Thunkar authored Jul 2, 2024
1 parent 51d93eb commit de303e2
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 25 deletions.
2 changes: 1 addition & 1 deletion avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ fn handle_storage_read(
inputs: &Vec<ValueOrArray>,
) {
// For the foreign calls we want to handle, we do not want inputs, as they are getters
assert!(inputs.len() == 1); // storage_slot
assert!(inputs.len() == 2); // output, len. The latter is not used by the AVM, but required in the oracle call so that TXE knows how many slots to read.
assert!(destinations.len() == 1); // return values

let slot_offset_maybe = inputs[0];
Expand Down
8 changes: 4 additions & 4 deletions noir-projects/Dockerfile.test
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ COPY . .
# Build & test
RUN cd ./noir-protocol-circuits && ./bootstrap.sh && nargo test --use-legacy --silence-warnings

RUN cd /usr/src/yarn-project/txe && yarn start & echo $! > /tmp/txe.pid && \
RUN cd /usr/src/yarn-project/txe && yarn start & \
# Wait for TXE to initialize
sleep 5 && \
cd ./noir-contracts && \
# We need to increase the timeout since all tests running in parallel hammer TXE at the same time, and processing slows down leading to timeouts
# The only way we currently have to batch tests is via RAYON_NUM_THREADS, which is not ideal
./bootstrap.sh && NARGO_FOREIGN_CALL_TIMEOUT=300000 nargo test --use-legacy --silence-warnings --oracle-resolver http://localhost:8080 ; \
kill $(cat /tmp/txe.pid)
./bootstrap.sh && \
NARGO_FOREIGN_CALL_TIMEOUT=300000 nargo test --use-legacy --silence-warnings --oracle-resolver http://localhost:8080

RUN cd /usr/src/yarn-project/txe && yarn start & echo $! > /tmp/txe.pid && \
RUN cd /usr/src/yarn-project/txe && yarn start & \
# Wait for TXE to initialize
sleep 5 && \
cd ./aztec-nr && \
Expand Down
11 changes: 5 additions & 6 deletions noir-projects/Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,19 @@ test:
COPY +build/. /usr/src/noir-projects

RUN cd /usr/src/noir-projects/noir-protocol-circuits && nargo test --silence-warnings
RUN cd /usr/src/yarn-project/txe && yarn start & echo $! > /tmp/txe.pid && \

RUN cd /usr/src/yarn-project/txe && yarn start & \
# Wait for TXE to initialize
sleep 5 && \
cd /usr/src/noir-projects/aztec-nr && nargo test --use-legacy --silence-warnings --oracle-resolver http://localhost:8080 ; \
kill $(cat /tmp/txe.pid)
cd /usr/src/noir-projects/aztec-nr && nargo test --use-legacy --silence-warnings --oracle-resolver http://localhost:8080

RUN cd /usr/src/yarn-project/txe && yarn start & echo $! > /tmp/txe.pid && \
RUN cd /usr/src/yarn-project/txe && yarn start & \
# Wait for TXE to initialize
sleep 5 && \
cd /usr/src/noir-projects/noir-contracts && \
# We need to increase the timeout since all tests running in parallel hammer TXE at the same time and processing slows down, leading to timeouts
# The only way we currently have to batch tests is via RAYON_NUM_THREADS, which is not ideal
NARGO_FOREIGN_CALL_TIMEOUT=300000 nargo test --use-legacy --silence-warnings --oracle-resolver http://localhost:8080 ; \
kill $(cat /tmp/txe.pid)
NARGO_FOREIGN_CALL_TIMEOUT=300000 nargo test --use-legacy --silence-warnings --oracle-resolver http://localhost:8080

format:
FROM +build
Expand Down
4 changes: 2 additions & 2 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ unconstrained fn call_static<RET_SIZE>(
}

unconstrained fn storage_read<N>(storage_slot: Field) -> [Field; N] {
storage_read_opcode(storage_slot)
storage_read_opcode(storage_slot, N)
}

unconstrained fn storage_write<N>(storage_slot: Field, values: [Field; N]) {
Expand Down Expand Up @@ -373,7 +373,7 @@ unconstrained fn call_static_opcode<RET_SIZE>(
// ^ return data ^ success

#[oracle(avmOpcodeStorageRead)]
unconstrained fn storage_read_opcode<N>(storage_slot: Field) -> [Field; N] {}
unconstrained fn storage_read_opcode<N>(storage_slot: Field, length: Field) -> [Field; N] {}

#[oracle(avmOpcodeStorageWrite)]
unconstrained fn storage_write_opcode<N>(storage_slot: Field, values: [Field; N]) {}
Expand Down
24 changes: 20 additions & 4 deletions noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/test.nr
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,11 @@ fn test_get_current_value_in_private_bad_value_hints() {
let private_state_var = in_private(&mut env, schedule_block_number);

let mocked: ScheduledValueChange<Field> = ScheduledValueChange::new(0, new_value + 1, schedule_block_number);
let _ = OracleMock::mock("storageRead").with_params((private_state_var.get_value_change_storage_slot(), 3)).returns(mocked.serialize()).times(1);
let _ = OracleMock::mock("storageRead").with_params(
(
env.contract_address().to_field(), private_state_var.get_value_change_storage_slot(), schedule_block_number, 3
)
).returns(mocked.serialize()).times(1);

let _ = private_state_var.get_current_value_in_private();
}
Expand All @@ -291,7 +295,11 @@ fn test_get_current_value_in_private_bad_delay_hints() {
let private_state_var = in_private(&mut env, schedule_block_number);

let mocked: ScheduledDelayChange<TEST_INITIAL_DELAY> = ScheduledDelayChange::new(Option::none(), Option::some(42), schedule_block_number);
let _ = OracleMock::mock("storageRead").with_params((private_state_var.get_delay_change_storage_slot(), 1)).returns(mocked.serialize()).times(1);
let _ = OracleMock::mock("storageRead").with_params(
(
env.contract_address().to_field(), private_state_var.get_delay_change_storage_slot(), schedule_block_number, 1
)
).returns(mocked.serialize()).times(1);

let _ = private_state_var.get_current_value_in_private();
}
Expand All @@ -304,7 +312,11 @@ fn test_get_current_value_in_private_bad_zero_hash_value_hints() {
let state_var = in_private(&mut env, historical_block_number);

let mocked: ScheduledValueChange<Field> = ScheduledValueChange::new(0, new_value, 0);
let _ = OracleMock::mock("storageRead").with_params((state_var.get_value_change_storage_slot(), 3)).returns(mocked.serialize()).times(1);
let _ = OracleMock::mock("storageRead").with_params(
(
env.contract_address().to_field(), state_var.get_value_change_storage_slot(), historical_block_number, 3
)
).returns(mocked.serialize()).times(1);

let _ = state_var.get_current_value_in_private();
}
Expand All @@ -317,7 +329,11 @@ fn test_get_current_value_in_private_bad_zero_hash_delay_hints() {
let state_var = in_private(&mut env, historical_block_number);

let mocked: ScheduledDelayChange<TEST_INITIAL_DELAY> = ScheduledDelayChange::new(Option::none(), Option::some(new_delay), 0);
let _ = OracleMock::mock("storageRead").with_params((state_var.get_delay_change_storage_slot(), 1)).returns(mocked.serialize()).times(1);
let _ = OracleMock::mock("storageRead").with_params(
(
env.contract_address().to_field(), state_var.get_delay_change_storage_slot(), historical_block_number, 1
)
).returns(mocked.serialize()).times(1);

let _ = state_var.get_current_value_in_private();
}
13 changes: 13 additions & 0 deletions noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@ unconstrained pub fn add_note_hashes(contractAddress: AztecAddress, inner_note_h
oracle_add_note_hashes(contractAddress, inner_note_hashes)
}

unconstrained pub fn get_function_selector() -> FunctionSelector {
oracle_get_function_selector()
}

unconstrained pub fn set_fn_selector(selector: FunctionSelector) {
oracle_set_function_selector(selector)
}

#[oracle(reset)]
fn oracle_reset() {}

Expand Down Expand Up @@ -181,3 +189,8 @@ fn oracle_add_nullifiers(contractAddress: AztecAddress, nullifiers: [Field]) {}
#[oracle(addNoteHashes)]
fn oracle_add_note_hashes(contractAddress: AztecAddress, inner_note_hashes: [Field]) {}

#[oracle(getFunctionSelector)]
fn oracle_get_function_selector() -> FunctionSelector {}

#[oracle(setFunctionSelector)]
fn oracle_set_function_selector(selector: FunctionSelector) {}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ impl TestEnvironment {
cheatcodes::get_block_number()
}

fn contract_address(self) -> AztecAddress {
cheatcodes::get_contract_address()
}

fn advance_block_to(&mut self, block_number: u32) {
let difference = block_number - cheatcodes::get_block_number();
self.advance_block_by(difference);
Expand All @@ -40,7 +44,8 @@ impl TestEnvironment {
}

fn public(self) -> PublicContext {
PublicContext::empty()
let mut inputs = cheatcodes::get_public_context_inputs();
PublicContext::new(inputs)
}

fn private(&mut self) -> PrivateContext {
Expand Down Expand Up @@ -159,15 +164,19 @@ impl TestEnvironment {
let original_fn = call_interface.get_original();
let original_msg_sender = cheatcodes::get_msg_sender();
let original_contract_address = cheatcodes::get_contract_address();
let original_fn_selector = cheatcodes::get_function_selector();
let target_address = call_interface.get_contract_address();
let fn_selector = call_interface.get_selector();

cheatcodes::set_fn_selector(fn_selector);
cheatcodes::set_contract_address(target_address);
cheatcodes::set_msg_sender(original_contract_address);
let mut inputs = cheatcodes::get_public_context_inputs();
inputs.args_hash = hash_args(call_interface.get_args());
inputs.is_static_call = call_interface.get_is_static();
let result = original_fn(inputs);

cheatcodes::set_fn_selector(original_fn_selector);
cheatcodes::set_contract_address(original_contract_address);
cheatcodes::set_msg_sender(original_msg_sender);
result
Expand Down
3 changes: 3 additions & 0 deletions noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,16 @@ impl<N> Deployer<N> {
let original_fn = call_interface.get_original();
let original_msg_sender = cheatcodes::get_msg_sender();
let original_contract_address = cheatcodes::get_contract_address();
let original_fn_selector = cheatcodes::get_function_selector();

cheatcodes::set_fn_selector(call_interface.get_selector());
cheatcodes::set_contract_address(instance.to_address());
cheatcodes::set_msg_sender(original_contract_address);
let mut inputs = cheatcodes::get_public_context_inputs();
inputs.args_hash = hash_args(call_interface.get_args());
let _result: T = original_fn(inputs);

cheatcodes::set_fn_selector(original_fn_selector);
cheatcodes::set_contract_address(original_contract_address);
cheatcodes::set_msg_sender(original_msg_sender);
instance
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/txe/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules ../node_modules/.bin/jest --passWithNoTests",
"dev": "DEBUG='aztec:*' && node ./dest/bin/index.js",
"dev": "DEBUG='aztec:*' LOG_LEVEL=debug && node ./dest/bin/index.js",
"start": "node ./dest/bin/index.js"
},
"inherits": [
Expand Down
38 changes: 34 additions & 4 deletions yarn-project/txe/src/oracle/txe_oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ export class TXE implements TypedOracle {
return this.msgSender;
}

getFunctionSelector() {
return this.functionSelector;
}

setMsgSender(msgSender: Fr) {
this.msgSender = msgSender;
}
Expand Down Expand Up @@ -185,11 +189,10 @@ export class TXE implements TypedOracle {

getPublicContextInputs() {
const inputs = {
functionSelector: FunctionSelector.fromField(new Fr(0)),
argsHash: new Fr(0),
isStaticCall: false,
toFields: function () {
return [this.functionSelector.toField(), this.argsHash, new Fr(this.isStaticCall)];
return [this.argsHash, new Fr(this.isStaticCall)];
},
};
return inputs;
Expand Down Expand Up @@ -434,13 +437,40 @@ export class TXE implements TypedOracle {
throw new Error('Method not implemented.');
}

async avmOpcodeStorageRead(slot: Fr, length: Fr) {
const db = this.trees.asLatest();

const result = [];

for (let i = 0; i < length.toNumber(); i++) {
const leafSlot = computePublicDataTreeLeafSlot(this.contractAddress, slot.add(new Fr(i))).toBigInt();

const lowLeafResult = await db.getPreviousValueIndex(MerkleTreeId.PUBLIC_DATA_TREE, leafSlot);
if (!lowLeafResult || !lowLeafResult.alreadyPresent) {
result.push(Fr.ZERO);
continue;
}

const preimage = (await db.getLeafPreimage(
MerkleTreeId.PUBLIC_DATA_TREE,
lowLeafResult.index,
)) as PublicDataTreeLeafPreimage;

result.push(preimage.value);
}
return result;
}

async storageRead(
contractAddress: Fr,
startStorageSlot: Fr,
blockNumber: number, // TODO(#7230): use block number
blockNumber: number,
numberOfElements: number,
): Promise<Fr[]> {
const db = this.trees.asLatest();
const db =
blockNumber === (await this.getBlockNumber())
? this.trees.asLatest()
: new MerkleTreeSnapshotOperationsFacade(this.trees, blockNumber);

const values = [];
for (let i = 0n; i < numberOfElements; i++) {
Expand Down
51 changes: 49 additions & 2 deletions yarn-project/txe/src/txe_service/txe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,16 @@ export class TXEService {
return toForeignCallResult([]);
}

setFunctionSelector(functionSelector: ForeignCallSingle) {
(this.typedOracle as TXE).setFunctionSelector(FunctionSelector.fromField(fromSingle(functionSelector)));
return toForeignCallResult([]);
}

getFunctionSelector() {
const functionSelector = (this.typedOracle as TXE).getFunctionSelector();
return toForeignCallResult([toSingle(functionSelector.toField())]);
}

// PXE oracles

getRandomField() {
Expand All @@ -277,6 +287,11 @@ export class TXEService {
return toForeignCallResult([toSingle(new Fr(blockNumber))]);
}

avmOpcodeFunctionSelector() {
const functionSelector = (this.typedOracle as TXE).getFunctionSelector();
return toForeignCallResult([toSingle(functionSelector.toField())]);
}

async packArgumentsArray(args: ForeignCallArray) {
const packed = await this.typedOracle.packArgumentsArray(fromArray(args));
return toForeignCallResult([toSingle(packed)]);
Expand Down Expand Up @@ -506,13 +521,41 @@ export class TXEService {
fromSingle(address),
FunctionSelector.fromField(fromSingle(functionSelector)),
fromArray(args),
false,
false,
/* isStaticCall */ false,
/* isDelegateCall */ false,
);

return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]);
}

async avmOpcodeStaticCall(
_gas: ForeignCallArray,
address: ForeignCallSingle,
_length: ForeignCallSingle,
args: ForeignCallArray,
functionSelector: ForeignCallSingle,
) {
const result = await (this.typedOracle as TXE).avmOpcodeCall(
fromSingle(address),
FunctionSelector.fromField(fromSingle(functionSelector)),
fromArray(args),
/* isStaticCall */ true,
/* isDelegateCall */ false,
);

return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]);
}

async avmOpcodeStorageRead(slot: ForeignCallSingle, length: ForeignCallSingle) {
const values = await (this.typedOracle as TXE).avmOpcodeStorageRead(fromSingle(slot), fromSingle(length));
return toForeignCallResult([toArray(values)]);
}

async avmOpcodeStorageWrite(startStorageSlot: ForeignCallSingle, values: ForeignCallArray) {
await this.typedOracle.storageWrite(fromSingle(startStorageSlot), fromArray(values));
return toForeignCallResult([]);
}

async getPublicKeysAndPartialAddress(address: ForeignCallSingle) {
const parsedAddress = AztecAddress.fromField(fromSingle(address));
const { publicKeys, partialAddress } = await this.typedOracle.getCompleteAddress(parsedAddress);
Expand Down Expand Up @@ -571,6 +614,10 @@ export class TXEService {
return toForeignCallResult([]);
}

emitEncryptedEventLog(_contractAddress: AztecAddress, _randomness: Fr, _encryptedEvent: Buffer, _counter: number) {
return toForeignCallResult([]);
}

async callPrivateFunction(
targetContractAddress: ForeignCallSingle,
functionSelector: ForeignCallSingle,
Expand Down

0 comments on commit de303e2

Please sign in to comment.