[WIP] Add basic API for certificate validation

[amesgen]

Closes tweag/cardano-peras#94

[agustinmista]

Hey @amesgen @tbagrel1

The PR still needs a lot of work, but I think it would be good to get some early feedback on 7212954. I pushed a bunch of changes adapting call sites to use ValidatedPerasCerts whenever needed, as implied by the two TODOs left in 3425d67:

• Adapting PerasCertDB.addCert to take a ValidatedPerasCert, and
• Adapting ChainDB.addPerasCertAsync to take a ValidatedPerasCert

Based on my current understanding, there is a tension between keeping either PerasCerts or ValidatedPerasCerts around, depending on what we want to enforce:

1. Validating certs early on, and using those everywhere

• Somewhat better assurance that we don't ever try to operate on potentially-invalid certs
• Potentially useful in the cases where the extra evidence they carry allows us to avoid recomputing stuff (e.g. the block's boost)
• Needs a "big" refactor to adapt all call sites, including the "generation" of the now needed validation evidence in some property tests

2. Validating certs when needed, but using the original PerasCert everywhere

• Might open the door for bugs caused by operating on invalid certs, not sure how likely this is, though
• When useful, validated certs (or at least their useful evidence) might need to be passed around manually all the way down to the consumer
• Likely as smaller refactor, as we can probably unwrap ValidatedPerasCerts in a couple of places and keep the original type signature

Do you think (1) is the right approach here? Or is (2) something more akin to what we would want at this stage in the dev cycle?

[amesgen]

I think (1) is indeed the right approach, the benefits seem compelling even at this stage.

(In the standup, we just briefly talked about some of the implications this has once we store certificates on disk, such as whether we also want to store the weight boost of a certificate on disk, or recompute it on startup. But this is not important for now.)

including the "generation" of the now needed validation evidence in some property tests

This is actually a desired outcome of this change, see the last paragraph in the issue description of #94. To keep this PR minimal, still always using boostPerCert sounds good as you currently do; enriching the generators could be follow-up work.

[agustinmista]

This pattern pops up everywhere, so we might want to introduce some helpers:

validatedPerasCertRound :: ValidatedPerasCert blk -> PerasRoundNo
validatedPerasCertBoostedBlock :: ValidatedPerasCert blk -> Point blk

This naming would be consistent with the existing validatedPerasCertBoost projection.

[agustinmista]

This will have to be turned into an external parameter at some point, IIUC.

[agustinmista]

Another value that will have to be computed upon some external parameters, IIUC.

[agustinmista]

Mocked-up value for now.

I'm not sure whether we need to parameterize makePerasCertPoolWriterFromChainDB, or if there's a way to extract the current PerasRoundNo from the monadic context somehow.

[agustinmista]

Do we have a concrete exception type in mind for this (e.g. some existing one to add a variant to, or perhaps a new PerasException).

A quick (and rather uninformed) search didn't reveal any obvious candidate 🤔

[agustinmista]

There's also the question of whether we want to short-circuit and throw an exception upon the first validation error (at it happens now with sequence), or to be pedantic and collect all the validation errors in the current batch and report them at once.

[agustinmista]

Not sure exactly what these synonyms are currently capturing. The changes in this module were needed after massaging the rest of the code.

However, having ended up with the duality:

• *Inbound | *Writer -> PerasCert, and
• *Outbound | *Reader -> ValidatedPerasCert

Makes me feel this might make a bit of sense. Please let me know if I am way off-base here 😅

[agustinmista]

This signature didn't change, so I'm not sure why this change crept in.

If fourmolu running on some lenient/not-strict mode?