From 4e59b06cd1956d43bc44a219448603b4bcf58d27 Mon Sep 17 00:00:00 2001 From: Leila Wang Date: Mon, 16 Dec 2024 15:53:39 +0000 Subject: [PATCH] feat: check max fees per gas (#10283) Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. --- .../tail_to_public_output_validator.nr | 3 +- .../base_or_merge_rollup_public_inputs.nr | 3 +- .../crates/rollup-lib/src/abis/mod.nr | 1 - .../src/base/components/constants.nr | 5 +- .../rollup-lib/src/base/components/mod.nr | 7 ++ .../components/private_tube_data_validator.nr | 28 ++++++ .../components/public_tube_data_validator.nr | 28 ++++++ .../src/base/components/validate_tube_data.nr | 12 +++ .../crates/rollup-lib/src/base/mod.nr | 1 + .../src/base/private_base_rollup.nr | 38 ++++---- .../rollup-lib/src/base/public_base_rollup.nr | 31 +++---- .../crates/rollup-lib/src/base/tests/mod.nr | 1 + .../mod.nr | 24 +++++ .../validate_with_rollup_data.nr | 27 ++++++ .../src/abis/avm_circuit_public_inputs.nr | 2 +- .../src/abis/constant_rollup_data.nr | 2 +- .../crates/types/src/abis/mod.nr | 1 + .../abis/private_kernel/private_call_data.nr | 2 +- .../crates/types/src/tests/fixture_builder.nr | 87 +++++++++++-------- .../src/tx/validator/empty_validator.ts | 4 +- .../src/tx/validator/tx_validator.ts | 2 +- .../aggregate_tx_validator.test.ts | 49 ++++++++--- .../tx_validator/aggregate_tx_validator.ts | 8 +- .../src/sequencer/sequencer.test.ts | 42 +++++++++ .../src/tx_validator/gas_validator.test.ts | 75 ++++++++++------ .../src/tx_validator/gas_validator.ts | 38 ++++++-- .../src/tx_validator/tx_validator_factory.ts | 7 +- 27 files changed, 398 insertions(+), 130 deletions(-) create mode 100644 noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/private_tube_data_validator.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_tube_data_validator.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/validate_tube_data.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/mod.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/mod.nr create mode 100644 noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/validate_with_rollup_data.nr rename noir-projects/noir-protocol-circuits/crates/{rollup-lib => types}/src/abis/constant_rollup_data.nr (99%) diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_to_public_output_validator.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_to_public_output_validator.nr index bbf93064ee1..77f00bbd3f3 100644 --- a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_to_public_output_validator.nr +++ b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/tail_to_public_output_validator.nr @@ -9,9 +9,8 @@ use dep::types::{ log_hash::ScopedLogHash, note_hash::ScopedNoteHash, nullifier::ScopedNullifier, - private_log::{PrivateLog, PrivateLogData}, public_call_request::PublicCallRequest, - side_effect::{Counted, scoped::Scoped}, + side_effect::Counted, }, messaging::l2_to_l1_message::ScopedL2ToL1Message, utils::arrays::{ diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/base_or_merge_rollup_public_inputs.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/base_or_merge_rollup_public_inputs.nr index 7dc56ab336a..155471c7962 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/base_or_merge_rollup_public_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/base_or_merge_rollup_public_inputs.nr @@ -1,6 +1,5 @@ -use crate::abis::constant_rollup_data::ConstantRollupData; use dep::types::{ - abis::sponge_blob::SpongeBlob, + abis::{constant_rollup_data::ConstantRollupData, sponge_blob::SpongeBlob}, constants::BASE_OR_MERGE_PUBLIC_INPUTS_LENGTH, partial_state_reference::PartialStateReference, traits::{Deserialize, Empty, Serialize}, diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/mod.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/mod.nr index 716b67bf953..0bcaaa80b27 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/mod.nr @@ -1,4 +1,3 @@ -pub(crate) mod constant_rollup_data; pub(crate) mod base_or_merge_rollup_public_inputs; pub(crate) mod block_root_or_block_merge_public_inputs; pub(crate) mod previous_rollup_data; diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/constants.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/constants.nr index 0e7f459fecd..dbe2eb34fd6 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/constants.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/constants.nr @@ -1,5 +1,6 @@ -use crate::abis::constant_rollup_data::ConstantRollupData; -use types::abis::combined_constant_data::CombinedConstantData; +use types::abis::{ + combined_constant_data::CombinedConstantData, constant_rollup_data::ConstantRollupData, +}; pub(crate) fn validate_combined_constant_data( constants: CombinedConstantData, diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/mod.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/mod.nr index 9e3e70f9433..128c7bed5bd 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/mod.nr @@ -3,3 +3,10 @@ pub(crate) mod constants; pub(crate) mod fees; pub(crate) mod nullifier_tree; pub(crate) mod public_data_tree; + +mod private_tube_data_validator; +mod public_tube_data_validator; +pub(crate) mod validate_tube_data; + +pub(crate) use private_tube_data_validator::PrivateTubeDataValidator; +pub(crate) use public_tube_data_validator::PublicTubeDataValidator; diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/private_tube_data_validator.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/private_tube_data_validator.nr new file mode 100644 index 00000000000..8027741142f --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/private_tube_data_validator.nr @@ -0,0 +1,28 @@ +use super::validate_tube_data::validate_max_fees_per_gas; +use dep::types::abis::{constant_rollup_data::ConstantRollupData, tube::PrivateTubeData}; + +pub struct PrivateTubeDataValidator { + pub data: PrivateTubeData, +} + +// TODO: Move relevant verifications here. +impl PrivateTubeDataValidator { + pub fn new(data: PrivateTubeData) -> Self { + PrivateTubeDataValidator { data } + } + + pub fn verify_proof(self, _allowed_previous_circuits: [u32; N]) { + if !dep::std::runtime::is_unconstrained() { + self.data.verify(); + // TODO(#7410) + // self.data.vk_data.validate_in_vk_tree(self.data.public_inputs.constants.vk_tree_root, allowed_previous_circuits); + } + } + + pub fn validate_with_rollup_data(self, constants: ConstantRollupData) { + validate_max_fees_per_gas( + self.data.public_inputs.constants.tx_context.gas_settings.max_fees_per_gas, + constants.global_variables.gas_fees, + ); + } +} diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_tube_data_validator.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_tube_data_validator.nr new file mode 100644 index 00000000000..6edeb1ad88b --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/public_tube_data_validator.nr @@ -0,0 +1,28 @@ +use super::validate_tube_data::validate_max_fees_per_gas; +use dep::types::abis::{constant_rollup_data::ConstantRollupData, tube::PublicTubeData}; + +pub struct PublicTubeDataValidator { + pub data: PublicTubeData, +} + +// TODO: Move relevant verifications here. +impl PublicTubeDataValidator { + pub fn new(data: PublicTubeData) -> Self { + PublicTubeDataValidator { data } + } + + pub fn verify_proof(self) { + if !dep::std::runtime::is_unconstrained() { + self.data.verify(); + // TODO(#7410) + // self.tube_data.vk_data.validate_in_vk_tree(self.tube_data.public_inputs.constants.vk_tree_root, ALLOWED_PREVIOUS_CIRCUITS); + } + } + + pub fn validate_with_rollup_data(self, constants: ConstantRollupData) { + validate_max_fees_per_gas( + self.data.public_inputs.constants.tx_context.gas_settings.max_fees_per_gas, + constants.global_variables.gas_fees, + ); + } +} diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/validate_tube_data.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/validate_tube_data.nr new file mode 100644 index 00000000000..3c815f2d235 --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/components/validate_tube_data.nr @@ -0,0 +1,12 @@ +use dep::types::abis::gas_fees::GasFees; + +pub fn validate_max_fees_per_gas(max_fees_per_gas: GasFees, gas_fees: GasFees) { + assert( + !max_fees_per_gas.fee_per_da_gas.lt(gas_fees.fee_per_da_gas), + "da gas is higher than the maximum specified by the tx", + ); + assert( + !max_fees_per_gas.fee_per_l2_gas.lt(gas_fees.fee_per_l2_gas), + "l2 gas is higher than the maximum specified by the tx", + ); +} diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/mod.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/mod.nr index 03b442b4eaa..9b9bc52e9bd 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/mod.nr @@ -2,6 +2,7 @@ pub(crate) mod components; pub(crate) mod state_diff_hints; mod private_base_rollup; mod public_base_rollup; +mod tests; pub use crate::abis::base_or_merge_rollup_public_inputs::BaseOrMergeRollupPublicInputs; pub use private_base_rollup::PrivateBaseRollupInputs; diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/private_base_rollup.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/private_base_rollup.nr index b5952b6b3d7..06bdc554757 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/private_base_rollup.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/private_base_rollup.nr @@ -1,13 +1,11 @@ use crate::{ - abis::{ - base_or_merge_rollup_public_inputs::{BASE_ROLLUP_TYPE, BaseOrMergeRollupPublicInputs}, - constant_rollup_data::ConstantRollupData, - }, + abis::base_or_merge_rollup_public_inputs::{BASE_ROLLUP_TYPE, BaseOrMergeRollupPublicInputs}, base::{ components::{ archive::perform_archive_membership_check, constants::validate_combined_constant_data, fees::compute_fee_payer_fee_juice_balance_leaf_slot, - nullifier_tree::nullifier_tree_batch_insert, public_data_tree::public_data_tree_insert, + nullifier_tree::nullifier_tree_batch_insert, PrivateTubeDataValidator, + public_data_tree::public_data_tree_insert, }, state_diff_hints::PrivateBaseStateDiffHints, }, @@ -15,12 +13,13 @@ use crate::{ }; use dep::types::{ abis::{ - append_only_tree_snapshot::AppendOnlyTreeSnapshot, + append_only_tree_snapshot::AppendOnlyTreeSnapshot, constant_rollup_data::ConstantRollupData, nullifier_leaf_preimage::NullifierLeafPreimage, public_data_write::PublicDataWrite, sponge_blob::SpongeBlob, tube::PrivateTubeData, }, constants::{ ARCHIVE_HEIGHT, MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, NOTE_HASH_SUBTREE_HEIGHT, + TUBE_VK_INDEX, }, data::{hash::compute_public_data_tree_value, public_data_hint::PublicDataHint}, hash::silo_l2_to_l1_message, @@ -32,7 +31,7 @@ use dep::types::{ traits::is_empty, }; -// global ALLOWED_PREVIOUS_CIRCUITS: [u32; 2] = [PRIVATE_KERNEL_EMPTY_INDEX, TUBE_VK_INDEX]; +global ALLOWED_PREVIOUS_CIRCUITS: [u32; 1] = [TUBE_VK_INDEX]; pub struct PrivateBaseRollupInputs { tube_data: PrivateTubeData, @@ -55,10 +54,14 @@ impl PrivateBaseRollupInputs { } pub fn execute(self) -> BaseOrMergeRollupPublicInputs { - if !dep::std::runtime::is_unconstrained() { - self.tube_data.verify(); - // TODO(#7410) - // self.tube_data.vk_data.validate_in_vk_tree(self.tube_data.public_inputs.constants.vk_tree_root, ALLOWED_PREVIOUS_CIRCUITS); + // TODO(#10311): There should be an empty base rollup. + // There's at least 1 nullifier for a tx. So gas_used won't be empty if the previous circuit is private_tail. + let is_empty_tube = self.tube_data.public_inputs.gas_used.is_empty(); + + let tube_data_validator = PrivateTubeDataValidator::new(self.tube_data); + tube_data_validator.verify_proof(ALLOWED_PREVIOUS_CIRCUITS); + if !is_empty_tube { + tube_data_validator.validate_with_rollup_data(self.constants); } let transaction_fee = self.compute_transaction_fee(); @@ -223,10 +226,7 @@ impl PrivateBaseRollupInputs { mod tests { use crate::{ - abis::{ - base_or_merge_rollup_public_inputs::BaseOrMergeRollupPublicInputs, - constant_rollup_data::ConstantRollupData, - }, + abis::base_or_merge_rollup_public_inputs::BaseOrMergeRollupPublicInputs, base::{ components::fees::compute_fee_payer_fee_juice_balance_leaf_slot, private_base_rollup::PrivateBaseRollupInputs, @@ -239,7 +239,8 @@ mod tests { use dep::types::{ abis::{ accumulated_data::CombinedAccumulatedData, - append_only_tree_snapshot::AppendOnlyTreeSnapshot, gas::Gas, gas_fees::GasFees, + append_only_tree_snapshot::AppendOnlyTreeSnapshot, + constant_rollup_data::ConstantRollupData, gas::Gas, gas_fees::GasFees, kernel_circuit_public_inputs::KernelCircuitPublicInputs, nullifier_leaf_preimage::NullifierLeafPreimage, public_data_write::PublicDataWrite, sponge_blob::SpongeBlob, @@ -887,6 +888,8 @@ mod tests { builder.tube_data.set_gas_used(100, 200); builder.constants.global_variables.gas_fees.fee_per_da_gas = 1; builder.constants.global_variables.gas_fees.fee_per_l2_gas = 1; + builder.tube_data.tx_context.gas_settings.max_fees_per_gas.fee_per_da_gas = 1; + builder.tube_data.tx_context.gas_settings.max_fees_per_gas.fee_per_l2_gas = 1; let tx_fee = builder.compute_transaction_fee(); // builder.transaction_fee = tx_fee; builder.tube_data.append_note_hashes_with_logs(NUM_NOTES); @@ -1118,6 +1121,7 @@ mod tests { // Set gas builder.tube_data.gas_used = gas_used; + builder.tube_data.tx_context.gas_settings.max_fees_per_gas = gas_fees; builder.constants.global_variables.gas_fees = gas_fees; // Set fee payer @@ -1167,6 +1171,7 @@ mod tests { // Set gas builder.tube_data.gas_used = gas_used; + builder.tube_data.tx_context.gas_settings.max_fees_per_gas = gas_fees; builder.constants.global_variables.gas_fees = gas_fees; // Set fee payer @@ -1197,6 +1202,7 @@ mod tests { // Set gas builder.tube_data.gas_used = gas_used; + builder.tube_data.tx_context.gas_settings.max_fees_per_gas = gas_fees; builder.constants.global_variables.gas_fees = gas_fees; // Set fee payer diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr index 3b680e66e7c..827d87973b0 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/public_base_rollup.nr @@ -1,12 +1,10 @@ use crate::{ - abis::{ - base_or_merge_rollup_public_inputs::{BASE_ROLLUP_TYPE, BaseOrMergeRollupPublicInputs}, - constant_rollup_data::ConstantRollupData, - }, + abis::base_or_merge_rollup_public_inputs::{BASE_ROLLUP_TYPE, BaseOrMergeRollupPublicInputs}, base::{ components::{ archive::perform_archive_membership_check, constants::validate_combined_constant_data, nullifier_tree::nullifier_tree_batch_insert, public_data_tree::public_data_tree_insert, + PublicTubeDataValidator, }, state_diff_hints::PublicBaseStateDiffHints, }, @@ -18,6 +16,7 @@ use dep::types::{ append_only_tree_snapshot::AppendOnlyTreeSnapshot, avm_circuit_public_inputs::AvmProofData, combined_constant_data::CombinedConstantData, + constant_rollup_data::ConstantRollupData, log_hash::ScopedLogHash, nullifier_leaf_preimage::NullifierLeafPreimage, public_data_write::PublicDataWrite, @@ -92,11 +91,9 @@ impl PublicBaseRollupInputs { } pub fn execute(self) -> BaseOrMergeRollupPublicInputs { - if !dep::std::runtime::is_unconstrained() { - self.tube_data.verify(); - // TODO(#7410) - // self.tube_data.vk_data.validate_in_vk_tree([TUBE_VK_INDEX]); - } + let tube_data_validator = PublicTubeDataValidator::new(self.tube_data); + tube_data_validator.verify_proof(); + tube_data_validator.validate_with_rollup_data(self.constants); // Warning: Fake verification! TODO(#8470) // if !dep::std::runtime::is_unconstrained() { @@ -263,10 +260,7 @@ impl PublicBaseRollupInputs { mod tests { use crate::{ - abis::{ - base_or_merge_rollup_public_inputs::BaseOrMergeRollupPublicInputs, - constant_rollup_data::ConstantRollupData, - }, + abis::base_or_merge_rollup_public_inputs::BaseOrMergeRollupPublicInputs, base::{ public_base_rollup::PublicBaseRollupInputs, state_diff_hints::PublicBaseStateDiffHints, }, @@ -278,6 +272,7 @@ mod tests { abis::{ accumulated_data::CombinedAccumulatedData, append_only_tree_snapshot::AppendOnlyTreeSnapshot, + constant_rollup_data::ConstantRollupData, nullifier_leaf_preimage::NullifierLeafPreimage, public_data_write::PublicDataWrite, sponge_blob::SpongeBlob, }, @@ -298,7 +293,11 @@ mod tests { merkle_tree::MembershipWitness, messaging::l2_to_l1_message::ScopedL2ToL1Message, partial_state_reference::PartialStateReference, - tests::{fixture_builder::FixtureBuilder, fixtures, merkle_tree_utils::NonEmptyMerkleTree}, + tests::{ + fixture_builder::FixtureBuilder, + fixtures::{self, merkle_tree::generate_full_sha_tree}, + merkle_tree_utils::NonEmptyMerkleTree, + }, traits::{Empty, is_empty}, utils::{ arrays::{array_concat, get_sorted_tuple::get_sorted_tuple}, @@ -1070,9 +1069,7 @@ mod tests { ); // Since we fill the tree completely, we know to expect a full tree as below - let expected_tree = dep::types::merkle_tree::variable_merkle_tree::tests::generate_full_sha_tree( - siloed_l2_to_l1_msgs.storage(), - ); + let expected_tree = generate_full_sha_tree(siloed_l2_to_l1_msgs.storage()); assert_eq(out_hash, expected_tree.get_root()); } diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/mod.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/mod.nr new file mode 100644 index 00000000000..7e88fe33e7d --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/mod.nr @@ -0,0 +1 @@ +mod private_tube_data_validator_builder; diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/mod.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/mod.nr new file mode 100644 index 00000000000..cefbf64d37d --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/mod.nr @@ -0,0 +1,24 @@ +mod validate_with_rollup_data; + +use crate::base::components::PrivateTubeDataValidator; +use dep::types::tests::fixture_builder::FixtureBuilder; + +pub struct PrivateTubeDataValidatorBuilder { + pub tube_data: FixtureBuilder, + pub rollup_data: FixtureBuilder, +} + +impl PrivateTubeDataValidatorBuilder { + pub fn new() -> Self { + PrivateTubeDataValidatorBuilder { + tube_data: FixtureBuilder::new(), + rollup_data: FixtureBuilder::new(), + } + } + + pub fn validate_with_rollup_data(self) { + let tube_data = self.tube_data.to_private_tube_data(); + let rollup_data = self.rollup_data.to_constant_rollup_data(); + PrivateTubeDataValidator::new(tube_data).validate_with_rollup_data(rollup_data); + } +} diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/validate_with_rollup_data.nr b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/validate_with_rollup_data.nr new file mode 100644 index 00000000000..64756b1efb9 --- /dev/null +++ b/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/base/tests/private_tube_data_validator_builder/validate_with_rollup_data.nr @@ -0,0 +1,27 @@ +use super::PrivateTubeDataValidatorBuilder; + +#[test] +fn validate_with_rollup_data_success() { + let builder = PrivateTubeDataValidatorBuilder::new(); + builder.validate_with_rollup_data(); +} + +#[test(should_fail_with = "da gas is higher than the maximum specified by the tx")] +fn validate_with_rollup_data_not_enough_fee_per_da_gas_fails() { + let mut builder = PrivateTubeDataValidatorBuilder::new(); + + builder.tube_data.tx_context.gas_settings.max_fees_per_gas.fee_per_da_gas = 3; + builder.rollup_data.global_variables.gas_fees.fee_per_da_gas = 4; + + builder.validate_with_rollup_data(); +} + +#[test(should_fail_with = "l2 gas is higher than the maximum specified by the tx")] +fn validate_with_rollup_data_not_enough_fee_per_l2_gas_fails() { + let mut builder = PrivateTubeDataValidatorBuilder::new(); + + builder.tube_data.tx_context.gas_settings.max_fees_per_gas.fee_per_l2_gas = 3; + builder.rollup_data.global_variables.gas_fees.fee_per_l2_gas = 4; + + builder.validate_with_rollup_data(); +} diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/avm_circuit_public_inputs.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/avm_circuit_public_inputs.nr index 0352937e504..88fe770eb0d 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/avm_circuit_public_inputs.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/avm_circuit_public_inputs.nr @@ -214,7 +214,7 @@ impl AvmProofData { ); let mut result: [Field; 4] = [input_hash, 0, 0, 0]; - for i in 0..DUMMY_AVM_VERIFIER_NUM_ITERATIONS { + for _i in 0..DUMMY_AVM_VERIFIER_NUM_ITERATIONS { result = poseidon2_permutation(result, 4); } diff --git a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/constant_rollup_data.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/constant_rollup_data.nr similarity index 99% rename from noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/constant_rollup_data.nr rename to noir-projects/noir-protocol-circuits/crates/types/src/abis/constant_rollup_data.nr index 9eab56a2092..0a8d6919164 100644 --- a/noir-projects/noir-protocol-circuits/crates/rollup-lib/src/abis/constant_rollup_data.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/constant_rollup_data.nr @@ -1,4 +1,4 @@ -use dep::types::{ +use crate::{ abis::{append_only_tree_snapshot::AppendOnlyTreeSnapshot, global_variables::GlobalVariables}, constants::CONSTANT_ROLLUP_DATA_LENGTH, traits::{Deserialize, Empty, Serialize}, diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/mod.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/mod.nr index f1bce9e245c..ff429eefad6 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/mod.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/mod.nr @@ -13,6 +13,7 @@ pub mod nullifier_leaf_preimage; pub mod tx_constant_data; pub mod combined_constant_data; +pub mod constant_rollup_data; pub mod side_effect; pub mod read_request; diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel/private_call_data.nr b/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel/private_call_data.nr index 0d887f71f42..37c1b1dca94 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel/private_call_data.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/abis/private_kernel/private_call_data.nr @@ -25,7 +25,7 @@ pub struct PrivateCallData { } impl PrivateCallData { - fn verify(self, is_first_app: bool) { + pub fn verify(self, is_first_app: bool) { let proof_type = if is_first_app { PROOF_TYPE_OINK } else { diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/tests/fixture_builder.nr b/noir-projects/noir-protocol-circuits/crates/types/src/tests/fixture_builder.nr index dc3940d0557..404bd4cbc7e 100644 --- a/noir-projects/noir-protocol-circuits/crates/types/src/tests/fixture_builder.nr +++ b/noir-projects/noir-protocol-circuits/crates/types/src/tests/fixture_builder.nr @@ -4,9 +4,11 @@ use crate::{ avm_accumulated_data::AvmAccumulatedData, CombinedAccumulatedData, PrivateAccumulatedData, PrivateAccumulatedDataBuilder, PrivateToPublicAccumulatedData, }, + append_only_tree_snapshot::AppendOnlyTreeSnapshot, avm_circuit_public_inputs::AvmProofData, call_context::CallContext, combined_constant_data::CombinedConstantData, + constant_rollup_data::ConstantRollupData, function_data::FunctionData, gas::Gas, gas_settings::GasSettings, @@ -157,6 +159,9 @@ pub struct FixtureBuilder { pub protocol_contract_tree_root: Field, pub protocol_contract_sibling_path: [Field; PROTOCOL_CONTRACT_TREE_HEIGHT], + // Tree snapshots. + pub archive_tree: AppendOnlyTreeSnapshot, + // Counters. pub min_revertible_side_effect_counter: u32, pub counter_start: u32, @@ -216,6 +221,10 @@ impl FixtureBuilder { *self } + pub fn vk_tree_root() -> Field { + fixtures::vk_tree::get_vk_merkle_tree().get_root() + } + pub fn set_protocol_contract_root(&mut self) { let tree = fixtures::protocol_contract_tree::get_protocol_contract_tree(); self.protocol_contract_tree_root = tree.get_root(); @@ -289,6 +298,15 @@ impl FixtureBuilder { } } + pub fn to_constant_rollup_data(self) -> ConstantRollupData { + ConstantRollupData { + last_archive: self.archive_tree, + vk_tree_root: self.vk_tree_root, + protocol_contract_tree_root: self.protocol_contract_tree_root, + global_variables: self.global_variables, + } + } + pub fn build_tx_request(self) -> TxRequest { TxRequest { origin: self.contract_address, @@ -525,6 +543,38 @@ impl FixtureBuilder { } } + pub fn to_private_tube_data(self) -> PrivateTubeData { + let mut result: PrivateTubeData = std::mem::zeroed(); + result.public_inputs = self.to_kernel_circuit_public_inputs(); + result + } + + pub fn to_public_tube_data(self) -> PublicTubeData { + let mut result: PublicTubeData = std::mem::zeroed(); + result.public_inputs = self.to_private_to_public_kernel_circuit_public_inputs(true); + result + } + + pub fn to_avm_accumulated_data(self) -> AvmAccumulatedData { + AvmAccumulatedData { + note_hashes: self.note_hashes.storage().map(|n: ScopedNoteHash| n.note_hash.value), + nullifiers: self.nullifiers.storage().map(|n: ScopedNullifier| n.nullifier.value), + l2_to_l1_msgs: self.l2_to_l1_msgs.storage(), + unencrypted_logs_hashes: self.unencrypted_logs_hashes.storage(), + public_data_writes: self.public_data_writes.storage(), + } + } + + pub fn to_avm_proof_data(self, reverted: bool) -> AvmProofData { + let mut result: AvmProofData = std::mem::zeroed(); + + result.public_inputs.reverted = reverted; + result.public_inputs.global_variables = self.global_variables; + result.public_inputs.accumulated_data = self.to_avm_accumulated_data(); + + result + } + pub fn add_new_note_hash(&mut self, value: Field) { self.note_hashes.push(NoteHash { value, counter: self.next_counter() }.scope( self.contract_address, @@ -1037,42 +1087,6 @@ impl FixtureBuilder { self.counter += 1; counter } - - fn vk_tree_root() -> Field { - fixtures::vk_tree::get_vk_merkle_tree().get_root() - } - - pub fn to_private_tube_data(self) -> PrivateTubeData { - let mut result: PrivateTubeData = std::mem::zeroed(); - result.public_inputs = self.to_kernel_circuit_public_inputs(); - result - } - - pub fn to_public_tube_data(self) -> PublicTubeData { - let mut result: PublicTubeData = std::mem::zeroed(); - result.public_inputs = self.to_private_to_public_kernel_circuit_public_inputs(true); - result - } - - pub fn to_avm_accumulated_data(self) -> AvmAccumulatedData { - AvmAccumulatedData { - note_hashes: self.note_hashes.storage().map(|n: ScopedNoteHash| n.note_hash.value), - nullifiers: self.nullifiers.storage().map(|n: ScopedNullifier| n.nullifier.value), - l2_to_l1_msgs: self.l2_to_l1_msgs.storage(), - unencrypted_logs_hashes: self.unencrypted_logs_hashes.storage(), - public_data_writes: self.public_data_writes.storage(), - } - } - - pub fn to_avm_proof_data(self, reverted: bool) -> AvmProofData { - let mut result: AvmProofData = std::mem::zeroed(); - - result.public_inputs.reverted = reverted; - result.public_inputs.global_variables = self.global_variables; - result.public_inputs.accumulated_data = self.to_avm_accumulated_data(); - - result - } } impl Empty for FixtureBuilder { @@ -1123,6 +1137,7 @@ impl Empty for FixtureBuilder { vk_tree_root: FixtureBuilder::vk_tree_root(), protocol_contract_tree_root: 0, protocol_contract_sibling_path: [0; PROTOCOL_CONTRACT_TREE_HEIGHT], + archive_tree: AppendOnlyTreeSnapshot::zero(), revert_code: 0, min_revertible_side_effect_counter: 0, counter_start: 0, diff --git a/yarn-project/circuit-types/src/tx/validator/empty_validator.ts b/yarn-project/circuit-types/src/tx/validator/empty_validator.ts index 7ad6f80dcf1..2ea10e7a55a 100644 --- a/yarn-project/circuit-types/src/tx/validator/empty_validator.ts +++ b/yarn-project/circuit-types/src/tx/validator/empty_validator.ts @@ -1,8 +1,8 @@ import { type AnyTx, type TxValidator } from './tx_validator.js'; export class EmptyTxValidator implements TxValidator { - public validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[]]> { - return Promise.resolve([txs, []]); + public validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[], skippedTxs: T[]]> { + return Promise.resolve([txs, [], []]); } public validateTx(_tx: T): Promise { diff --git a/yarn-project/circuit-types/src/tx/validator/tx_validator.ts b/yarn-project/circuit-types/src/tx/validator/tx_validator.ts index 6669b56055a..040d764cf3d 100644 --- a/yarn-project/circuit-types/src/tx/validator/tx_validator.ts +++ b/yarn-project/circuit-types/src/tx/validator/tx_validator.ts @@ -5,5 +5,5 @@ export type AnyTx = Tx | ProcessedTx; export interface TxValidator { validateTx(tx: T): Promise; - validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[]]>; + validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[], skippedTxs?: T[]]>; } diff --git a/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.test.ts b/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.test.ts index c74a0fc5e16..bd315664445 100644 --- a/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.test.ts +++ b/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.test.ts @@ -6,24 +6,53 @@ describe('AggregateTxValidator', () => { it('allows txs that pass all validation', async () => { const txs = [mockTx(0), mockTx(1), mockTx(2), mockTx(3), mockTx(4)]; const agg = new AggregateTxValidator( - new TxDenyList(txs[0].getTxHash(), txs[1].getTxHash()), - new TxDenyList(txs[2].getTxHash(), txs[3].getTxHash()), + new TxDenyList([txs[0].getTxHash(), txs[1].getTxHash()], []), + new TxDenyList([txs[2].getTxHash(), txs[4].getTxHash()], []), ); - await expect(agg.validateTxs(txs)).resolves.toEqual([[txs[4]], [txs[0], txs[1], txs[2], txs[3]]]); + const validTxs = [txs[3]]; + const invalidTxs = [txs[0], txs[1], txs[2], txs[4]]; + const skippedTxs: AnyTx[] = []; + await expect(agg.validateTxs(txs)).resolves.toEqual([validTxs, invalidTxs, skippedTxs]); + }); + + it('aggregate skipped txs ', async () => { + const txs = [mockTx(0), mockTx(1), mockTx(2), mockTx(3), mockTx(4)]; + const agg = new AggregateTxValidator( + new TxDenyList([txs[0].getTxHash()], []), + new TxDenyList([], [txs[1].getTxHash(), txs[2].getTxHash()]), + new TxDenyList([], [txs[4].getTxHash()]), + ); + + const validTxs = [txs[3]]; + const invalidTxs = [txs[0]]; + const skippedTxs = [txs[1], txs[2], txs[4]]; + await expect(agg.validateTxs(txs)).resolves.toEqual([validTxs, invalidTxs, skippedTxs]); }); class TxDenyList implements TxValidator { denyList: Set; - constructor(...txHashes: TxHash[]) { - this.denyList = new Set(txHashes.map(hash => hash.toString())); + skippedList: Set; + constructor(deniedTxHashes: TxHash[], skippedTxHashes: TxHash[]) { + this.denyList = new Set(deniedTxHashes.map(hash => hash.toString())); + this.skippedList = new Set(skippedTxHashes.map(hash => hash.toString())); } - validateTxs(txs: AnyTx[]): Promise<[AnyTx[], AnyTx[]]> { - return Promise.resolve([ - txs.filter(tx => !this.denyList.has(Tx.getHash(tx).toString())), - txs.filter(tx => this.denyList.has(Tx.getHash(tx).toString())), - ]); + validateTxs(txs: AnyTx[]): Promise<[AnyTx[], AnyTx[], AnyTx[] | undefined]> { + const validTxs: AnyTx[] = []; + const invalidTxs: AnyTx[] = []; + const skippedTxs: AnyTx[] = []; + txs.forEach(tx => { + const txHash = Tx.getHash(tx).toString(); + if (this.skippedList.has(txHash)) { + skippedTxs.push(tx); + } else if (this.denyList.has(txHash)) { + invalidTxs.push(tx); + } else { + validTxs.push(tx); + } + }); + return Promise.resolve([validTxs, invalidTxs, skippedTxs.length ? skippedTxs : undefined]); } validateTx(tx: AnyTx): Promise { diff --git a/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.ts b/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.ts index 99dfb6c282d..21bf24ddb8d 100644 --- a/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.ts +++ b/yarn-project/p2p/src/tx_validator/aggregate_tx_validator.ts @@ -10,16 +10,18 @@ export class AggregateTxValidator implements TxValid this.#validators = validators; } - async validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[]]> { + async validateTxs(txs: T[]): Promise<[validTxs: T[], invalidTxs: T[], skippedTxs: T[]]> { const invalidTxs: T[] = []; + const skippedTxs: T[] = []; let txPool = txs; for (const validator of this.#validators) { - const [valid, invalid] = await validator.validateTxs(txPool); + const [valid, invalid, skipped] = await validator.validateTxs(txPool); invalidTxs.push(...invalid); + skippedTxs.push(...(skipped ?? [])); txPool = valid; } - return [txPool, invalidTxs]; + return [txPool, invalidTxs, skippedTxs]; } async validateTx(tx: T): Promise { diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 5b7943d7025..72498b7e86c 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -26,6 +26,7 @@ import { EthAddress, Fr, GasFees, + type GasSettings, GlobalVariables, NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP, } from '@aztec/circuits.js'; @@ -390,6 +391,47 @@ describe('sequencer', () => { expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), validTxHashes, undefined); }); + it('builds a block out of several txs skipping the ones not providing enough fee per gas', async () => { + const gasFees = new GasFees(10, 20); + mockedGlobalVariables.gasFees = gasFees; + + const txs = Array(5) + .fill(0) + .map((_, i) => mockTxForRollup(0x10000 * i)); + + const skippedTxIndexes = [1, 2]; + const validTxHashes: TxHash[] = []; + txs.forEach((tx, i) => { + tx.data.constants.txContext.chainId = chainId; + const maxFeesPerGas: Writeable = gasFees.clone(); + const feeToAdjust = i % 2 ? 'feePerDaGas' : 'feePerL2Gas'; + if (skippedTxIndexes.includes(i)) { + // maxFeesPerGas is less than gasFees. + maxFeesPerGas[feeToAdjust] = maxFeesPerGas[feeToAdjust].sub(new Fr(i + 1)); + } else { + // maxFeesPerGas is greater than or equal to gasFees. + maxFeesPerGas[feeToAdjust] = maxFeesPerGas[feeToAdjust].add(new Fr(i)); + validTxHashes.push(tx.getTxHash()); + } + (tx.data.constants.txContext.gasSettings as Writeable).maxFeesPerGas = maxFeesPerGas; + }); + + p2p.getPendingTxs.mockResolvedValueOnce(txs); + blockBuilder.setBlockCompleted.mockResolvedValue(block); + publisher.proposeL2Block.mockResolvedValueOnce(true); + globalVariableBuilder.buildGlobalVariables.mockResolvedValueOnce(mockedGlobalVariables); + + await sequencer.doRealWork(); + + expect(blockBuilder.startNewBlock).toHaveBeenCalledWith( + mockedGlobalVariables, + Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n)), + ); + expect(publisher.proposeL2Block).toHaveBeenCalledWith(block, getSignatures(), validTxHashes, undefined); + // The txs are not included. But they are not dropped from the pool either. + expect(p2p.deleteTxs).not.toHaveBeenCalled(); + }); + it('builds a block once it reaches the minimum number of transactions', async () => { const txs = times(8, i => { const tx = mockTxForRollup(i * 0x10000); diff --git a/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts b/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts index 95210a1b69a..eec83ad034f 100644 --- a/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts +++ b/yarn-project/sequencer-client/src/tx_validator/gas_validator.test.ts @@ -1,6 +1,7 @@ import { type Tx, mockTx } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, GasFees, GasSettings, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; import { poseidon2Hash } from '@aztec/foundation/crypto'; +import { type Writeable } from '@aztec/foundation/types'; import { FeeJuiceContract } from '@aztec/noir-contracts.js'; import { ProtocolContractAddress } from '@aztec/protocol-contracts'; @@ -10,28 +11,28 @@ import { GasTxValidator, type PublicStateSource } from './gas_validator.js'; import { patchNonRevertibleFn, patchRevertibleFn } from './test_utils.js'; describe('GasTxValidator', () => { - let validator: GasTxValidator; + // Vars for validator. let publicStateSource: MockProxy; let feeJuiceAddress: AztecAddress; - - beforeEach(() => { - feeJuiceAddress = ProtocolContractAddress.FeeJuice; - publicStateSource = mock({ - storageRead: mockFn().mockImplementation((_address: AztecAddress, _slot: Fr) => Fr.ZERO), - }); - - validator = new GasTxValidator(publicStateSource, feeJuiceAddress, false); - }); - + let enforceFees: boolean; + let gasFees: Writeable; + // Vars for tx. let tx: Tx; let payer: AztecAddress; let expectedBalanceSlot: Fr; let feeLimit: bigint; beforeEach(() => { + publicStateSource = mock({ + storageRead: mockFn().mockImplementation((_address: AztecAddress, _slot: Fr) => Fr.ZERO), + }); + feeJuiceAddress = ProtocolContractAddress.FeeJuice; + enforceFees = false; + gasFees = new GasFees(11, 22); + tx = mockTx(1, { numberOfNonRevertiblePublicCallRequests: 2 }); tx.data.feePayer = AztecAddress.random(); - tx.data.constants.txContext.gasSettings = GasSettings.default({ maxFeesPerGas: new GasFees(10, 10) }); + tx.data.constants.txContext.gasSettings = GasSettings.default({ maxFeesPerGas: gasFees.clone() }); payer = tx.data.feePayer; expectedBalanceSlot = poseidon2Hash([FeeJuiceContract.storage.balances.slot, payer]); feeLimit = tx.data.constants.txContext.gasSettings.getFeeLimit().toBigInt(); @@ -43,21 +44,29 @@ describe('GasTxValidator', () => { ); }; - const expectValidateSuccess = async (tx: Tx) => { - const result = await validator.validateTxs([tx]); - expect(result[0].length).toEqual(1); - expect(result).toEqual([[tx], []]); + const validateTx = async (tx: Tx) => { + const validator = new GasTxValidator(publicStateSource, feeJuiceAddress, enforceFees, gasFees); + return await validator.validateTxs([tx]); }; - const expectValidateFail = async (tx: Tx) => { - const result = await validator.validateTxs([tx]); - expect(result[1].length).toEqual(1); - expect(result).toEqual([[], [tx]]); + const expectValid = async (tx: Tx) => { + const result = await validateTx(tx); + expect(result).toEqual([[tx], [], []]); + }; + + const expectInvalid = async (tx: Tx) => { + const result = await validateTx(tx); + expect(result).toEqual([[], [tx], []]); + }; + + const expectSkipped = async (tx: Tx) => { + const result = await validateTx(tx); + expect(result).toEqual([[], [], [tx]]); }; it('allows fee paying txs if fee payer has enough balance', async () => { mockBalance(feeLimit); - await expectValidateSuccess(tx); + await expectValid(tx); }); it('allows fee paying txs if fee payer claims enough balance during setup', async () => { @@ -69,16 +78,16 @@ describe('GasTxValidator', () => { args: [selector.toField(), payer.toField(), new Fr(1n)], msgSender: ProtocolContractAddress.FeeJuice, }); - await expectValidateSuccess(tx); + await expectValid(tx); }); it('rejects txs if fee payer has not enough balance', async () => { mockBalance(feeLimit - 1n); - await expectValidateFail(tx); + await expectInvalid(tx); }); it('rejects txs if fee payer has zero balance', async () => { - await expectValidateFail(tx); + await expectInvalid(tx); }); it('rejects txs if fee payer claims balance outside setup', async () => { @@ -87,17 +96,27 @@ describe('GasTxValidator', () => { selector: FunctionSelector.fromSignature('_increase_public_balance((Field),Field)'), args: [payer.toField(), new Fr(1n)], }); - await expectValidateFail(tx); + await expectInvalid(tx); }); it('allows txs with no fee payer if fees are not enforced', async () => { tx.data.feePayer = AztecAddress.ZERO; - await expectValidateSuccess(tx); + await expectValid(tx); }); it('rejects txs with no fee payer if fees are enforced', async () => { - validator.enforceFees = true; + enforceFees = true; tx.data.feePayer = AztecAddress.ZERO; - await expectValidateFail(tx); + await expectInvalid(tx); + }); + + it('skips txs with not enough fee per da gas', async () => { + gasFees.feePerDaGas = gasFees.feePerDaGas.add(new Fr(1)); + await expectSkipped(tx); + }); + + it('skips txs with not enough fee per l2 gas', async () => { + gasFees.feePerL2Gas = gasFees.feePerL2Gas.add(new Fr(1)); + await expectSkipped(tx); }); }); diff --git a/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts b/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts index 19ba9c2a79e..8b4167543c9 100644 --- a/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts +++ b/yarn-project/sequencer-client/src/tx_validator/gas_validator.ts @@ -1,5 +1,5 @@ import { type Tx, TxExecutionPhase, type TxValidator } from '@aztec/circuit-types'; -import { type AztecAddress, type Fr, FunctionSelector } from '@aztec/circuits.js'; +import { type AztecAddress, type Fr, FunctionSelector, type GasFees } from '@aztec/circuits.js'; import { createLogger } from '@aztec/foundation/log'; import { computeFeePayerBalanceStorageSlot, getExecutionRequestsByPhase } from '@aztec/simulator'; @@ -12,36 +12,62 @@ export class GasTxValidator implements TxValidator { #log = createLogger('sequencer:tx_validator:tx_gas'); #publicDataSource: PublicStateSource; #feeJuiceAddress: AztecAddress; + #enforceFees: boolean; + #gasFees: GasFees; - constructor(publicDataSource: PublicStateSource, feeJuiceAddress: AztecAddress, public enforceFees: boolean) { + constructor( + publicDataSource: PublicStateSource, + feeJuiceAddress: AztecAddress, + enforceFees: boolean, + gasFees: GasFees, + ) { this.#publicDataSource = publicDataSource; this.#feeJuiceAddress = feeJuiceAddress; + this.#enforceFees = enforceFees; + this.#gasFees = gasFees; } - async validateTxs(txs: Tx[]): Promise<[validTxs: Tx[], invalidTxs: Tx[]]> { + async validateTxs(txs: Tx[]): Promise<[validTxs: Tx[], invalidTxs: Tx[], skippedTxs: Tx[]]> { const validTxs: Tx[] = []; const invalidTxs: Tx[] = []; + const skippedTxs: Tx[] = []; for (const tx of txs) { - if (await this.#validateTxFee(tx)) { + if (this.#shouldSkip(tx)) { + skippedTxs.push(tx); + } else if (await this.#validateTxFee(tx)) { validTxs.push(tx); } else { invalidTxs.push(tx); } } - return [validTxs, invalidTxs]; + return [validTxs, invalidTxs, skippedTxs]; } validateTx(tx: Tx): Promise { return this.#validateTxFee(tx); } + #shouldSkip(tx: Tx): boolean { + const gasSettings = tx.data.constants.txContext.gasSettings; + + // Skip the tx if its max fees are not enough for the current block's gas fees. + const maxFeesPerGas = gasSettings.maxFeesPerGas; + const notEnoughMaxFees = + maxFeesPerGas.feePerDaGas.lt(this.#gasFees.feePerDaGas) || + maxFeesPerGas.feePerL2Gas.lt(this.#gasFees.feePerL2Gas); + if (notEnoughMaxFees) { + this.#log.warn(`Skipping transaction ${tx.getTxHash()} due to insufficient fee per gas`); + } + return notEnoughMaxFees; + } + async #validateTxFee(tx: Tx): Promise { const feePayer = tx.data.feePayer; // TODO(@spalladino) Eventually remove the is_zero condition as we should always charge fees to every tx if (feePayer.isZero()) { - if (this.enforceFees) { + if (this.#enforceFees) { this.#log.warn(`Rejecting transaction ${tx.getTxHash()} due to missing fee payer`); } else { return true; diff --git a/yarn-project/sequencer-client/src/tx_validator/tx_validator_factory.ts b/yarn-project/sequencer-client/src/tx_validator/tx_validator_factory.ts index dd36e4ebc3d..a3647b2d710 100644 --- a/yarn-project/sequencer-client/src/tx_validator/tx_validator_factory.ts +++ b/yarn-project/sequencer-client/src/tx_validator/tx_validator_factory.ts @@ -46,7 +46,12 @@ export class TxValidatorFactory { new MetadataTxValidator(globalVariables.chainId, globalVariables.blockNumber), new DoubleSpendTxValidator(this.nullifierSource), new PhasesTxValidator(this.contractDataSource, setupAllowList), - new GasTxValidator(this.publicStateSource, ProtocolContractAddress.FeeJuice, this.enforceFees), + new GasTxValidator( + this.publicStateSource, + ProtocolContractAddress.FeeJuice, + this.enforceFees, + globalVariables.gasFees, + ), ); }