Skip to content

Conversation

@terencechain
Copy link
Collaborator

@terencechain terencechain commented Apr 26, 2023

Before broadcast, this PR applies consensus and equivocation checks for the beacon API's propose block endpoint.

Consensus check:

  • Get pre-state from input block's parent root using stategen's state by root
  • if pre state + input block to execute state transition fails, error out

Equivocation check:

  • Add a cache to track seen proposer index for the block slot. The first index of the cache is the block slot
  • Append proposer index after valid block signature. Clear the cache if the slot is higher than the last written slot. Do nothing if the block slot is not from the current slot

⚠️ : This is intended for relayer only. It should not be merged to develop

@terencechain terencechain requested a review from a team as a code owner April 26, 2023 14:18
Resync() error
}

type EqChecker interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would call it EquivocationChecker and add a thorough comment. Eq sounds like equals in other programming languages

@terencechain terencechain marked this pull request as draft April 26, 2023 14:30
}
_, err = transition.ExecuteStateTransition(ctx, parentState, block)
if err != nil {
return errors.Wrap(err, "could not execute state transition")
Copy link
Contributor

Choose a reason for hiding this comment

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

Better change the Godoc for SubmitBlockSSZ then.

}()

b := block.Block()
parentState, err := bs.StateGenService.StateByRoot(ctx, b.ParentRoot())
Copy link
Contributor

Choose a reason for hiding this comment

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

In our usual block processing pipeline we verify the block time disparity at this stage. Do we need to do it here?

func (s *Service) HasBlock(slot primitives.Slot, proposerIdx primitives.ValidatorIndex) bool {
blks := s.pendingBlocksInCache(slot)
for _, blk := range blks {
if blk.Block().Slot() == slot && blk.Block().ProposerIndex() == proposerIdx {
Copy link
Contributor

Choose a reason for hiding this comment

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

are nil checks for blk.Block() necessary?

Comment on lines 403 to 404
case uint64(s.seenProposerIndexCache[0]) > uint64(slot):
return
Copy link
Contributor

Choose a reason for hiding this comment

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

This case literally can't happen.

}

func (s *Service) SeenProposerIndex(slot primitives.Slot, proposerIdx primitives.ValidatorIndex) bool {
if s.seenProposerIndexCache[0] != primitives.ValidatorIndex(slot) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nilcheck for seenProposerIndexCache

if s.cfg.chain.CurrentSlot() != slot {
return
}

Copy link
Contributor

Choose a reason for hiding this comment

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

nilcheck for seenProposerIndexCache

Comment on lines 409 to 410
s.seenProposerIndexCache = []primitives.ValidatorIndex{primitives.ValidatorIndex(slot)}
s.seenProposerIndexCache = append(s.seenProposerIndexCache, proposerIdx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
s.seenProposerIndexCache = []primitives.ValidatorIndex{primitives.ValidatorIndex(slot)}
s.seenProposerIndexCache = append(s.seenProposerIndexCache, proposerIdx)
s.seenProposerIndexCache = []primitives.ValidatorIndex{primitives.ValidatorIndex(slot), proposerIdx}

@terencechain terencechain changed the title Apply consensus and equivocation check before broadcast [DO NOT MERGE]: Apply consensus and equivocation check before broadcast Apr 27, 2023
Comment on lines +1021 to +1023
if block == nil || block.IsNil() {
return errors.New("nil block")
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you do this before the defer function?

@dewindtk
Copy link

I am setting up a testnet and I would insanely appreciate a Dockerfile in order to set this up in Docker. Would it be possible to get a hold of one?

@MrKoberman
Copy link

Is this PR still required when using the latest version of prysm used by the relays?

@rauljordan
Copy link
Contributor

hi @dewindtk check out https://github.com/OffchainLabs/eth-pos-devnet if you want to set up a testnet. If you want to test out a specific change, such as this PR, you will need to build images with Bazel here: https://docs.prylabs.network/docs/install/install-with-bazel#running-images. We do not use dockerfiles

@terencechain terencechain deleted the verify-before-broadcast branch June 5, 2024 05:36
Copy link

@Dustin4444 Dustin4444 left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants