Skip to content

Commit

Permalink
Merge branch 'master' into devnet
Browse files Browse the repository at this point in the history
  • Loading branch information
spypsy authored Jul 16, 2024
2 parents cd61415 + 1dcae45 commit 85c4063
Show file tree
Hide file tree
Showing 25 changed files with 148 additions and 35 deletions.
3 changes: 2 additions & 1 deletion l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ library Constants {
// Prime field modulus
uint256 internal constant P =
21888242871839275222246405745257275088548364400416034343698204186575808495617;
uint256 internal constant MAX_FIELD_VALUE = P - 1;

uint256 internal constant MAX_FIELD_VALUE =
21888242871839275222246405745257275088548364400416034343698204186575808495616;
uint256 internal constant ARGS_LENGTH = 16;
uint256 internal constant MAX_NOTE_HASHES_PER_CALL = 16;
uint256 internal constant MAX_NULLIFIERS_PER_CALL = 16;
Expand Down
6 changes: 3 additions & 3 deletions l1-contracts/test/Inbox.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ contract InboxTest is Test {

function testRevertIfActorTooLarge() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
message.recipient.actor = bytes32(Constants.MAX_FIELD_VALUE + 1);
message.recipient.actor = bytes32(Constants.P);
vm.expectRevert(
abi.encodeWithSelector(Errors.Inbox__ActorTooLarge.selector, message.recipient.actor)
);
Expand All @@ -114,14 +114,14 @@ contract InboxTest is Test {

function testRevertIfContentTooLarge() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
message.content = bytes32(Constants.MAX_FIELD_VALUE + 1);
message.content = bytes32(Constants.P);
vm.expectRevert(abi.encodeWithSelector(Errors.Inbox__ContentTooLarge.selector, message.content));
inbox.sendL2Message(message.recipient, message.content, message.secretHash);
}

function testRevertIfSecretHashTooLarge() public {
DataStructures.L1ToL2Msg memory message = _fakeMessage();
message.secretHash = bytes32(Constants.MAX_FIELD_VALUE + 1);
message.secretHash = bytes32(Constants.P);
vm.expectRevert(
abi.encodeWithSelector(Errors.Inbox__SecretHashTooLarge.selector, message.secretHash)
);
Expand Down
6 changes: 3 additions & 3 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::hash::{compute_secret_hash, compute_message_hash, compute_message_nullifier};
use dep::protocol_types::address::{AztecAddress, EthAddress};
use dep::protocol_types::constants::MAX_FIELD_VALUE;
use dep::protocol_types::traits::{Serialize, Deserialize, Empty};
use dep::protocol_types::abis::function_selector::FunctionSelector;
use crate::context::inputs::public_context_inputs::PublicContextInputs;
Expand Down Expand Up @@ -185,10 +186,9 @@ impl PublicContext {
fn gas_for_call(user_gas: GasOpts) -> [Field; 2] {
// It's ok to use the max possible gas here, because the gas will be
// capped by the gas left in the (STATIC)CALL instruction.
let MAX_POSSIBLE_FIELD: Field = 0 - 1;
[
user_gas.l2_gas.unwrap_or(MAX_POSSIBLE_FIELD),
user_gas.da_gas.unwrap_or(MAX_POSSIBLE_FIELD)
user_gas.l2_gas.unwrap_or(MAX_FIELD_VALUE),
user_gas.da_gas.unwrap_or(MAX_FIELD_VALUE)
]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ contract AppSubscription {
AztecAddress, FunctionSelector, PrivateContext, NoteHeader, Map, PrivateMutable, PublicMutable,
SharedImmutable
},
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note
encrypted_logs::encrypted_note_emission::encode_and_encrypt_note,
protocol_types::constants::MAX_FIELD_VALUE
};
use authwit::{auth_witness::get_auth_witness, auth::assert_current_call_valid_authwit};
use gas_token::GasToken;
Expand All @@ -34,7 +35,8 @@ contract AppSubscription {

#[aztec(private)]
fn entrypoint(payload: DAppPayload, user_address: AztecAddress) {
assert(context.msg_sender().to_field() == 0);
// Default msg_sender for entrypoints is now Fr.max_value rather than 0 addr (see #7190 & #7404)
assert(context.msg_sender().to_field() == MAX_FIELD_VALUE);
assert_current_call_valid_authwit(&mut context, user_address);

let mut note = storage.subscriptions.at(user_address).get_note().note;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use dep::types::{
private_kernel::private_call_data::PrivateCallData, side_effect::{Ordered, RangeOrdered}
},
address::{AztecAddress, PartialAddress}, contract_class_id::ContractClassId,
constants::MAX_FIELD_VALUE,
hash::{private_functions_root_from_siblings, stdlib_recursion_verification_key_compress_native_vk},
traits::is_empty, transaction::tx_request::TxRequest, utils::arrays::find_index
};
Expand Down Expand Up @@ -121,6 +122,9 @@ impl PrivateCallDataValidator {
let call_context = public_inputs.call_context;
assert(call_context.is_delegate_call == false, "Users cannot make a delegatecall");
assert(call_context.is_static_call == false, "Users cannot make a static call");
assert(
call_context.msg_sender == AztecAddress::from_field(MAX_FIELD_VALUE), "Users cannot set msg_sender in first call"
);
}

// Confirm that the TxRequest (user's intent) matches the private call being executed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ mod tests {

impl PrivateKernelInitInputsBuilder {
pub fn new() -> Self {
let private_call = FixtureBuilder::new();
let private_call = FixtureBuilder::new().is_first_call();
let tx_request = private_call.build_tx_request();
PrivateKernelInitInputsBuilder { tx_request, private_call }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ impl PrivateCallDataValidatorBuilder {
*self
}

pub fn is_first_call(&mut self) -> Self {
let _ = self.private_call.is_first_call();
*self
}

pub fn validate(self) {
let private_call = self.private_call.to_private_call_data();
let mut accumulated_note_hashes = self.previous_note_hashes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@ use crate::tests::private_call_data_validator_builder::PrivateCallDataValidatorB

#[test]
fn validate_as_first_call_regular_call_succeeds() {
let builder = PrivateCallDataValidatorBuilder::new();
let builder = PrivateCallDataValidatorBuilder::new().is_first_call();
builder.validate_as_first_call();
}

#[test(should_fail_with="Users cannot make a static call")]
fn validate_as_first_call_static_call_fails() {
let builder = PrivateCallDataValidatorBuilder::new().is_static_call();
let builder = PrivateCallDataValidatorBuilder::new().is_first_call().is_static_call();
builder.validate_as_first_call();
}

#[test(should_fail_with="Users cannot make a delegatecall")]
fn validate_as_first_call_delegate_call_fails() {
let builder = PrivateCallDataValidatorBuilder::new().is_delegate_call();
let builder = PrivateCallDataValidatorBuilder::new().is_first_call().is_delegate_call();
builder.validate_as_first_call();
}

#[test(should_fail_with="Users cannot set msg_sender in first call")]
fn validate_as_first_call_msg_sender_fails() {
let builder = PrivateCallDataValidatorBuilder::new();
builder.validate_as_first_call();
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
global MAX_FIELD_VALUE: Field = 21888242871839275222246405745257275088548364400416034343698204186575808495616;
global ARGS_LENGTH: u32 = 16;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
address::{AztecAddress, EthAddress, SaltedInitializationHash, PublicKeysHash},
constants::{
FUNCTION_TREE_HEIGHT, MAX_NOTE_HASHES_PER_TX, MAX_NULLIFIERS_PER_TX, MAX_L2_TO_L1_MSGS_PER_TX,
MAX_PUBLIC_DATA_READS_PER_TX, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
MAX_PUBLIC_DATA_READS_PER_TX, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_FIELD_VALUE,
MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX, MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX,
MAX_NOTE_HASH_READ_REQUESTS_PER_TX, MAX_NULLIFIER_READ_REQUESTS_PER_TX,
MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_TX, MAX_KEY_VALIDATION_REQUESTS_PER_TX, VK_TREE_HEIGHT,
Expand Down Expand Up @@ -203,6 +203,11 @@ impl FixtureBuilder {
*self
}

pub fn is_first_call(&mut self) -> Self {
self.msg_sender = AztecAddress::from_field(MAX_FIELD_VALUE);
*self
}

pub fn to_constant_data(self) -> CombinedConstantData {
CombinedConstantData {
historical_header: self.historical_header,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,18 @@ unconstrained fn bytes_field_test() {
}
assert_eq(field2, field);
}

#[test]
unconstrained fn max_field_test() {
// Tests the hardcoded value in constants.nr vs underlying modulus
// NB: We can't use 0-1 in constants.nr as it will be transpiled incorrectly to ts and sol constants files
let max_value = crate::constants::MAX_FIELD_VALUE;
assert_eq(max_value, 0 - 1);
// modulus == 0 is tested elsewhere, so below is more of a sanity check
let max_bytes = max_value.to_be_bytes(32);
let mod_bytes = std::field::modulus_be_bytes();
for i in 0..31 {
assert_eq(max_bytes[i], mod_bytes[i]);
}
assert_eq(max_bytes[31], mod_bytes[31] - 1);
}
2 changes: 1 addition & 1 deletion yarn-project/aztec.js/src/wallet/base_wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export abstract class BaseWallet implements Wallet {
proveTx(txRequest: TxExecutionRequest, simulatePublic: boolean): Promise<Tx> {
return this.pxe.proveTx(txRequest, simulatePublic);
}
simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean, msgSender: AztecAddress): Promise<SimulatedTx> {
simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean, msgSender?: AztecAddress): Promise<SimulatedTx> {
return this.pxe.simulateTx(txRequest, simulatePublic, msgSender);
}
sendTx(tx: Tx): Promise<TxHash> {
Expand Down
1 change: 1 addition & 0 deletions yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable */
// GENERATED FILE - DO NOT EDIT, RUN yarn remake-constants
export const MAX_FIELD_VALUE = 21888242871839275222246405745257275088548364400416034343698204186575808495616n;
export const ARGS_LENGTH = 16;
export const MAX_NOTE_HASHES_PER_CALL = 16;
export const MAX_NULLIFIERS_PER_CALL = 16;
Expand Down
1 change: 0 additions & 1 deletion yarn-project/circuits.js/src/scripts/constants.in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ library Constants {
// Prime field modulus
uint256 internal constant P =
21888242871839275222246405745257275088548364400416034343698204186575808495617;
uint256 internal constant MAX_FIELD_VALUE = P - 1;
${processConstantsSolidity(constants)}
}\n`;
Expand Down
10 changes: 10 additions & 0 deletions yarn-project/circuits.js/src/utils/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { makeTuple } from '@aztec/foundation/array';
import { Fr } from '@aztec/foundation/fields';
import { type Tuple } from '@aztec/foundation/serialize';

import { MAX_FIELD_VALUE } from '../constants.gen.js';
import { type IsEmpty } from '../interfaces/index.js';
import {
countAccumulatedItems,
Expand Down Expand Up @@ -502,4 +503,13 @@ describe('utils', () => {
expect(getNonEmptyItems(arr)).toEqual([]);
});
});

describe('Constants', () => {
it('fr.max and const.max should be in sync', () => {
// Ideally this test would live in foundation/field, but that creates a circular dependency
// since constants live in circuits.js
expect(new Fr(MAX_FIELD_VALUE)).toEqual(Fr.MAX_FIELD_VALUE);
expect(new Fr(MAX_FIELD_VALUE)).toEqual(Fr.ONE.negate());
});
});
});
2 changes: 1 addition & 1 deletion yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('e2e_avm_simulator', () => {
describe('Gas metering', () => {
it('Tracks L2 gas usage on simulation', async () => {
const request = await avmContract.methods.add_args_return(20n, 30n).create();
const simulation = await wallet.simulateTx(request, true, wallet.getAddress());
const simulation = await wallet.simulateTx(request, true);
// Subtract the teardown gas allocation from the gas used to figure out the gas used by the contract logic.
const l2TeardownAllocation = GasSettings.simulation().getTeardownLimits().l2Gas;
const l2GasUsed = simulation.publicOutput!.end.gasUsed.l2Gas! - l2TeardownAllocation;
Expand Down
45 changes: 44 additions & 1 deletion yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import {
Fr,
Note,
type PXE,
PackedValues,
TxExecutionRequest,
type TxHash,
computeSecretHash,
deriveKeys,
} from '@aztec/aztec.js';
import { computePartialAddress } from '@aztec/circuits.js';
import { GasSettings, TxContext, computePartialAddress } from '@aztec/circuits.js';
import { InclusionProofsContract } from '@aztec/noir-contracts.js';
import { ClaimContract } from '@aztec/noir-contracts.js/Claim';
import { CrowdfundingContract } from '@aztec/noir-contracts.js/Crowdfunding';
Expand Down Expand Up @@ -361,6 +363,47 @@ describe('e2e_crowdfunding_and_claim', () => {
).rejects.toThrow();
});

it('cannot withdraw as non operator', async () => {
const donationAmount = 500n;

// 1) We add authwit so that the Crowdfunding contract can transfer donor's DNT
const action = donationToken
.withWallet(donorWallets[1])
.methods.transfer_from(donorWallets[1].getAddress(), crowdfundingContract.address, donationAmount, 0);
const witness = await donorWallets[1].createAuthWit({ caller: crowdfundingContract.address, action });
await donorWallets[1].addAuthWitness(witness);

// 2) We donate to the crowdfunding contract
await crowdfundingContract.withWallet(donorWallets[1]).methods.donate(donationAmount).send().wait({
debug: true,
});

// Calling the function normally will fail as msg_sender != operator
await expect(
crowdfundingContract.withWallet(donorWallets[1]).methods.withdraw(donationAmount).send().wait(),
).rejects.toThrow('Assertion failed: Not an operator');

// Instead, we construct a call and impersonate operator by skipping the usual account contract entrypoint...
const call = crowdfundingContract.withWallet(donorWallets[1]).methods.withdraw(donationAmount).request();
// ...using the withdraw fn as our entrypoint
const entrypointPackedValues = PackedValues.fromValues(call.args);
const request = new TxExecutionRequest(
call.to,
call.selector,
entrypointPackedValues.hash,
new TxContext(donorWallets[1].getChainId(), donorWallets[1].getVersion(), GasSettings.default()),
[entrypointPackedValues],
[],
);
// NB: Removing the msg_sender assertion from private_init will still result in a throw, as we are using
// a non-entrypoint function (withdraw never calls context.end_setup()), meaning the min revertible counter will remain 0.
// This does not protect fully against impersonation as the contract could just call context.end_setup() and the below would pass.
// => the private_init msg_sender assertion is required (#7190, #7404)
await expect(donorWallets[1].simulateTx(request, true, operatorWallet.getAddress())).rejects.toThrow(
'Assertion failed: Users cannot set msg_sender in first call',
);
});

it('cannot donate after a deadline', async () => {
const donationAmount = 1000n;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ describe('e2e_token_contract minting', () => {
);
});

it('mint_private as non-minter, bypassing account entrypoint', async () => {
const request = await asset.withWallet(wallets[1]).methods.mint_private(amount, secretHash).create();
await expect(wallets[1].simulateTx(request, true, accounts[0].address)).rejects.toThrow(
'Assertion failed: Users cannot set msg_sender in first call',
);
});

it('mint >u128 tokens to overflow', async () => {
const amount = 2n ** 128n; // U128::max() + 1;
await expect(asset.methods.mint_private(amount, secretHash).simulate()).rejects.toThrow(BITSIZE_TOO_BIG_ERROR);
Expand Down
8 changes: 6 additions & 2 deletions yarn-project/entrypoints/src/dapp_entrypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ export class DefaultDappEntrypoint implements EntrypointInterface {
const entrypointPackedArgs = PackedValues.fromValues(encodeArguments(abi, [payload, this.userAddress]));
const gasSettings = exec.fee?.gasSettings ?? GasSettings.default();
const functionSelector = FunctionSelector.fromNameAndParameters(abi.name, abi.parameters);

const innerHash = computeInnerAuthWitHash([Fr.ZERO, functionSelector.toField(), entrypointPackedArgs.hash]);
// Default msg_sender for entrypoints is now Fr.max_value rather than 0 addr (see #7190 & #7404)
const innerHash = computeInnerAuthWitHash([
Fr.MAX_FIELD_VALUE,
functionSelector.toField(),
entrypointPackedArgs.hash,
]);
const outerHash = computeAuthWitMessageHash(
{ consumer: this.dappEntrypointAddress, innerHash },
{ chainId: new Fr(this.chainId), version: new Fr(this.version) },
Expand Down
Loading

0 comments on commit 85c4063

Please sign in to comment.