[ETCM-177] Simplify ommers handling

[mirkoAlic]

Description

fix for: https://jira.iohk.io/browse/ETCM-177

Proposed solution

• If the ommers are invalid we just create a block without any.
• Add ommers only at resolve branch level
• Stop manually removing ommers from pool, eventually older ones will disappear.

TODO

• [ETCM-279] Validate ommers at pool level

[ntallar]

Should we go further in the pool validation for ommers (ex. check if there were already used)

I think we are currently in the middle between 2 approaches:

1. Keeping the ommers pool with a lot of tentative ommers query, which is why the pool response depends on the block being mined
2. Keeping the ommers pool with only the ommers that will be valid for the block being mined, which ideally won't require any validations (neither on pool query or block generation)

Option 1. seems to be closer to what we currently have, so I'm more inclined to it as well... we could keep the removals of ommers to the bare minimum (as they are only tentative ommers), improve the validations done on the pool upon query as you said and remove validations on the block generator

[ntallar]

Should we log this case?

[mirkoAlic]

mmm...now i'm thinking we could log this at ommers pool level once we finally do all the validations. Btw, the amount of ommers per block is something that we measure, so we couyld produce metrics, i mean, it won't be an issue to get track of that.

[ntallar]

I don't like much the middle ground of keeping the pool and ignoring any problems when using it on the miner, but only updating the pool it half of the times, should we:

1. Remove the pool entirely and have no support with mining blocks with ommers
2. Keep the pool updates as on develop and use the patch for producing blocks without ommers for handling the case that it has any invalid one

[mirkoAlic]

Should we go further in the pool validation for ommers (ex. check if there were already used)

I think we are currently in the middle between 2 approaches:

1. Keeping the ommers pool with a lot of tentative ommers query, which is why the pool response depends on the block being mined

2. Keeping the ommers pool with only the ommers that will be valid for the block being mined, which ideally won't require any validations (neither on pool query or block generation)

Option 1. seems to be closer to what we currently have, so I'm more inclined to it as well... we could keep the removals of ommers to the bare minimum (as they are only tentative ommers), improve the validations done on the pool upon query as you said and remove validations on the block generator

In answer to this comment and the other one too.

• I prefer the ommers pool that works as a service, that a user can query using a given header, instead of provide a component that is relying on the current top.

• My next step (i will create the task and add the reference) is to refactor ommers pool in order to always return valid ones (currently the missing part is to check if those where used before).

• I strongly recommend to just filter bad ommers when someone is trying to create a new block. I mean, IMHO, including them is not a hard stop requirement for a new block.

• i don't want to provide a way to manually remove the ommers, i mean ommers pool already have a mechanism to forget older ones. IMO, it should be a independently component that only relies on the information in the blockchain, and apply internal mechanism for deletes. The only "drawback" i saw with current approach is to require a little bit more of RAM but is in post of simplify the interaction with other components in the system.
  (disclaimer: the idea was to also keep addition to the bare minimum)

[ntallar]

LGTM!

 .\\            //.
. \ \          / /.
.\  ,\     /` /,.-
 -.   \  /'/ /  .
 ` -   `-'  \  -
   '.       /.\`
      -    .-
      :`//.'
      .`.'
      .' BP