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

Make sure that we prefer own blocks over incoming blocks #1286

Closed
edsko opened this issue Nov 25, 2019 · 6 comments · Fixed by #2348
Closed

Make sure that we prefer own blocks over incoming blocks #1286

edsko opened this issue Nov 25, 2019 · 6 comments · Fixed by #2348
Assignees
Labels
consensus issues related to ouroboros-consensus

Comments

@edsko
Copy link
Contributor

edsko commented Nov 25, 2019

When an upstream node and we ourselves are both elected leader, in the (admittedly unlikely) scenario that we receive the upstream block before we manage to produce our own, I think the current chain DB does not guarantee that we will prefer our own block -- but it should; this is a clear violation of the Ouroboros spec.

@edsko
Copy link
Contributor Author

edsko commented Nov 25, 2019

Note that clock skew cannot make this worse: although we might allow for clock skew in the chain sync client, the chain DB will store such "blocks from the future" but not adopt them.

See also #1150, especially #1150 (comment) .

@edsko
Copy link
Contributor Author

edsko commented Dec 18, 2019

The Praos protocol implementation (OuroborosTag instance) can guarantee this by preferring chains that are signed by itself (after all, it must know its own identity anyway in order to be able to decide if its a leader).

@edsko
Copy link
Contributor Author

edsko commented Dec 18, 2019

Note that this is not essential for the testnet: if two nodes are both producing a block, a fork is created in the chain, which must later be resolved; one of the two nodes is going to lose anyway and its block will be lost. Not implementing this ticket will mean that the second leader, if it gets the other block before it manages to produce its own (rather unlikely anyway), will just "resolve the fork" of its own accord.

@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Dec 18, 2019
@mrBliss mrBliss self-assigned this Apr 10, 2020
@mrBliss mrBliss added this to the S11 2020-04-23 milestone Apr 10, 2020
@edsko
Copy link
Contributor Author

edsko commented Apr 22, 2020

Lowering this to medium priority since the probability of it happening is sufficiently unlikely and it won't break the system if we don't do it.

@mrBliss mrBliss removed this from the S11 2020-04-23 milestone Apr 22, 2020
@mrBliss mrBliss added this to the S16 2020-07-02 milestone Jun 30, 2020
@mrBliss
Copy link
Contributor

mrBliss commented Jun 30, 2020

Lowering this to medium priority since the probability of it happening is sufficiently unlikely and it won't break the system if we don't do it.

Raising this to high priority again, because people might intentionally set their clocks ahead to increase the probability of this happening.

The Ouroboros spec stipulates: if there are multiple leaders for any given slot, you should prefer your own block.

We have a good way to handle it: in case of a tie in block numbers during chain selection, when the slots are equal, prefer the block we produced.

mrBliss added a commit that referenced this issue Jun 30, 2020
Fixes #1286.

* Include `SelfIssued` in `TPraosChainSelectView`, use that to prefer the self
  issued tip in case the tips are produced in the same slot.

* Store the block issuer verification key (new, strict `BlockIssuerVKey` data
  type) in the `BlockConfig` so that we have access to it when we need to
  extract the 'SelectView' for a header.

* Fix the ordering: first look at the ocert issue number instead of the VRF
  hashes, doing it in the opposite order (as before) defeated the purpose.
mrBliss added a commit that referenced this issue Jun 30, 2020
Fixes #1286.

* Include `SelfIssued` in `TPraosChainSelectView`, use that to prefer the self
  issued tip in case the tips are produced in the same slot.

* Store the block issuer verification key (new, strict `BlockIssuerVKey` data
  type) in the `BlockConfig` so that we have access to it when we need to
  extract the 'SelectView' for a header.

* Fix the ordering: first look at the ocert issue number instead of the VRF
  hashes, doing it in the opposite order (as before) defeated the purpose.
mrBliss added a commit that referenced this issue Jul 1, 2020
Fixes #1286.

* Include `SelfIssued` in `TPraosChainSelectView`, use that to prefer the self
  issued tip in case the tips are produced in the same slot.

* Store the block issuer verification key (new, strict `BlockIssuerVKey` data
  type) in the `BlockConfig` so that we have access to it when we need to
  extract the 'SelectView' for a header.

* Fix the ordering: first look at the ocert issue number instead of the VRF
  hashes, doing it in the opposite order (as before) defeated the purpose.
iohk-bors bot added a commit that referenced this issue Jul 1, 2020
2348: Shelley: prefer self-produced blocks during chain selection r=mrBliss a=mrBliss

Fixes #1286.

* Include `SelfIssued` in `TPraosChainSelectView`, use that to prefer the self
  issued tip in case the tips are produced in the same slot.

* Store the block issuer verification key (new, strict `BlockIssuerVKey` data
  type) in the `BlockConfig` so that we have access to it when we need to
  extract the 'SelectView' for a header.

Co-authored-by: Thomas Winant <thomas@well-typed.com>
@iohk-bors iohk-bors bot closed this as completed in 6ea23ff Jul 1, 2020
@iohk-bors iohk-bors bot closed this as completed in #2348 Jul 1, 2020
@nfrisby
Copy link
Contributor

nfrisby commented Feb 11, 2021

For future reference, we're not sure the spec specifies you should favor your own (eg the Praos paper does not obviously handle the case where the result of maxvalid includes the current slot). But the observation about accidentally motivating users to set their clock forward is reason enough.

Edit: I found a Slack thread in sl-ouroboros channel from 2020 July 1st in which Alexander Russell confirmed that preferring your own block is not crucial for correctness but it's a reasonable way to balanced the needs of the protocol versus incentivizing rational actors. Quoting, where "current rule" is this issue:

Yes--I think the current rule is a good trade-off. Here's the story we can tell ourselves:

  • The rule is one for which we have a proof that the algorithm does the right thing. (In fact, any rule would be fine that first sorts chains according to length.)
  • It doesn't conspicuously fly in the face of the incentive mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants