Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Handle large storage proofs. #2103

Closed
wants to merge 1 commit into from
Closed

RFC: Handle large storage proofs. #2103

wants to merge 1 commit into from

Conversation

rahulksnv
Copy link
Contributor

  1. When the storage proof in InvalidStateTransitionProof exceeds a threshold, a summary of the storage proof is included in the fraud proof instead of the full storage proof.
  2. The verifier is expected to recreate the storage proof locally and verify that the summary is correct.

The PR is meant to get feedback on the approach/feasibility for now, will be opened for further review based on feedback.

Issue: #1596

Code contributor checklist:

1. When the storage proof in InvalidStateTransitionProof
   exceeds a threshold, a summary of the storage proof is
   included in the fraud proof instead of the full storage
   proof.
2. The verifier is expected to recreate the storage proof
   and verify that the summary is correct.
@vedhavyas
Copy link
Member

Would have been nice to open the draft on your fork and requested feedback there to avoid PR cluttering.

What we want to achive here is similar to Ethereum's OutOfGas. In our case, since we do not execute extrinsic through gas metering, we want to introduce a metering on Storage reads where we will have max amount of storage read bytes and mark a transaction as failed if the read bytes exceeds the max value. Max value for each transaction will be setup such that a fraud proof carrying such reads in the Storage proof will fit in the consensus block. If a malicious operator ends up not exiting when such read data is exceeded and submits an ER, any honest node can produce invalid state transition fraud proof and verifier can run this transaction and comes to same error as honest node there by verifying the invalid state transition.

Hope that make sense

@rahulksnv rahulksnv closed this Oct 14, 2023
@rahulksnv rahulksnv reopened this Oct 14, 2023
@rahulksnv
Copy link
Contributor Author

What we want to achive here is similar to Ethereum's OutOfGas. In our case, since we do not execute extrinsic through gas metering, we want to introduce a metering on Storage reads where we will have max amount of storage read bytes and mark a transaction as failed if the read bytes exceeds the max value. Max value for each transaction will be setup such that a fraud proof carrying such reads in the Storage proof will fit in the consensus block. If a malicious operator ends up not exiting when such read data is exceeded and submits an ER, any honest node can produce invalid state transition fraud proof and verifier can run this transaction and comes to same error as honest node there by verifying the invalid state transition.

I did spend two days looking at adding OutOfGas kind of functionality on the upstream run time execution environment. That certainly is a bigger effort, may be more than a week(also mentioned here: paritytech/substrate#14350). This PR was meant to get feedback on alternate(less perfect) proposal.

If OutOfSpace error during execution is what we want, I can go back and work on that. @jfrank-summit @vedhavyas please confirm

@rahulksnv rahulksnv closed this Oct 17, 2023
@nazar-pc nazar-pc deleted the rsub/fp-size branch February 23, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants