From ef047800a9afb1b603d1bf247b0492e99119a820 Mon Sep 17 00:00:00 2001 From: Mateusz Nowakowski Date: Fri, 16 Feb 2024 08:54:48 +0100 Subject: [PATCH 1/2] L1Update message verification --- pallets/rolldown/src/lib.rs | 28 +++++++++++- pallets/rolldown/src/messages.rs | 1 + pallets/rolldown/src/tests.rs | 73 ++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/pallets/rolldown/src/lib.rs b/pallets/rolldown/src/lib.rs index 24eb15d067..31bb7499cb 100644 --- a/pallets/rolldown/src/lib.rs +++ b/pallets/rolldown/src/lib.rs @@ -6,7 +6,7 @@ use frame_support::{ StorageHasher, }; use frame_system::{ensure_signed, pallet_prelude::*}; -use messages::UpdateType; +use messages::{UpdateType, PendingRequestType}; use sp_runtime::traits::SaturatedConversion; use alloy_sol_types::SolValue; @@ -158,6 +158,8 @@ pub mod pallet { EmptyUpdate, AddressDeserializationFailure, RequestDoesNotExist, + TooManyRequests, + InvalidUpdate, } #[pallet::config] @@ -215,6 +217,30 @@ pub mod pallet { let sequencer = ensure_signed(origin)?; ensure!(!requests.order.is_empty(), Error::::EmptyUpdate); + ensure!(requests.order.len() <= 10, Error::::TooManyRequests); + + + let withdraws_count = requests.pendingWithdraws.len(); + let deposits_count = requests.pendingDeposits.len(); + let cancels_count = requests.pendingCancelResultions.len(); + let l2_updates_count = requests.pendingL2UpdatesToRemove.len(); + + ensure!( + requests.order.iter().filter(|e| **e == PendingRequestType::DEPOSIT).count() == deposits_count, + Error::::InvalidUpdate + ); + ensure!( + requests.order.iter().filter(|e| **e == PendingRequestType::WITHDRAWAL).count() == withdraws_count, + Error::::InvalidUpdate + ); + ensure!( + requests.order.iter().filter(|e| **e == PendingRequestType::CANCEL_RESOLUTION).count() == cancels_count, + Error::::InvalidUpdate + ); + ensure!( + requests.order.iter().filter(|e| **e == PendingRequestType::L2_UPDATES_TO_REMOVE).count() == l2_updates_count, + Error::::InvalidUpdate + ); // check json length to prevent big data spam, maybe not necessary as it will be checked later and slashed let current_block_number = diff --git a/pallets/rolldown/src/messages.rs b/pallets/rolldown/src/messages.rs index 8301d20dc6..de7f323f78 100644 --- a/pallets/rolldown/src/messages.rs +++ b/pallets/rolldown/src/messages.rs @@ -52,6 +52,7 @@ pub struct L1Update { pub pendingL2UpdatesToRemove: Vec, } +#[derive(Eq, PartialEq, RuntimeDebug, Clone, Encode, Decode, TypeInfo, Serialize)] pub enum L1UpdateRequest { Withdraw(Withdraw), Deposit(Deposit), diff --git a/pallets/rolldown/src/tests.rs b/pallets/rolldown/src/tests.rs index 5579152d9e..3936b09a5a 100644 --- a/pallets/rolldown/src/tests.rs +++ b/pallets/rolldown/src/tests.rs @@ -176,6 +176,36 @@ fn deposit_executed_after_dispute_period() { }); } +#[test] +#[serial] +fn each_request_executed_only_once() { + ExtBuilder::new() + .issue(ALICE, ETH_TOKEN_ADDRESS_MGX, 0u128) + .execute_with_default_mocks(|| { + forward_to_block::(10); + let update = create_l1_update(vec![L1UpdateRequest::Deposit(messages::Deposit { + depositRecipient: DummyAddressConverter::convert_back(CHARLIE), + tokenAddress: ETH_TOKEN_ADDRESS, + amount: sp_core::U256::from(MILLION), + })]); + Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), update.clone()).unwrap(); + + forward_to_block::(11); + Rolldown::update_l2_from_l1(RuntimeOrigin::signed(BOB), update).unwrap(); + + + forward_to_block::(14); + assert!(!pending_updates::::contains_key(sp_core::U256::from(0u128))); + assert_eq!(TokensOf::::free_balance(ETH_TOKEN_ADDRESS_MGX, &CHARLIE), 0_u128); + + forward_to_block::(15); + assert_eq!(TokensOf::::free_balance(ETH_TOKEN_ADDRESS_MGX, &CHARLIE), MILLION); + + forward_to_block::(20); + assert_eq!(TokensOf::::free_balance(ETH_TOKEN_ADDRESS_MGX, &CHARLIE), MILLION); + }); +} + #[test] #[serial] fn withdraw_executed_after_dispute_period() { @@ -336,6 +366,49 @@ fn cancel_request() { }); } +#[test] +#[serial] +fn reject_update_with_too_many_requests() { + ExtBuilder::new() + .execute_with_default_mocks(|| { + forward_to_block::(10); + + let requests = + vec![ + L1UpdateRequest::Withdraw(messages::Withdraw { + depositRecipient: ETH_RECIPIENT_ACCOUNT, + tokenAddress: ETH_TOKEN_ADDRESS, + amount: sp_core::U256::from(MILLION), + }); 11]; + + let withdraw_update = create_l1_update(requests); + + assert_err!( + Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), withdraw_update), + Error::::TooManyRequests + ); + }); +} + +#[test] +#[serial] +fn reject_update_with_missing_requests() { + ExtBuilder::new() + .execute_with_default_mocks(|| { + forward_to_block::(10); + + let update = L1Update { + order: vec![messages::PendingRequestType::DEPOSIT], + .. Default::default() + }; + + assert_err!( + Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), update), + Error::::InvalidUpdate + ); + }); +} + #[test] fn test_conversion_u256() { let val = sp_core::U256::from(1u8); From c28afa27e98dc5bcf96870e3be6ae306890f027b Mon Sep 17 00:00:00 2001 From: Mateusz Nowakowski Date: Fri, 16 Feb 2024 09:27:40 +0100 Subject: [PATCH 2/2] formatting --- pallets/rolldown/src/lib.rs | 21 ++++++++++---- pallets/rolldown/src/tests.rs | 54 ++++++++++++++++------------------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/pallets/rolldown/src/lib.rs b/pallets/rolldown/src/lib.rs index 31bb7499cb..7d17c8e2c1 100644 --- a/pallets/rolldown/src/lib.rs +++ b/pallets/rolldown/src/lib.rs @@ -6,7 +6,7 @@ use frame_support::{ StorageHasher, }; use frame_system::{ensure_signed, pallet_prelude::*}; -use messages::{UpdateType, PendingRequestType}; +use messages::{PendingRequestType, UpdateType}; use sp_runtime::traits::SaturatedConversion; use alloy_sol_types::SolValue; @@ -219,26 +219,35 @@ pub mod pallet { ensure!(!requests.order.is_empty(), Error::::EmptyUpdate); ensure!(requests.order.len() <= 10, Error::::TooManyRequests); - let withdraws_count = requests.pendingWithdraws.len(); let deposits_count = requests.pendingDeposits.len(); let cancels_count = requests.pendingCancelResultions.len(); let l2_updates_count = requests.pendingL2UpdatesToRemove.len(); ensure!( - requests.order.iter().filter(|e| **e == PendingRequestType::DEPOSIT).count() == deposits_count, + requests.order.iter().filter(|e| **e == PendingRequestType::DEPOSIT).count() == + deposits_count, Error::::InvalidUpdate ); ensure!( - requests.order.iter().filter(|e| **e == PendingRequestType::WITHDRAWAL).count() == withdraws_count, + requests.order.iter().filter(|e| **e == PendingRequestType::WITHDRAWAL).count() == + withdraws_count, Error::::InvalidUpdate ); ensure!( - requests.order.iter().filter(|e| **e == PendingRequestType::CANCEL_RESOLUTION).count() == cancels_count, + requests + .order + .iter() + .filter(|e| **e == PendingRequestType::CANCEL_RESOLUTION) + .count() == cancels_count, Error::::InvalidUpdate ); ensure!( - requests.order.iter().filter(|e| **e == PendingRequestType::L2_UPDATES_TO_REMOVE).count() == l2_updates_count, + requests + .order + .iter() + .filter(|e| **e == PendingRequestType::L2_UPDATES_TO_REMOVE) + .count() == l2_updates_count, Error::::InvalidUpdate ); diff --git a/pallets/rolldown/src/tests.rs b/pallets/rolldown/src/tests.rs index 3936b09a5a..c7958d7c83 100644 --- a/pallets/rolldown/src/tests.rs +++ b/pallets/rolldown/src/tests.rs @@ -193,7 +193,6 @@ fn each_request_executed_only_once() { forward_to_block::(11); Rolldown::update_l2_from_l1(RuntimeOrigin::signed(BOB), update).unwrap(); - forward_to_block::(14); assert!(!pending_updates::::contains_key(sp_core::U256::from(0u128))); assert_eq!(TokensOf::::free_balance(ETH_TOKEN_ADDRESS_MGX, &CHARLIE), 0_u128); @@ -369,44 +368,41 @@ fn cancel_request() { #[test] #[serial] fn reject_update_with_too_many_requests() { - ExtBuilder::new() - .execute_with_default_mocks(|| { - forward_to_block::(10); + ExtBuilder::new().execute_with_default_mocks(|| { + forward_to_block::(10); - let requests = - vec![ - L1UpdateRequest::Withdraw(messages::Withdraw { - depositRecipient: ETH_RECIPIENT_ACCOUNT, - tokenAddress: ETH_TOKEN_ADDRESS, - amount: sp_core::U256::from(MILLION), - }); 11]; + let requests = vec![ + L1UpdateRequest::Withdraw(messages::Withdraw { + depositRecipient: ETH_RECIPIENT_ACCOUNT, + tokenAddress: ETH_TOKEN_ADDRESS, + amount: sp_core::U256::from(MILLION), + }); + 11 + ]; - let withdraw_update = create_l1_update(requests); + let withdraw_update = create_l1_update(requests); - assert_err!( - Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), withdraw_update), - Error::::TooManyRequests - ); - }); + assert_err!( + Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), withdraw_update), + Error::::TooManyRequests + ); + }); } #[test] #[serial] fn reject_update_with_missing_requests() { - ExtBuilder::new() - .execute_with_default_mocks(|| { - forward_to_block::(10); + ExtBuilder::new().execute_with_default_mocks(|| { + forward_to_block::(10); - let update = L1Update { - order: vec![messages::PendingRequestType::DEPOSIT], - .. Default::default() - }; + let update = + L1Update { order: vec![messages::PendingRequestType::DEPOSIT], ..Default::default() }; - assert_err!( - Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), update), - Error::::InvalidUpdate - ); - }); + assert_err!( + Rolldown::update_l2_from_l1(RuntimeOrigin::signed(ALICE), update), + Error::::InvalidUpdate + ); + }); } #[test]