From 7d4e37ee9c8a2e6a7cbc6c4cff4c2c5a8273a7ca Mon Sep 17 00:00:00 2001 From: Luciano Freitas Date: Thu, 31 Jul 2025 12:29:49 +0200 Subject: [PATCH 1/5] Tezlink/Kernel/Execution: extract burn_fees from validate_and_apply_operation --- .../kernel_latest/tezos_execution/src/lib.rs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/etherlink/kernel_latest/tezos_execution/src/lib.rs b/etherlink/kernel_latest/tezos_execution/src/lib.rs index 8207384089c7..ad82b790f6d1 100644 --- a/etherlink/kernel_latest/tezos_execution/src/lib.rs +++ b/etherlink/kernel_latest/tezos_execution/src/lib.rs @@ -452,6 +452,16 @@ fn execute_smart_contract<'a>( Ok((internal_operations, new_storage)) } +fn burn_fees( + host: &mut Host, + validation_info: &mut ValidationInfo, +) -> Result<(), ValidityError> { + validation_info + .source_account + .set_balance(host, &validation_info.new_source_balance) + .map_err(|_| ValidityError::FailedToUpdateBalance) +} + pub fn validate_and_apply_operation( host: &mut Host, context: &context::Context, @@ -491,17 +501,11 @@ pub fn validate_and_apply_operation( log!(safe_host, Debug, "Operation is valid"); log!(safe_host, Debug, "Updates balance to pay fees"); - if validation_info - .source_account - .set_balance(&mut safe_host, &validation_info.new_source_balance) - .is_err() - { + if let Err(validity_err) = burn_fees(&mut safe_host, &mut validation_info) { log!(safe_host, Debug, "Could not update balance!"); safe_host.revert()?; - return Err(OperationError::Validation( - ValidityError::FailedToUpdateBalance, - )); - }; + return Err(OperationError::Validation(validity_err)); + } safe_host.promote()?; safe_host.promote_trace()?; -- GitLab From 2eff05f4fda1e04222fd50723c02fc7dbff5eaef Mon Sep 17 00:00:00 2001 From: Luciano Freitas Date: Thu, 31 Jul 2025 14:04:40 +0200 Subject: [PATCH 2/5] Tezlink/Kernel/Execution: closes #8031 - increment counter upon fee burn Tezlink/Kernel/Transfer: move counter increment to pay_fees and fix broken counter tests - Invalid operations do not increment counter - Valid operations that produce an application error should increment the counter --- .../tezos/src/operation_result.rs | 6 ++-- .../kernel_latest/tezos_execution/src/lib.rs | 35 +++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/etherlink/kernel_latest/tezos/src/operation_result.rs b/etherlink/kernel_latest/tezos/src/operation_result.rs index 0a96626bae0b..7a828bb792d7 100644 --- a/etherlink/kernel_latest/tezos/src/operation_result.rs +++ b/etherlink/kernel_latest/tezos/src/operation_result.rs @@ -60,6 +60,8 @@ pub enum ValidityError { FailedToFetchBalance, #[error("Failed to update balance")] FailedToUpdateBalance, + #[error("Failed to increment counter.")] + FailedToIncrementCounter, } #[derive(Error, Debug, PartialEq, Eq, NomReader, BinWriter)] @@ -72,8 +74,6 @@ pub enum RevealError { InconsistentPublicKey(PublicKeyHash), #[error("Could not retrieve manager.")] UnretrievableManager, - #[error("Failed to increment counter.")] - FailedToIncrementCounter, #[error("Failed to write manager.")] FailedToWriteManager, } @@ -123,8 +123,6 @@ pub enum TransferError { FailedToUpdateDestinationBalance, #[error("An internal operation failed: {0}")] FailedToApplyInternalOperation(String), - #[error("Failed to increment counter")] - FailedToIncrementCounter, #[error("Apply operation failed because of an unsupported address error")] MirAddressUnsupportedError, } diff --git a/etherlink/kernel_latest/tezos_execution/src/lib.rs b/etherlink/kernel_latest/tezos_execution/src/lib.rs index ad82b790f6d1..c15d61f8bc06 100644 --- a/etherlink/kernel_latest/tezos_execution/src/lib.rs +++ b/etherlink/kernel_latest/tezos_execution/src/lib.rs @@ -68,10 +68,6 @@ fn reveal( account .set_manager_public_key(host, public_key) .map_err(|_| RevealError::FailedToWriteManager)?; - // TODO : Counter Increment should be done after successful validation (see issue #8031) - account - .increment_counter(host) - .map_err(|_| RevealError::FailedToIncrementCounter)?; log!(host, Debug, "Reveal operation succeed"); @@ -306,7 +302,7 @@ pub fn transfer_external( let mut ctx = Ctx::default(); ctx.source = address_from_contract(src_contract.clone()); - let success = transfer( + transfer( host, context, &src_contract, @@ -318,11 +314,7 @@ pub fn transfer_external( value, &parser, &mut ctx, - ); - src_account - .increment_counter(host) - .map_err(|_| TransferError::FailedToIncrementCounter)?; - success + ) } /// Prepares balance updates when accounting fees in the format expected by the Tezos operation. @@ -452,14 +444,19 @@ fn execute_smart_contract<'a>( Ok((internal_operations, new_storage)) } -fn burn_fees( +fn burn_fees_and_increment_counter( host: &mut Host, validation_info: &mut ValidationInfo, ) -> Result<(), ValidityError> { validation_info .source_account .set_balance(host, &validation_info.new_source_balance) - .map_err(|_| ValidityError::FailedToUpdateBalance) + .map_err(|_| ValidityError::FailedToUpdateBalance)?; + + validation_info + .source_account + .increment_counter(host) + .map_err(|_| ValidityError::FailedToIncrementCounter) } pub fn validate_and_apply_operation( @@ -501,8 +498,10 @@ pub fn validate_and_apply_operation( log!(safe_host, Debug, "Operation is valid"); log!(safe_host, Debug, "Updates balance to pay fees"); - if let Err(validity_err) = burn_fees(&mut safe_host, &mut validation_info) { - log!(safe_host, Debug, "Could not update balance!"); + + if let Err(validity_err) = + burn_fees_and_increment_counter(&mut safe_host, &mut validation_info) + { safe_host.revert()?; return Err(OperationError::Validation(validity_err)); } @@ -981,8 +980,8 @@ mod tests { assert_eq!(receipt, expected_receipt); assert_eq!( account.counter(&host).unwrap(), - 0.into(), - "Counter should not have been incremented" + 1.into(), + "Counter should have been incremented" ); } @@ -1534,8 +1533,8 @@ mod tests { assert_eq!(receipt, expected_receipt); assert_eq!( source.counter(&host).unwrap(), - 0.into(), - "Counter should not have been incremented" + 1.into(), + "Counter should have been incremented" ); } -- GitLab From 9d359f7fa988d6b8309a2f5e641736d8e8a4f16e Mon Sep 17 00:00:00 2001 From: Luciano Freitas Date: Mon, 4 Aug 2025 09:31:35 +0200 Subject: [PATCH 3/5] Tezlink/Kernel/Transfer: refacto execution of validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Raphaël Cauderlier --- .../kernel_latest/tezos_execution/src/lib.rs | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/etherlink/kernel_latest/tezos_execution/src/lib.rs b/etherlink/kernel_latest/tezos_execution/src/lib.rs index c15d61f8bc06..ac6df931d23d 100644 --- a/etherlink/kernel_latest/tezos_execution/src/lib.rs +++ b/etherlink/kernel_latest/tezos_execution/src/lib.rs @@ -444,10 +444,13 @@ fn execute_smart_contract<'a>( Ok((internal_operations, new_storage)) } -fn burn_fees_and_increment_counter( +fn execute_validation( host: &mut Host, - validation_info: &mut ValidationInfo, -) -> Result<(), ValidityError> { + context: &Context, + operation: &Operation, +) -> Result { + let mut validation_info = validate::validate_operation(host, context, operation)?; + validation_info .source_account .set_balance(host, &validation_info.new_source_balance) @@ -456,7 +459,9 @@ fn burn_fees_and_increment_counter( validation_info .source_account .increment_counter(host) - .map_err(|_| ValidityError::FailedToIncrementCounter) + .map_err(|_| ValidityError::FailedToIncrementCounter)?; + + Ok(validation_info) } pub fn validate_and_apply_operation( @@ -485,26 +490,13 @@ pub fn validate_and_apply_operation( log!(safe_host, Debug, "Verifying that the operation is valid"); - let mut validation_info = - match validate::validate_operation(&safe_host, context, &operation) { - Ok(validation_info) => validation_info, - Err(validity_err) => { - log!(safe_host, Debug, "Operation is invalid: {:?}", validity_err); - safe_host.revert()?; - return Err(OperationError::Validation(validity_err)); - } - }; - - log!(safe_host, Debug, "Operation is valid"); - - log!(safe_host, Debug, "Updates balance to pay fees"); - - if let Err(validity_err) = - burn_fees_and_increment_counter(&mut safe_host, &mut validation_info) - { - safe_host.revert()?; - return Err(OperationError::Validation(validity_err)); - } + let validation_info = match execute_validation(&mut safe_host, context, &operation) { + Ok(validation_info) => validation_info, + Err(validity_err) => { + safe_host.revert()?; + return Err(OperationError::Validation(validity_err)); + } + }; safe_host.promote()?; safe_host.promote_trace()?; -- GitLab From 2c1a3b01948e22adb50056d463f11b51bec894bf Mon Sep 17 00:00:00 2001 From: Luciano Freitas Date: Fri, 1 Aug 2025 12:38:40 +0200 Subject: [PATCH 4/5] Tezlink/Kernel/Transfer: check counter on all tests --- .../kernel_latest/tezos_execution/src/lib.rs | 72 +++++++++++++++++-- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/etherlink/kernel_latest/tezos_execution/src/lib.rs b/etherlink/kernel_latest/tezos_execution/src/lib.rs index ac6df931d23d..23c3b3d72222 100644 --- a/etherlink/kernel_latest/tezos_execution/src/lib.rs +++ b/etherlink/kernel_latest/tezos_execution/src/lib.rs @@ -800,7 +800,7 @@ mod tests { // to avoid getting an error when initializing the safe_storage let other = bootstrap2(); - init_account(&mut host, &other.pkh); + let src_account = init_account(&mut host, &other.pkh); let operation = make_reveal_operation(15, 1, 4, 5, source); @@ -813,6 +813,12 @@ mod tests { let expected_error = OperationError::Validation(ValidityError::EmptyImplicitContract); + assert_eq!( + src_account.counter(&host).unwrap(), + 0.into(), + "Counter should not have been incremented" + ); + assert_eq!(result, Err(expected_error)); } @@ -823,7 +829,7 @@ mod tests { let source = bootstrap1(); - let _ = init_account(&mut host, &source.pkh); + let src_account = init_account(&mut host, &source.pkh); // Fees are too high for source's balance let operation = make_reveal_operation(100, 1, 4, 5, source); @@ -837,6 +843,12 @@ mod tests { let expected_error = OperationError::Validation(ValidityError::CantPayFees(100_u64.into())); + assert_eq!( + src_account.counter(&host).unwrap(), + 0.into(), + "Counter should not have been incremented" + ); + assert_eq!(result, Err(expected_error)); } @@ -847,7 +859,7 @@ mod tests { let source = bootstrap1(); - let _ = init_account(&mut host, &source.pkh); + let src_account = init_account(&mut host, &source.pkh); // Counter is incoherent for source's counter let operation = make_reveal_operation(15, 15, 4, 5, source); @@ -863,6 +875,13 @@ mod tests { expected: 1_u64.into(), found: 15_u64.into(), })); + + assert_eq!( + src_account.counter(&host).unwrap(), + 0.into(), + "Counter should not have been incremented" + ); + assert_eq!(result, Err(expected_error)); } @@ -917,6 +936,13 @@ mod tests { ), internal_operation_results: vec![], }); + + assert_eq!( + account.counter(&host).unwrap(), + 1.into(), + "Counter should have been incremented" + ); + assert_eq!(receipt, expected_receipt); } @@ -991,7 +1017,7 @@ mod tests { let source = Bootstrap { pk, ..bootstrap1() }; // Even if we don't use it we need to init the account - let _ = init_account(&mut host, &source.pkh); + let account = init_account(&mut host, &source.pkh); let operation = make_reveal_operation(15, 1, 4, 5, source.clone()); @@ -1003,6 +1029,12 @@ mod tests { let expected_error = OperationError::Validation(ValidityError::InvalidSignature); + assert_eq!( + account.counter(&host).unwrap(), + 0.into(), + "Counter should not have been incremented" + ); + assert_eq!(result, Err(expected_error)); } @@ -1063,6 +1095,12 @@ mod tests { .expect("Read manager should have succeed"); assert_eq!(manager, Manager::Revealed(pk)); + + assert_eq!( + account.counter(&host).unwrap(), + 1.into(), + "Counter should have been incremented" + ); } // Test an invalid transfer operation, source has not enough balance to fullfil the Transfer @@ -1130,6 +1168,12 @@ mod tests { // Verify that source only paid the fees and the destination balance is unchanged assert_eq!(source_account.balance(&host).unwrap(), 35.into()); assert_eq!(destination_account.balance(&host).unwrap(), 50_u64.into()); + + assert_eq!( + source_account.counter(&host).unwrap(), + 1.into(), + "Counter should have been incremented" + ); } // Bootstrap 1 successfully transfer 30 mutez to Bootstrap 2 @@ -1293,6 +1337,13 @@ mod tests { assert_eq!(source.balance(&host).unwrap(), 35_u64.into()); assert_eq!(receipt, expected_receipt); + + // Verify that the source's counter has been incremented + assert_eq!( + source.counter(&host).unwrap(), + 1.into(), + "Counter should have been incremented" + ); } #[test] @@ -1356,6 +1407,12 @@ mod tests { requester.balance(&host).unwrap(), (requester_balance + requested_amount - fees).into() ); // The faucet should have transferred 100 mutez to the source + + assert_eq!( + requester.counter(&host).unwrap(), + 1.into(), + "Counter should have been incremented" + ); } #[test] @@ -1447,6 +1504,12 @@ mod tests { assert_eq!(destination.balance(&host).unwrap(), 80_u64.into()); assert_eq!(receipt, expected_receipt); + + assert_eq!( + source.counter(&host).unwrap(), + 1.into(), + "Counter should have been incremented" + ); } #[test] @@ -1523,6 +1586,7 @@ mod tests { assert_eq!(destination.balance(&host).unwrap(), 50_u64.into()); assert_eq!(receipt, expected_receipt); + assert_eq!( source.counter(&host).unwrap(), 1.into(), -- GitLab From c3c2d7aeeaabecc26e9a76d9068ad93a16b0795f Mon Sep 17 00:00:00 2001 From: Luciano Freitas Date: Thu, 31 Jul 2025 14:31:20 +0200 Subject: [PATCH 5/5] Tezlink/Kernel/Transfer: check read value for storage in smart contract tests --- etherlink/kernel_latest/tezos_execution/src/lib.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/etherlink/kernel_latest/tezos_execution/src/lib.rs b/etherlink/kernel_latest/tezos_execution/src/lib.rs index 23c3b3d72222..2ca96747cdeb 100644 --- a/etherlink/kernel_latest/tezos_execution/src/lib.rs +++ b/etherlink/kernel_latest/tezos_execution/src/lib.rs @@ -1456,7 +1456,7 @@ mod tests { "validate_and_apply_operation should not have failed with a kernel error", ); - let storage = Some(storage_value); + let storage = Some(storage_value.clone()); let expected_receipt = OperationResultSum::Transfer(OperationResult { balance_updates: vec![ @@ -1510,6 +1510,12 @@ mod tests { 1.into(), "Counter should have been incremented" ); + + assert_eq!( + destination.storage(&host).unwrap(), + storage_value, + "Storage has not been updated" + ) } #[test] @@ -1592,6 +1598,12 @@ mod tests { 1.into(), "Counter should have been incremented" ); + + assert_eq!( + destination.storage(&host).unwrap(), + parser.parse(initial_storage).unwrap().encode(), + "Storage should not have been updated" + ) } #[test] -- GitLab