Fix parsing of blocks in .era
#3899
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
| else if (slot < ssz.ForkSlots.Capella) | ||
| return ssz.BellatrixSignedBeaconBlock.decode(data.data as Uint8Array) | ||
| else if (slot < ssz.ForkSlots.Deneb) | ||
| return ssz.CapellaSignedBeaconBlock.decode(data.data as Uint8Array) | ||
| else return ssz.ETH2_TYPES.SignedBeaconBlock.decode(data.data as Uint8Array) |
There was a problem hiding this comment.
so the eth2_types has latest i.e. deneb in this case?
hopefully microsigner can provider the better api to get types for some slot
There was a problem hiding this comment.
Yes. Per my other comment, will work with Paul on a better structure for these
| else if (stateSlot < ForkSlots.Capella) | ||
| return BellatrixBeaconState.decode(data.data as Uint8Array) | ||
| else if (stateSlot < ForkSlots.Deneb) return CapellaBeaconState.decode(data.data as Uint8Array) | ||
| if (stateSlot < ssz.ForkSlots.Altair) return ssz.Phase0BeaconState.decode(data.data as Uint8Array) |
There was a problem hiding this comment.
I guess this if else still works if two/more forks are scheduled on the same slot, but adding a comment to that effect will be helpful to a third reader or may be (much) later to ourselves as well lol
There was a problem hiding this comment.
I guess I'm not sure what you mean. This code can't stay static and will have to update with more forks down the line. There's probably a better way to do this but I haven't set up a forkFinder sort of helper method and I really want to do another pr to meth to put all the fork specific containers under a ssz.ETH2_TYPES['fork'].containerType structure and not the way it is now where everything is exported from the base
| @@ -40,7 +27,8 @@ export const readSlotIndex = (bytes: Uint8Array): SlotIndex => { | |||
|
|
|||
| for (let i = 0; i < count; i++) { | |||
| const slotEntry = slotIndexEntry.data.subarray((i + 1) * 8, (i + 2) * 8) | |||
| const slotOffset = Number(new DataView(slotEntry.slice(0, 8).buffer).getBigInt64(0, true)) | |||
| let slotOffset = Number(new DataView(slotEntry.slice(0, 8).buffer).getBigInt64(0, true)) | |||
| if (slotOffset === -1 * recordStart) slotOffset = 0 // If offset is the same as the block record start, this is a skipped slot | |||
There was a problem hiding this comment.
UPDATE: so the era spec offset 0 encoding for missed data/block might mean absolute 0 offset which then makes sense here. In that case I would suggest pushing undefined in slotOffsets to make it type friendly way of detecting skipped data/slots
previous comment:
slotoffset can be a negative number? ah so all offsets are negative numbers reading file from back
but this doesn't make sense as some block/state could be recordStart bytes to the left of the slot index
| } | ||
| throw err | ||
| } | ||
| if (indices.blockSlotIndex!.slotOffsets[x] === 0) continue // skip empty slots |
There was a problem hiding this comment.
ok so this is handled here 👍 , may be the equality indices.blockSlotIndex!.slotOffsets[x] === -1 *indices.blockSlotIndex!.recordStart should be moved here instead of pushing 0 over there. whatever seems more cleaner to you
or how about pushing undefined in slotOffsets than 0 ? that is more type friendly way of dealing with skipped slots
2 updates
slotIndexoffsets were computed for missed slots (i.e. when a Beacon Block wasn't produced for a given slot)micro-eth-signerto v0.14.0 across the monorepo so we can remove the SSZ types for the pre-Deneb forks (since Paul has kindly added them tom-eth