ValidationResult enum type

[stefantalpalaru]

[this is in relation to https://github.com/status-im/nimbus/issues/198]

• also added as a return for persistBlocks()

[stefantalpalaru]

Unrecoverable error code replaced with an exception.

[arnetheduck]

+1 bool - this looks over the top

[stefantalpalaru]

Boolean results don't really make sense for functions without predicate-like names. What is persistBlocks() supposed to return by default? What are the chances that it's false, so we can rely on Nim's zeroed-by-default return value like we do with an enum?

Unlike a mysterious bool (we can't have custom names for proc return values, can we?), an enum is self-documenting in both type name and values:

  case ctx.chain.persistBlocks(wi.headers, wi.bodies)
  of ValidationResult.OK:
    ctx.finalizedBlock = wi.endIndex
    wi.state = Persisted
  of ValidationResult.Error:
    wi.state = Initial

You don't even need to read that proc's signature, let alone body, to quickly follow the control flow here.

Another example where a bool would make you waste your time when trying to understand the code:

if syncCtx.returnWorkItem(workItemIdx) != ValidationResult.OK:
  giveUpOnPeer = true

(no, it's not a "WorkItem" that's being returned by returnWorkItem())

For booleans to work as well, you need clear predicate names like passesValidation() or processedWorkItemWithoutProblems(). Even better if you can append a ? suffix to the procedure name, like it's done in Scheme and Ruby.