Skip to content

Change Bundle::receipts from Vec<Receipt> to Receipt#1528

Merged
nazar-pc merged 8 commits intomainfrom
bundle-receipt
Jun 12, 2023
Merged

Change Bundle::receipts from Vec<Receipt> to Receipt#1528
nazar-pc merged 8 commits intomainfrom
bundle-receipt

Conversation

@NingLin-P
Copy link
Copy Markdown
Contributor

close #1512

This PR change Bundle::receipts from Vec to Receipt, mostly refactoring with some notable changes in the client:

  • Skip domain slot if primary block number is zero, because the receipt for block 0 doesn't exist, previously we use an empty vec, and now since we changed Vec<Receipt> to Receipt, we can use a dummy receipt but it will bring a lot of special handling for this receipt thus I chose to simply skip the slot to make life easier.
  • Now we use the receipt at (head_receipt_number + One::one()).min(available_best_receipt_number) for bundle construction, it mostly matches our design except that in some case available_best_receipt_number may point to an older receipt than the current head receipt, that may required non-trivial work to handle, I have left a TODO for now.

Code contributor checklist:

NingLin-P added 4 commits June 9, 2023 18:45
Pure refactoring with some dead code removed

Signed-off-by: linning <linningde25@gmail.com>
Load the receipt at (head_receipt_number + One::one()).min(available_best_receipt_number) for bundle
construction, this commit also skip domain slot if the primary block number is zero such that we don't
need to construct a dummy receipt (and all the special handling) in this case, and make the code cleaner.

Signed-off-by: linning <linningde25@gmail.com>
Signed-off-by: linning <linningde25@gmail.com>
Signed-off-by: linning <linningde25@gmail.com>
Copy link
Copy Markdown
Contributor

@liuchengxu liuchengxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me as a refactoring towards the next step, TODOs are fine, I believe we'll rewrite a lot of things once spec is done.

Signed-off-by: linning <linningde25@gmail.com>
Copy link
Copy Markdown
Contributor

@liuchengxu liuchengxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're good to go once the conflicts (should be renamings caused by #1531) are resolved.

@NingLin-P NingLin-P enabled auto-merge June 12, 2023 12:30
@nazar-pc nazar-pc requested review from liuchengxu and vedhavyas and removed request for vedhavyas June 12, 2023 15:22
@nazar-pc nazar-pc disabled auto-merge June 12, 2023 15:46
@nazar-pc nazar-pc merged commit 2d3e8e2 into main Jun 12, 2023
@nazar-pc nazar-pc deleted the bundle-receipt branch June 12, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change Bundle::receipts from Vec<Receipt> to Receipt

3 participants