Fraud proof verification on the primary node

[liuchengxu]

This PR aims to implement the fraud proof verification on the primary node, which is done by introducing a fraud proof externality extension so that it can be used in the runtime. There are also a few refactorings to the FraudProof struct so that it contains all the necessary information for the primary node to verify it.

There are still a few TODOs that may not necessarily have to be done in this PR:

• Test ProofVerifier
• Fetch the secondary runtime code from primary chain.

[nazar-pc]

Overall makes sense considering the approach you went for.

I'm not a big fan that we need to reach to the client from runtime to handle it though. What is the end goal here?

If we want to have compact fraud proof then we don't want the whole thing in the runtime and we'll have to do verification in runtime before committing the whole thing into the actual block. In which case we probably don't want to have verification from within runtime in the first place since we'll have to remove it later.

I guess we might have compact fraud proof in the extrinsic and host call will not only do the actual verification, but will also fetch the full fraud proof from some kind of cache, but then it seems to be very convoluted the way things need to be implemented.

I just want to make sure this is a step forward, not a temporary workaround we'll have to throw away since it is pretty big.

[liuchengxu]

I don't think using compact proof will change the overall workflow, it's an optimization of the efficiency, making the data smaller, but the general idea still holds. As long as you need to access some function from the client, I mean the std environment, the externality extension is necessary.

Based on all the information I have right now, I don't see it will be abandoned and there is another way out, unless something will be changed significantly, e.g., we won't verify the proof in the runtime but in the other places.

[nazar-pc]

With compact proofs we'll probably verify things in the client in the first place, hence no verification in runtime, externality is not needed and all the code around that will be gone as well.

[liuchengxu]

Hmm, not sure we're saying about the same compact proof, what I meant is actually this TODO https://github.com/subspace/subspace/blob/46420f74bc/cumulus/client/cirrus-executor/src/lib.rs#L673-L674, which just makes the origin storage proof in a compact form, so no changes to the workflow. Any you saying another kind of compact proof? 🤔

[nazar-pc]

Yes, the same one. My understanding is that compact proof will not include input storage items in the extrinsic. Which means we no longer have all of the information in the runtime to verifythe fraud proof. Hence we have two options to do that as I described in the comment above. One option means no runtime logic needed to verify it, second option means very complex workflow and thus not ideal (IMHO).

[liuchengxu]

Okay, then I positively believe it's a step forward as the compact proof only saves some bandwidth, and all the state information is still there. Not sure about the gains we could get in fact, an old comment indicates that 10% can be expected paritytech/substrate#8574 (comment). We could also measure it once we come to that stage.

[nazar-pc]

I believe compact proof notion is not the same for Subspace.

The basic idea as I understand it is the following:

• without compact fraud proofs we'll be able to only allow transactions/migrations that read the amount of state that can later be contained in a single transaction, otherwise fraud proof will be impossible to include in the primary chain
• with compact fraud proof however we only need to include in the transaction details about stage at which fraud occurred, the actual storage proof will only be available in the client:
  • this way we are no longer limited by the transaction size
  • storage proof is a derivative of the execution anyway and is not any useful unique info on its own

[liuchengxu]

We were not talking about the same thing :D

My idea of getting around the blocks of regular extrinsic might be that we include the hash of the storage proof(which might be even needless) instead of including the entire storage proof. Based on the other info of FraudProof, the farmer client is able to retrieve it from the storage network. But this doesn't change the workflow either IMO, the runtime still needs the result of verification of the fraud proof reported via an extrinsic to slash the executor or something.

[liuchengxu]

Even one day we change something significantly and decide to abandon the FraudProof extension, I don't see it's huge regarding the code changes, only partial of edc515f will be reverted, the other things in this PR will mostly still remain IMHO.

[nazar-pc]

the farmer client is able to retrieve it from the storage network

This can't be done during block execution and block it for indefinite amount of time. This needs to happen upfront, ideally before transaction is even included into the transaction pool so we can block, if necessary, bad source (network peer), which in turn means you need to decode extrinsic and check its content on arrival whether it is a fraud proof extrinsic. Once you figure out that it is a fraud proof extrinsic, you can just verify if it is valid right away with subspace-fraud-proof.

As the result, extrinsic included in transaction pool (or inherents, whatever we decide) is already pre-validated and doesn't need any runtime logic.

If above assumptions are correct, we probably don't want to add runtime logic with new externality just to remove it later. I mean we can merge it, but then we'll spend some time maintaining it and will have to remove anyway. Also since fraud proofs are free transactions (don't pay fees), allowing anyone to include it will mean spamming farmer blocks with useless contents for free, which is an DoS attack vector.

[liuchengxu]

Okay, that rings a bell, I remember we discussed this before, only the validated result will be reported the runtime, but you mentioned that Jeremiah is not a fan of this approach due to the DA or something?

[nazar-pc]

I don't recall that right now. But regardless of full or compact fraud proof, we can still do it in the client and be ready for the future.

[liuchengxu]

Regarding the attack of spamming of free fraud proof transaction, I think we should choose the way of charging some fees ahead and then only refund it once it's validated to be correct.

[nazar-pc]

We could apply fees, but IIRC it was a deliberate design decision to make those free, such that any full node would be able to produce it.

[liuchengxu]

Be future-proof, yeah. As mentioned in #301 (comment), even someday we drop the FraudProof extension, most of this PR should still remain and revelant.

[nazar-pc]

Good to go 👍

Let's plan to move the fraud proof verification logic to the client in the near future though.