Skip to content

Comments

Add fraud prover#544

Closed
jliphard wants to merge 31 commits intoethereum-optimism:masterfrom
jliphard:add_fraud_prover
Closed

Add fraud prover#544
jliphard wants to merge 31 commits intoethereum-optimism:masterfrom
jliphard:add_fraud_prover

Conversation

@jliphard
Copy link

@jliphard jliphard commented Apr 21, 2021

Basic Function Testing Completed - See Readme for Documentation Thereof

HOWTO
For basic instructions on adding a Verifier and Fraud_Prover pair to your local test system, please see the Readme.

Description
Refactor the now archived fraud-prover service for the new monorepo.

Context
The fraud-prover service is relevant to the sequencer operating in its verifier mode. Adding the fraud-prover service into the monorepo raises several questions that relate to other repos, the dockers, the ci, etc. For example, some of the utility functions needed by the fraud-prover might be best located in core-utils? More generally, it could be useful to write a few simple scripts/commands related to running a verifier, rather than running the system in its normal operating mode.

Please let me know what would be most useful for you all!

@changeset-bot
Copy link

changeset-bot bot commented Apr 21, 2021

⚠️ No Changeset found

Latest commit: ce696e6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@tynes
Copy link
Contributor

tynes commented Apr 21, 2021

Hey @jliphard thanks for your contribution! It looks like some files could use some linting. I think the first step is to get the code into the repo and then we can follow up with additional PRs that add in the CI. Let us know when its ready for review and we can remove the DO-NOT-MERGE label

@jliphard
Copy link
Author

Ok, lint looks good and cleaned up some other things. No idea why the unit tests are failing for smockit (Error: src/smockit/binding.ts(2,43): error TS2307: Cannot find module 'hardhat/internal/hardhat-network/provider/errors' or its corresponding type declarations.) since that's in an entirely different part of the codebase.

@tynes
Copy link
Contributor

tynes commented Apr 22, 2021

Ok, lint looks good and cleaned up some other things. No idea why the unit tests are failing for smockit (Error: src/smockit/binding.ts(2,43): error TS2307: Cannot find module 'hardhat/internal/hardhat-network/provider/errors' or its corresponding type declarations.) since that's in an entirely different part of the codebase.

I bet this is due it being on an older version of smock, try updating to 1.0.2

Update: i see that the fraud prover doesn't have a dependency on smock. Not quite sure what is causing this error right now

@tynes tynes mentioned this pull request Apr 22, 2021
@tynes
Copy link
Contributor

tynes commented Apr 22, 2021

Created an issue to track fixing this bug: #554

Jan Liphardt and others added 2 commits April 22, 2021 13:43
Complete function testing and document spinup of a Verifier/Fraud Prover pair in the setting of a generic local deployment
@jliphard
Copy link
Author

jliphard commented Apr 22, 2021

Ok, things seems to work as expected. I documented (in the readme) the needed local.yaml, local.env.yaml, Dockerfile.fraud_prover, and wait_for_Verifier_and_L2.sh code - this might also help to generate the needed /ops files.

@tynes
Copy link
Contributor

tynes commented Apr 23, 2021

Hey @jliphard, a fix to smock was merged here: #576. The problem seems to have gone away before this was merged? Would love to see if the integration tests pass now. You may be already aware of this, but fixing the yarn.lock can be done automatically just by rebasing and then running yarn

@jliphard
Copy link
Author

GitHub actions seem to be stuck? Will reset them by closing the PR, and then re-opening it.

@jliphard jliphard closed this Apr 23, 2021
@jliphard jliphard reopened this Apr 23, 2021
@gakonst
Copy link
Contributor

gakonst commented Apr 25, 2021

Hey @jliphard, we'll hold off on merging this PR until we can get some integration tests in for this functionality, as well as bringing it up in the docker-compose stack.

If you end up writing any of these, go ahead and push them in this PR. Thank you for the contribution!

@jliphard
Copy link
Author

Hello @gakonst - makes perfect sense not to merge for the time being. I'm happy to help, but would require some input on what you think would be most useful. Which fraud scenarios should be evaluated? There are 'simple' fraud scenarios but also presumably others (e.g. involving the safety checker or gas limits or timing).

What kind(s) of fraud to consider -> Simple Fraudulent State Roots only?

A basic thing could be to inject/generate fraudulent state roots in the state-batch-submitter (specifically, _generateStateCommitmentBatch) which can already return an incorrect stateRoot if desired. Since that's done simply by using specific preconfigured blockTx.from values, that's really convenient. That should trigger the Verifier/Fraud Prover, although from a testing perspective, it would be good to write some valid state roots into the SCC first, so that correct pruning by finalizeFraudVerification and deleteStateBatch can be verified. Alternatively, the test-fraud could be injected upstream of the batch submitter (in the l2Provider) but the batch-submitter seems like a good place for that - and the entire functionality is already in place anyway.

Mockchain vs. live local system

It would be easiest (in terms of lines of code written) to just spin up a local system and define/use a suitable env.FRAUD_SUBMISSION_ADDRESS value. One could then push N transactions and, knowing that the generateStateCommitmentBatch will trigger after e.g. 5 transactions, test against that. This would require a few lines of code in generateStateCommitmentBatch. A test would be considered successful if the Verifier and Fraud Prover kick in, the system switches to Fraud Proof Mode, and after some time delay, the SCC roots would be reconciled the CTC transactions. Could also make sure that the gas was in line with expectations.

Where could/should the integration test live?

Setup could be (partially) handled in /integration-tests/test/shared/util.ts, although not entirely, since the batch_submitter would be needed to be brought up first.

Let me know what would be useful (and please correct the above) - there are dozens of ways of doing this - in the above tried to minimize the number of lines of code that would be affected.

@gakonst
Copy link
Contributor

gakonst commented Apr 26, 2021

If you wanted to use the Mockchain, I think it'd end up being a fraud prover unit test - see how the batch submitter unit tests are written.

For the integration tests, the setup code would be:

  1. a new docker image (see e.g. batch submitter, and the monorepo builder
  2. adding it to the docker-compose
  3. adding a new l2geth container which syncs from L1 data

In order to get a fraudulent batch in, you'd make a manual tx/state root submission to the smart contracts such that the post-tx state root is not the one which corresponds to the one from the transaction. Then you'd wait for the fraud prover to do its job.

@tynes
Copy link
Contributor

tynes commented Apr 26, 2021

you'd make a manual tx/state root submission to the smart contracts such that the post-tx state root is not the one which corresponds to the one from the transaction.

This could be done by calling the contracts directly from the tests, ie skipping the batch submitter. The key to use in the tests can be found here:

SEQUENCER_PRIVATE_KEY: "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"

@jliphard
Copy link
Author

Yes, @gakonst and @tynes - that's a much more elegant way to do it - perfect! Give me some time though...

@gakonst
Copy link
Contributor

gakonst commented Apr 27, 2021

All good - take your time. Thank you again.

@snario
Copy link
Contributor

snario commented May 6, 2021

Closing for now until there is more activity.

@snario snario closed this May 6, 2021
theochap pushed a commit that referenced this pull request Dec 10, 2025
* feat(executor): Migrate to `thiserror`

* lint
theochap pushed a commit that referenced this pull request Jan 15, 2026
### Description

In order to construct a signature to add to an
`OpNetworkPayloadEnvelope`, the op payload has to be serialized as ssz.
The serialized ssz bytes are then signed over using a local signer.
Along with constructing the payload hash, the signature is used to
construct the `OpNetworkPayloadEnvelope`.

Once the `OpNetworkPayloadEnvelope` is constructed with the signature
over the ssz encoded payload bytes,
the `OpNetworkPayloadEnvelope` is encoded and sent over the wire through
op p2p gossip.
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.

4 participants