Implement getBlockHash in LedgerPeersConsensusInterface

[crocodile-dentist]

I'd appreciate if someone could lend me a hand with implementing a small facility for the upcoming node 10.7 release.

We want to verify that a particular block hash matches the one recorded in the big ledger peer snapshot file when syncing in GenesisMode. The function has the signature forall r. SlotNo -> (forall a. STM m (Block SlotNo (Hash Blake2b_256 a)) -> r) -> r. The first parameter is the slot number to be examined and the second is a continuation which accepts an STM action, which is expected to block until the slot becomes part of the immutable chain, and which provides the corresponding Block value for the given input slot when it is available. A dummy implementation is defined in Ouroboros.Consensus.Node on mw/integrate-o-n-0.23 in commit wip.

[geo2a]

@crocodile-dentist the commit 7cde5cc82586444c5ad98b7be5eb16c3b32217b4 referenced in the s-r-p on ouroboros-network does not exist. Which branch should I track? Would it be OK to try integrating with the current main, or is there a dedicated release branch?

[crocodile-dentist]

I've updated the srp in the branch. The wip commit just shows the location where we need this, but conceptually this work sits on top of the network branch mw/ledgerpeersnapshot-hash which is under review. I gave the branch mw/integrate-o-n-0.23 instead because at least the main consensus libraries build with it as there are other unrelated breaking changes which we're still integrating.

[geo2a]

@crocodile-dentist I have a couple of questions that came into my head when I started working on this:

Questions 1

is is possible that there's no block at the requested slot? What would you like to get at as result if that's the case? Some sort of Left Error?

As I understand, the precondition is that the requested slot has a block, since the slot comes from the big ledger peer snapshot?

Question 2

Would it be OK to return a Point instead of a (Block SlotNo (Hash Blake2b_256 a)? I think it alights a little better with the API that is available in Consensus.

[crocodile-dentist]

is is possible that there's no block at the requested slot? What would you like to get at as result if that's the case? Some sort of Left Error?

If we change the signature to return a boolean, then simply False will be sufficient for me.

As I understand, the precondition is that the requested slot has a block, since the slot comes from the big ledger peer snapshot?

True.

[geo2a]

@crocodile-dentist I've pushed b5f25fd95 to the mw/integrate-o-n-0.23 branch.

I ended up needing to change the signature of getBlockHash again, but now it is much closer to your original request:

getBlockHash :: forall r. SlotNo -> (forall a. STM m (Point a) -> r) -> r

i.e. given a slot, the implementation will wait for the immutable tip to appear at that slot, and return the Point. I unfortunately cannot give you Block SlotNo (Hash Blake2b_256 a) as the immutable DB only allows me to extract the more abstract Point. Is this sufficient?

[crocodile-dentist]

I've changed the signature slightly to
forall r. SlotNo -> (forall blk. (StandardHash blk) => STM m (Point blk) -> r) -> r
I'm also worried about this precondition:

-- PRECONDITON: the slot must be know to be filled in the future.

In the unlikely event a node ends up on the wrong chain, this particular slot number may not be occupied and it will be ok for you to return the first Point blk that meets or exceeds the given SlotNo - the hash is unlikely to match and so I can trace this event. The complication is that in waitForImmutableBlock:

  immutableTip <- ImmutableDB.tipToPoint <$> ImmutableDB.getTip cdbImmutableDB
  check (pointSlot immutableTip == At targetSlot)
  pure immutableTip

afaik the check is not guaranteed to fire on the targetSlot, even when on the honest chain.

[geo2a]

the check is not guaranteed to fire on the targetSlot, even when on the honest chain.

I may be misunderstanding, but I though that, on the honest chain, this check would be guaranteed to fire as we know, from the ledger peer snapshot, that the target slot is occupied. Is this assumption not correct?

In the unlikely event a node ends up on the wrong chain

This is indeed something I have not considered.

I think if we rewrote the check as this waitForImmutableBlock:

  immutableTip <- ImmutableDB.tipToPoint <$> ImmutableDB.getTip cdbImmutableDB
  check (pointSlot immutableTip >= At targetSlot)
  pure immutableTip

we theoretically may skip a block. Even though my intuition suggests it to be unlikely, it is possible. Would that be a problem?