celestiaorg/blobstream-contracts

Return false or revert in DAVerifier

Closed this issue · 3 comments

Currently, in the DAVerifier library, if the proof verification fails at some stage, it reverts with an error:
https://github.com/celestiaorg/quantum-gravity-bridge/blob/0dc9e3f04884d547db044289270d5c8080e500bc/src/lib/verifier/DAVerifier.sol#L50-L68

However, if the execution succeeds, it returns true.

Does it make sense to keep the errors or it's better if we return false instead of reverting?

Thinking out loud: libraries that are intended to be used by third parties might be better if they return false with some error rather than reverting. That allows for maximum flexibility.

I'll second returning false, I think we need to in order to trigger fraud proof logic in the case of InvalidSharesToRowsProof or some equivalent where is no data in the square that was committed to in the rollup header.

on a kinda related note @sweexordious, are there any scenarios where any of the errors above mean something something other than fraud? Like, if the data committed to cannot be proved, is the result always the same? The reason I'm asking is because ideally the UX is super simple and I don't want to limit the burden for the developer. Knowing what each of the fraud proofs means above feels like a lot of overhead