feat: count block approval towards chunk endorsement #11255
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11255 +/- ##
=======================================
Coverage 70.99% 70.99%
=======================================
Files 781 781
Lines 155423 155505 +82
Branches 155423 155505 +82
=======================================
+ Hits 110344 110403 +59
- Misses 40311 40333 +22
- Partials 4768 4769 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -67,6 +77,7 @@ impl Chain { | |||
| // Verify that the signature in block body are valid for given chunk_validator. | |||
| // Signature can be either None, or Some(signature). | |||
| // We calculate the stake of the chunk_validators for who we have the signature present. | |||
| // We always include block producers who have sent approvals, since that indicate they have applied the corresponding chunk | |||
There was a problem hiding this comment.
I do not get this. My understanding is in stateless validation, block producers do not apply chunks but only validate blocks (headers), so their approval will not be for the application of chunks. Even if they are also chunk validators how do we relate their approval with witness validation?
There was a problem hiding this comment.
Below (L109-112) the approvals are filtered down to those from chunk producers for the shard. So if you are a chunk producer and also a block producer, your block approval is implictly an endorsement of the chunk.
The part I'm not sure I understand is this: the ultimate goal is to "stop chunk producers from validating state witness if they are assigned also as a validator". Would it be simpler to just send an endorsement without validating in that case instead of also adding this implicit endorsement logic?
There was a problem hiding this comment.
ok thanks Saketh, I missed that part. Maybe change comment to
// We always include chunk producers that are also block producers who have sent approvals, since that indicate they have applied the corresponding chunk
This (chunk producer sending endorsement right away) is already happening here:
(the missing piece is that this does not prevent chunk producer to send witness to itself and validate it)
is the concern that that endorsement may not reach all potential block producers (eg. in case of skips)?
There was a problem hiding this comment.
the ultimate goal is to "stop chunk producers from validating state witness if they are assigned also as a validator"
This is only part of the goal. The other part is to allow chunk producers to use block approvals as chunk endorsements so that their chunk endorsements would arrive earlier, which increases the likelihood of a chunk getting enough endorsements and being included in a block.
)" This reverts commit 0a61a29.
#11330) The logic in #11255 is not correct because chunk endorsement is dependent on the chunk header included in the current block and endorses that chunk header is valid. Block approvals cannot replace chunk endorsements because when a block approval is sent, there is no guarantee that the chunk producer has seen the new chunk header and therefore a malicious attacker can still produce an invalid chunk with correct block approvals. Thanks @pugachAG for catching this!
Count chunk producers who send block approvals towards chunk endorsements if they also happen to be chunk validator for a specific shard. Note that this change does not stop chunk producers from validating state witness if they are assigned also as a validator. That will be done separately.