adding verifiers to ProofService

[gupadhyaya]

Fixes #4. As part of rollkit/rollkit#982, we would like to have a facility to invoke a state machine verifier as part of Proof.Validate, such that we can move the proxyApp.VerifyFraudProofSync to Proof.Validate. If the state machine verifier is not passed (or nil), Proof.Validate can safely ignore it. An example, usage is:

func(fraudProof fraud.Proof) (bool, error) {
	stateFraudProof, ok := fraudProof.(*types.StateFraudProof)
	if !ok {
		return false, errors.New("unknown fraud proof")
	}
	resp, err := proxyApp.Consensus().VerifyFraudProofSync(
		abci.RequestVerifyFraudProof{
			FraudProof: &stateFraudProof.FraudProof, 
			ExpectedValidAppHash: stateFraudProof.ExpectedValidAppHash,
		},
	)
	if err != nil {
		return false, err
	}
	return resp.Success, nil
}

[vgonkivs]

Hello @gupadhyaya, I have a suggestion how we can improve your approach. The problem here is that extending input arguments is not the generic way that me and @Wondertan were trying to achieve in the library. For example, in celestia-node we don't need statemachine verifier but with these changes we will have to keep nil.
My suggestion is to have a Verifier type and have an ability to register custom verifiers as we have done with Marshalers. So,you will have the same result but without breaking an interface.

[Wondertan]

Agree with @vgonkivs. Also:

• Stateful verification on the method of a stateless object is an antipattern(not blockchain state)
• StateMachineValidator is implementation specific; not all FPs need Blockchain state machine.
• It's interface breaking

Instead, I suggest adding a new method to FraudService. Something like AddVerifier that takes the fraud type and the verification func. It's essentially the same as @vgonkivs proposed but without the global state of register approach.

[gupadhyaya]

@vgonkivs @Wondertan thanks for your suggestion. agree that we should keep the validate interface stateless. will update the FraudService with AddVerifier approach and invoke the verifier as part of processingIncoming.

[gupadhyaya]

@vgonkivs @Wondertan ready for re-review. thanks.

[vgonkivs]

1. What if we will allow to register multiple verifiers per proofType? wdyt?
2. Please add a mutex for verifiers(verifiersLK sync.RWMutex)

[gupadhyaya]

@vgonkivs

1. I thought about it, but for now we don't need multiple verifiers. In future if needed, we will extend it.
2. Done.

[vgonkivs]

LGTM. Lets wait for @Wondertan review and merge

[Wondertan]

LGTM now.
Thank you, @gupadhyaya!

[gupadhyaya]

@Wondertan @vgonkivs could you merge and make a release? thanks.

[Wondertan]

@gupadhyaya, I was in the series of calls, so could not do, but glad that you have permissions now