Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "header matches block" #595

Closed
mrBliss opened this issue May 30, 2019 · 2 comments · Fixed by #875
Closed

Implement "header matches block" #595

mrBliss opened this issue May 30, 2019 · 2 comments · Fixed by #875
Assignees
Labels
byron ledger integration consensus issues related to ouroboros-consensus

Comments

@mrBliss
Copy link
Contributor

mrBliss commented May 30, 2019

The Block Fetch logic requires the following function:

-- | Given a block header, validate the supposed corresponding block
-- body.
blockMatchesHeader :: header -> block -> Bool

We need an implementation of it for ByronHeader and ByronBlock (from Ouroboros.Consensus.Ledger.Byron).

From Slack:

Damian Nadales [1 hour ago]
updateBody checks the delegation, update, and utxo proofs

Damian Nadales [1 hour ago]
but it doesn't use the header

Edsko de Vries [1 hour ago]
But surely something somewhere must check that these match?

Damian Nadales [1 hour ago]
Oh, wait, it does use the header

Damian Nadales [1 hour ago]
here

  :: MonadError ChainValidationError m
  => BodyEnvironment
  -> BodyState
  -> ABlock ByteString
  -> m BodyState
updateBody env bs b = do
  -- Validate the block size
  blockLength b <= maxBlockSize `orThrowError` ChainValidationBlockTooLarge

  -- Validate the delegation payload signature
  proofDelegation (blockProof b)
    == hashDecoded (blockDlgPayload b)
    `orThrowError` ChainValidationDelegationProofError

Damian Nadales [1 hour ago]
where blockProof comes from the header ...

blockProof :: ABlock a -> Proof
blockProof = headerProof . blockHeader

Edsko de Vries [45 minutes ago]
so its basically

Edsko de Vries [44 minutes ago]

  proofDelegation (blockProof b)
    == hashDecoded (blockDlgPayload b)
    `orThrowError` ChainValidationDelegationProofError

Edsko de Vries [44 minutes ago]
that does the "does this thing match the header" check right?

Damian Nadales [44 minutes ago]
Yes, that'd be my understanding

Damian Nadales [44 minutes ago]
the blockDlgPayload comes from the body

Edsko de Vries [43 minutes ago]
oh

Edsko de Vries [43 minutes ago]
but that's just one part of the body

Edsko de Vries [43 minutes ago]
there are a bunch of these proofs

Damian Nadales [43 minutes ago]
yes

Damian Nadales [43 minutes ago]
I didn't paste them 😉

Damian Nadales [43 minutes ago]
but they follow a similar pattern

Edsko de Vries [43 minutes ago]
@ru do you think it would make sense to move all of these proof checks to a separate function that we could call, passing a header and a block?

Edsko de Vries [41 minutes ago]
In the consensus layer we downloaded headers and blocks separately, and then need to check "does this header indeed belong to this block"?

Damian Nadales [40 minutes ago]

@ru do you think it would make sense to move all of these proof checks to a separate function that we could call, passing a header and a block?

... and if so we might want to update the specs accordingly ....

@mrBliss
Copy link
Contributor Author

mrBliss commented Jun 17, 2019

If not done before testnet, definitely before mainnet, because it is a security risk.

@mrBliss
Copy link
Contributor Author

mrBliss commented Jul 5, 2019

iohk-bors bot added a commit to input-output-hk/cardano-ledger-byron that referenced this issue Jul 31, 2019
591: Export validateHeaderMatchesBody in Cardano.Chain.Block.Validation r=intricate a=intricate

This function is required for IntersectMBO/ouroboros-network#595. It seems I forgot to export it in IntersectMBO/cardano-ledger#549.

Co-authored-by: Luke Nadur <[email protected]>
@mrBliss mrBliss added this to the Consensus_IntegrateLedger milestone Aug 2, 2019
iohk-bors bot added a commit that referenced this issue Aug 5, 2019
875: [#595] Implement and utilize byronBlockOrEBBMatchesHeader r=intricate a=intricate

Closes #595 

Co-authored-by: Luke Nadur <[email protected]>
@iohk-bors iohk-bors bot closed this as completed in #875 Aug 5, 2019
@mrBliss mrBliss removed this from the Consensus_IntegrateByronLedger milestone Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
byron ledger integration consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants