[go: up one dir, main page]

Rollup/DAL: remove useless calls to compute an already given published level

What

Once upon a time, slots were indexed and addressed by their commitment hashes on the DAL node side. So, functions like get_slot_pages take a commitment as an argument to return the pages of the slot. The function download_confirmed_slot_pages#very_old below takes a published_level and a slot index. Then, it retrieves the corresponding slot header to use the commitment inside it to fetch the pages.

The store on the DAL node was reworked 1.5 years ago (2024-04-12). Commit 227c6ae5 introduced direct indexation by slot IDs. But, the function download_confirmed_slot_pages#old below still retrieves the slot header given a published level to finally build a slot ID using the retrieved ... published level.

This MR simplifies the implementation as proposed in the function download_confirmed_slot_pages#new below

Why

  • We already have the published level. So it's pointless to retrieve the same info using two indirections
  • The rollup node can then avoid storing DAL slot headers published to L1 (to be done in a separate MR, cc @mebsout)

How

Use directly the published level provided to download_confirmed_slot_pages to call get_slot_pages

Very old vs Old vs New code

Very old variant of the function:

# very_old
let download_confirmed_slot_pages ({Node_context.dal_cctxt; _} as node_ctxt)
    ~published_level ~index =
  let open Lwt_result_syntax in
  let* published_in_block_hash =
    Node_context.hash_of_level node_ctxt (Raw_level.to_int32 published_level)
  in
  let* {commitment; _} =
    Node_context.get_slot_header node_ctxt ~published_in_block_hash index
  in
  let dal_cctxt = WithExceptions.Option.get ~loc:__LOC__ dal_cctxt in
  (* DAL must be configured for this point to be reached *)
  get_slot_pages dal_cctxt commitment

Old variant of the function:

# Old
let download_confirmed_slot_pages ({Node_context.dal_cctxt; _} as node_ctxt)
    ~published_level ~index =
  let open Lwt_result_syntax in
  let* published_in_block_hash =
    Node_context.hash_of_level node_ctxt (Raw_level.to_int32 published_level)
  in
  let* header =
    Node_context.get_slot_header node_ctxt ~published_in_block_hash index
  in
  let dal_cctxt = WithExceptions.Option.get ~loc:__LOC__ dal_cctxt in
  (* DAL must be configured for this point to be reached *)
  let slot_id : Slot_id.t =
    {slot_level = header.id.published_level; slot_index = header.id.index}
  in
  get_slot_pages dal_cctxt slot_id

Proposed simplification

# new

let download_confirmed_slot_pages {Node_context.dal_cctxt; _} ~published_level
    ~index =
  let dal_cctxt = WithExceptions.Option.get ~loc:__LOC__ dal_cctxt in
  (* DAL must be configured for this point to be reached *)
  get_slot_pages
    dal_cctxt
    {slot_level = Raw_level.to_int32 published_level; slot_index = index}

Which could be simplified further as done in this MR.

Moreover, the info accessed with get_slot_header is not used anymore

Edited by Mohamed IGUERNLALA

Merge request reports

Loading