Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce the bundles contain ER that derive from the latest domain block #2194

Merged
merged 4 commits into from
Nov 3, 2023

Conversation

NingLin-P
Copy link
Member

close #1731
close #1934

This PR implement #1731, to only accept ER that derive from the latest domain block, in this way:

  • Only bundles produced by operator that have imported the latest domain block (hence have the latest ER) will be accepted
  • The NewBranch ER will be rejected and there is only one branch in the block tree right now, an operator must submit fraud proof to prune the bad ER at a specific height before submitting an ER at the same height. This also simplify how fraud proof pruning bad ER
  • Since there is only one branch in the block tree right now, it also fix The current way we consider ER as confirmed is wrong #1934

This PR also rejected the Stale ER directly, previously Stale ER is needed to be included in the block body so that we can measure the bundle production rate for the dynamic tx-range adjustment, now since the dynamic tx-range is deprecated, we can reject the Stale ER directly and not include it in the block at all.

Code contributor checklist:

The stale type ER is previously used to measure bundle production rate for the
dynamic tx range, which is now deprecated so we should reject stale ER.

The new branch type ER is rejected, so the honest operator must submit a fraud
proof to prune the bad ER at the same height before submitting ER at that height

Signed-off-by: linning <linningde25@gmail.com>
This commit change the block tree to use Option to store ER instead of BTreeSet,
thus only one ER at a specific height. This change also bring a overhaul to the
fraud proof processing and remove some TODO and FIXME

Signed-off-by: linning <linningde25@gmail.com>
…instead of CurrentHead

In this way we can ensure only ER that derived from the latest domain block will
be accepted

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

@vedhavyas vedhavyas left a comment

Choose a reason for hiding this comment

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

Overall make sense. Some nits and question.

crates/pallet-domains/src/block_tree.rs Outdated Show resolved Hide resolved
crates/pallet-domains/src/block_tree.rs Show resolved Hide resolved
crates/pallet-domains/src/block_tree.rs Show resolved Hide resolved
crates/pallet-domains/src/block_tree.rs Show resolved Hide resolved
@vedhavyas
Copy link
Contributor

Some how github did not let me add one more comment
https://github.com/subspace/subspace/pull/2194/files#diff-54f02a2b0cc1c8238dc34ce62aa8cdda723443632222b36772e59e3a3a956832R108
&head_receipt_number.saturating_add(One::one()) if you can move this into a new var instead of having this at the match would help readability

Signed-off-by: linning <linningde25@gmail.com>
@NingLin-P NingLin-P added this pull request to the merge queue Nov 3, 2023
Merged via the queue into main with commit d529f02 Nov 3, 2023
10 checks passed
@NingLin-P NingLin-P deleted the ER-submission-constraint branch November 3, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants