feat: handle electra attester slashing #7397
Conversation
| return {data: chain.opPool.getAllAttesterSlashings()}; | ||
| }, | ||
|
|
||
| async getPoolAttesterSlashingsV2() { | ||
| // TODO Electra: Determine fork based on data returned by api | ||
| return {data: chain.opPool.getAllAttesterSlashings(), meta: {version: ForkName.phase0}}; | ||
| const fork = chain.config.getForkName(chain.clock.currentSlot); |
There was a problem hiding this comment.
Note: I think we can just use clock slot here and don't need to filter out phase0 attester slashings as those can be serialized using electra.AttesterSlashing ssz type and should still be able to include them onchain as well
| // TODO Electra: Refactor submitPoolAttesterSlashings and submitPoolAttesterSlashingsV2 | ||
| await this.submitPoolAttesterSlashings({attesterSlashing}); | ||
| await validateApiAttesterSlashing(chain, attesterSlashing); | ||
| chain.opPool.insertAttesterSlashing(ForkName.electra, attesterSlashing); |
There was a problem hiding this comment.
Note: hard coding to electra here seems fine since the api already ensures that correct attesting_indices length for pre-electra attester slashings
Edit: after further looking into this we need to use the proper type has hash tree root would be different otherwise
| @@ -68,12 +68,21 @@ export function getBeaconPoolApi({ | |||
| }, | |||
|
|
|||
| async getPoolAttesterSlashings() { | |||
| const fork = chain.config.getForkName(chain.clock.currentSlot); | |||
|
|
|||
| if (isForkPostElectra(fork)) { | |||
There was a problem hiding this comment.
Note: we might not be able to serialize data if this is called post-electra since the limit of attesting_indices is higher, better to throw an error and refer to v2 api
| @@ -66,7 +75,7 @@ export class OpPool { | |||
| ]); | |||
|
|
|||
| for (const attesterSlashing of attesterSlashings) { | |||
| this.insertAttesterSlashing(attesterSlashing.value, attesterSlashing.key); | |||
| this.insertAttesterSlashing(ForkName.electra, attesterSlashing.value, attesterSlashing.key); | |||
There was a problem hiding this comment.
Note: same as insert from api, we can just use electra type as it works for both phase0 and electra attester slashing
Edit: on the api we need to pick correct fork type since we calculate the hash tree root but here we pass rootHash which means fork will be unused
| import {Bucket, getBucketNameByValue} from "../buckets.js"; | ||
|
|
||
| // We add a 1-byte prefix where 0 means pre-electra and 1 means post-electra |
There was a problem hiding this comment.
Do we really need this? It should be sufficient to just use ssz.electra.AttesterSlashing to serialize / deserialize data, we already validate on api and gossip (using proper ssz type) that the limit is correct
| @@ -26,7 +26,7 @@ export enum Bucket { | |||
| phase0_depositData = 12, // [DEPRECATED] index -> DepositData | |||
| phase0_exit = 13, // ValidatorIndex -> VoluntaryExit | |||
| phase0_proposerSlashing = 14, // ValidatorIndex -> ProposerSlashing | |||
| phase0_attesterSlashing = 15, // Root -> AttesterSlashing | |||
| allForks_attesterSlashing = 15, // Root -> AttesterSlashing | |||
There was a problem hiding this comment.
Note: changing bucket name affects metrics label but I think we are fine with that
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #7397 +/- ##
============================================
- Coverage 48.60% 48.60% -0.01%
============================================
Files 603 603
Lines 40523 40524 +1
Branches 2070 2070
============================================
Hits 19698 19698
- Misses 20787 20788 +1
Partials 38 38 |
Update various places to handle electra attester slashing type: