Free finalized Ethereum updates #159

claravanstaden · 2024-07-18T13:43:49Z

Allows free Ethereum finalized header updates if they larger or equal to the last interval within a certain interval.

Adds pallet config FreeHeadersInterval, to be set to 32.
All beacon headers equal to or more than 32 slots newer than the latest imported header will be free.
All valid sync committee updates are free.
Any extrinsic resulting in an error condition will be a paid transaction.

Resolves: https://linear.app/snowfork/issue/SNO-1053

alistair-singh · 2024-07-24T12:20:51Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

@@ -281,10 +285,9 @@ pub mod pallet {
 			Ok(())
 		}

-		pub(crate) fn process_update(update: &Update) -> DispatchResult {
+		pub(crate) fn process_update(update: &Update) -> DispatchResultWithPostInfo {


Non-blocking: Do you think this function is even necessary?

Lol maybe not! I think its just to much the Ethereum consensus spec, which has a pattern of a "process" method, that does a "verify" and an "apply".

alistair-singh · 2024-07-24T22:49:46Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

+			// If free headers are allowed and the latest finalized header is larger than the
+			// minimum slot interval, the header import transaction is free.
+			if let Some(free_headers_interval) = T::FreeHeadersInterval::get() {
+				if improved_by_slot >= latest_slot + free_headers_interval as u64 {


So this is basically saying you get a free update if the update is far enough in the future?

Yeah, seems follow the same pattern for the P->K bridge here with free update at a minimal interval.

Exactly. It will be set to every 32 slots (so basically all the finalized updates), but if we ever need to make it less frequent we can.

yrong · 2024-07-29T11:28:31Z

Maybe we can add a runtime test checking the balance of the relayer before and after the extrinsic, make sure that there is no fee charged actually.

IIUC set pays_fee flag to false does not mean it won't charge for the extrinsic, just the fee will be refunded to the caller. But I may not wrong here.

yrong

+1

claravanstaden · 2024-07-30T12:13:15Z

Maybe we can add a runtime test checking the balance of the relayer before and after the extrinsic, make sure that there is no fee charged actually.

IIUC set pays_fee flag to false does not mean it won't charge for the extrinsic, just the fee will be refunded to the caller. But I may not wrong here.

@yrong that's a great suggestion, thanks. Added in c82647f.

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

vgeddes · 2024-07-30T12:34:50Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

@@ -83,6 +86,8 @@ pub mod pallet {
 		type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 		#[pallet::constant]
 		type ForkVersions: Get<ForkVersions>;
+		#[pallet::constant]
+		type FreeHeadersInterval: Get<Option<u32>>;


Why does this need to be optional? Can't we just make free headers the default?

vgeddes · 2024-07-30T12:42:13Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

+			if sync_committee_updated {
+				return Pays::No;
+			}


This effectively is just returning the parameter of sync_committee_updated of mapped to Pays, which seems redundant.

And given my other comment above, I would advise restructuring as follows:

fn check_refundable(update: &Update, latest_slot: u64) -> Pays

vgeddes · 2024-07-30T12:47:31Z

bridges/snowbridge/pallets/ethereum-client/src/lib.rs

@@ -464,13 +469,24 @@ pub mod pallet {
 				Self::deposit_event(Event::SyncCommitteeUpdated {
 					period: update_finalized_period,
 				});
+				sync_committee_updated = true;


This sync_committee_updated flag seems redundant. Following the code, when sync_committee_updated=True, it follows that update.next_sync_committee_update == Some(...)

claravanstaden · 2024-07-30T18:13:25Z

@vgeddes thanks for the suggestions, fixed in e3281f8.

* free finalized updates * update comment * fix check * fixes * tests * fixes * fmt * finishing touches * comments * adds missing impl * adds test * fixes * fix

Allow free Snowbridge consensus updates, if the header interval is larger than the configured value (set to 32, so once a epoch). This PR also moves the Rococo Snowbridge pallet config into its own module. Original PR: Snowfork#159 --------- Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>

Allow free Snowbridge consensus updates, if the header interval is larger than the configured value (set to 32, so once a epoch). This PR also moves the Rococo Snowbridge pallet config into its own module. Original PR: Snowfork#159 ---------- Original `pr_5201.prdoc` is present but moved to release dir, ergo `R0-Silent` for this backport PR. --------- Co-authored-by: Clara van Staden <claravanstaden64@gmail.com> Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>

Allow free Snowbridge consensus updates, if the header interval is larger than the configured value (set to 32, so once a epoch). This PR also moves the Rococo Snowbridge pallet config into its own module. Original PR: Snowfork#159 --------- Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>

claravanstaden added 10 commits July 18, 2024 13:09

free finalized updates

bbfcb8e

update comment

839c392

fix check

b13c057

fixes

7030318

tests

a18b81b

fixes

63bb960

fmt

e01bc31

finishing touches

4aee593

comments

170cdbc

adds missing impl

2483147

claravanstaden marked this pull request as ready for review July 18, 2024 18:38

claravanstaden requested review from vgeddes, alistair-singh and yrong July 18, 2024 18:39

alistair-singh approved these changes Jul 24, 2024

View reviewed changes

yrong approved these changes Jul 30, 2024

View reviewed changes

claravanstaden added 2 commits July 30, 2024 11:56

Merge branch 'snowbridge' into free-finalized-updates

76d494f

adds test

c82647f

vgeddes reviewed Jul 30, 2024

View reviewed changes

claravanstaden added 2 commits July 30, 2024 16:06

fixes

e3281f8

fix

d034cb8

claravanstaden requested a review from vgeddes July 30, 2024 18:13

vgeddes approved these changes Jul 31, 2024

View reviewed changes

claravanstaden merged commit c7cbd99 into snowbridge Jul 31, 2024
8 checks passed

claravanstaden deleted the free-finalized-updates branch July 31, 2024 12:30

claravanstaden added a commit that referenced this pull request Jul 31, 2024

Free finalized Ethereum updates (#159)

5dde91c

* free finalized updates * update comment * fix check * fixes * tests * fixes * fmt * finishing touches * comments * adds missing impl * adds test * fixes * fix

claravanstaden mentioned this pull request Jul 31, 2024

Snowbridge free consensus updates paritytech/polkadot-sdk#5201

Merged

acatangiu mentioned this pull request Sep 3, 2024

[backport] Snowbridge free consensus updates (#5201) paritytech/polkadot-sdk#5560

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Free finalized Ethereum updates #159

Free finalized Ethereum updates #159

claravanstaden commented Jul 18, 2024 •

edited

Loading

alistair-singh Jul 24, 2024

claravanstaden Jul 29, 2024

alistair-singh Jul 24, 2024

yrong Jul 29, 2024

claravanstaden Jul 29, 2024

yrong commented Jul 29, 2024

yrong left a comment

claravanstaden commented Jul 30, 2024

vgeddes Jul 30, 2024

vgeddes Jul 30, 2024

vgeddes Jul 30, 2024

claravanstaden commented Jul 30, 2024

Free finalized Ethereum updates #159

Free finalized Ethereum updates #159

Conversation

claravanstaden commented Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrong commented Jul 29, 2024

yrong left a comment

Choose a reason for hiding this comment

claravanstaden commented Jul 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claravanstaden commented Jul 30, 2024

claravanstaden commented Jul 18, 2024 •

edited

Loading