Only clear builder payment if the slashed validator is the proposer - #5365
Conversation
potuz
left a comment
There was a problem hiding this comment.
Very nice catch! This looks good to me, we need a regression test only. Perhaps we can have a solution without modifying the container, but looking for the proposer index in the previous epoch is awkward. I don' t see much problem in having the proposer index in the pending payments though.
Extend check_proposer_slashing_effect to expect the builder payment cleared only when the slashed validator is the payment's proposer (mirroring the spec guard), record the test payment for the slashed proposer by default, and add a regression test asserting that slashing a different validator equivocating on the same slot leaves an honest proposer's payment intact.
|
Thanks! Added the regression test (test_builder_payment_not_deleted_foreign_equivocation): it records a slot's payment for one proposer, slashes a different validator equivocating on the same slot, and asserts the payment is left intact. I also extended the shared check_proposer_slashing_effect to expect the entry cleared only when the slashed validator is the payment's proposer (mirroring the new guard), and updated the slashing-test helper to record the payment for the slashed proposer by default. On avoiding the container change: I went the same way you did -- the proposer can't be recomputed at slashing time for the previous-epoch case (proposer_lookahead only covers current and future epochs), so storing proposer_index on the pending payment seemed the cleanest option. Ran the full gloas fork suite + the eip8025 shard + make lint locally, all green. |
Use 'associated with' instead of 'armed' (per review).
jtraglia
left a comment
There was a problem hiding this comment.
LGTM, thank you @luca-zanolini!
Related to: * #5364 * #5365 I merged these back-to-back without pulling in updates from master & didn't realize that they conflicted. The first removes block as a parameter which the second uses to get the proposer index. Thankfully, the fix is simple; we can use `get_beacon_proposer_index` for this. It's worth mentioning that `process_block_header` checks that these are the same, so this change is safe.
implements the following spec changes required to pass `v1.7.0-alpha.11` spec tests - ethereum/consensus-specs#5359 - ethereum/consensus-specs#5377 - ethereum/consensus-specs#5373 - ethereum/consensus-specs#5365 - ethereum/consensus-specs#5364 - ethereum/consensus-specs#5368 --------- Co-authored-by: NC <17676176+ensi321@users.noreply.github.com>
Summary
In Gloas,
process_proposer_slashingclears theBuilderPendingPaymentIOU for a slot using only the slot number from the slashing evidence (header_1.slot), with no check that the slashed validator was that slot's proposer.consensus-specs/specs/gloas/beacon-chain.md
Lines 1816 to 1823 in 5e032ed
The grief vector
process_proposer_slashingvalidates a slashing by checking only that the two headers are consistent with each other — never that the proposer was the one actually assigned to the slot. Because the IOU is then cleared using onlyheader_1.slot, the slashed validator does not have to be the slot's proposer for the clear to fire.Concretely:
Pproposes the canonical block at slotNwith a positive bid, arming aBuilderPendingPaymentat slotN's index.Vsigns two distinct block headers that both claimslot = N. These are valid slashing evidence (V is equivocating), even though V was never slotN's proposer and the headers point to no real block.Nis still in the 2-epoch payment window. The clear keys onheader_1.slot = N, so it zeroesP's IOU.Result:
P's payment is cancelled — the proposer is never credited, the builder is never charged — purely becauseVequivocated on the same slot number.Impact
A validator can be sacrificed (one slashing) to cancel an honest proposer's builder payment. It applies to payments not yet settled via an on-chain payload reveal.
Fix
Bind each IOU to the proposer that armed it, and clear it only on a match:
proposer_index: ValidatorIndextoBuilderPendingPayment.block.proposer_indexinprocess_execution_payload_bid.process_proposer_slashing, clear the entry only whenpayment.proposer_index == header_1.proposer_index.The arming proposer can't be recomputed at slashing time for the previous-epoch case (
proposer_lookaheadno longer covers it), so the binding is stored on the entry.block.proposer_indexis sound to record becauseprocess_block_header(run before the bid) asserts it equalsget_beacon_proposer_index(state).