Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Keccak lookup error causes real prover to panic in testnet #723

Closed
AronisAt79 opened this issue Aug 29, 2022 · 24 comments
Closed

Keccak lookup error causes real prover to panic in testnet #723

AronisAt79 opened this issue Aug 29, 2022 · 24 comments
Assignees
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bug Type: bug

Comments

@AronisAt79
Copy link
Contributor

During proof generation (block consisting of a single deposit Tx):

[2022-08-29T12:52:09Z INFO prover::shared_state] ProvingKey: generated and cached key=/testnet/21.bin944750200000596662096896
[2022-08-29T14:05:32Z INFO prover::shared_state] task_result: Err(
"The constraint system is not satisfied",
)

commit: 2887314434c1fe52e88a7176385c2db09fadae8d

@AronisAt79
Copy link
Contributor Author

@pinkiebell
Copy link
Contributor

Ah, actually the commit should fix privacy-scaling-explorations/zkevm-chain#24 😆

@AronisAt79
Copy link
Contributor Author

@CPerezz
Copy link
Member

CPerezz commented Aug 31, 2022

That was fast!! Thanks @pinkiebell @AronisAt79 !!

Did we go for the approach discussed in our call @AronisAt79 ??

@CPerezz
Copy link
Member

CPerezz commented Aug 31, 2022

According to the logs. I see that:
SHL is used but has a DummyGadget impl. But this should not be causing the failure AFAIK.

As for the logs, They're pretty difficult to read. Isn't it possible to print the entire MockProver trace within a single log? Instead of one log per line of report??
I think that way would make it much more readable.
Also, looks like the error is in the Keccak lookup. There are no SHA3 opcodes.

But there's `MSTORE` for example in the block traces. See:
{
  3                 "pc": 677,
  2                 "op": "MSTORE",
  1                 "gas": 49341,
  0                 "gasCost": 3,                                                                                                                         
  1                 "depth": 1,
  2                 "stack": [
  3                     "0x4088a3e7",
  4                     "0x87",
  5                     "0x1", 
  6                     "0xeee29da5c88439c8d09fcebac34981465c948500563f26c235888e8f71d8ceec",
  7                     "0x84",  
  8                     "0x260", 
  9                     "0x2ce",
 10                     "0x84",
 11                     "0xa8",
 12                     "0x80",
 13                     "0x0", 
 14                     "0xeee29da5c88439c8d09fcebac34981465c948500563f26c235888e8f71d8ceec",
 15                     "0x2e4",
 16                     "0x1",
 17                     "0x80",
 18                     "0x20",
 19                     "0x88",
 20                     "0x2000000000000000000000000000000088",
 21                     "0x80"   
 22                 ]
 23             },

cc: @han0110 @ed255

@CPerezz CPerezz changed the title Prover: proof generation fails with "The constraint system is not satisfied", Keccak lookup error causes real prover to panic in testnet Aug 31, 2022
@CPerezz
Copy link
Member

CPerezz commented Aug 31, 2022

I think this should be moved to zkevm-circuits now that we have all the tools needed to analyze the issue.

@CPerezz CPerezz transferred this issue from privacy-scaling-explorations/zkevm-chain Aug 31, 2022
@CPerezz CPerezz added crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bug Type: bug labels Aug 31, 2022
@CPerezz CPerezz added this to the successful-erc20 milestone Aug 31, 2022
@AronisAt79
Copy link
Contributor Author

That was fast!! Thanks @pinkiebell @AronisAt79 !!

Did we go for the approach discussed in our call @AronisAt79 ??

@CPerezz pretty much. But simpler :). No extra service//container created. Once a proof_request fails, mockProver is activated on the same container. @pinkiebell said that we could also implement a proof_request option to control whether a real or mock prover is engaged per proof_request. I think that would be an awesome debug tool to consider.

@aguzmant103 aguzmant103 moved this to 🆕 Product Backlog Items in zkEVM Community Edition Sep 1, 2022
@aguzmant103 aguzmant103 moved this from 🆕 Product Backlog Items to 📋 Refined Backlog in zkEVM Community Edition Sep 1, 2022
@ChihChengLiang ChihChengLiang moved this from 📋 Refined Backlog to 🏗 In progress in zkEVM Community Edition Sep 7, 2022
@ChihChengLiang
Copy link
Collaborator

@AronisAt79 @CPerezz @pinkiebell do we know what's the pre-state for the block?

@AronisAt79
Copy link
Contributor Author

@AronisAt79 @CPerezz @pinkiebell do we know what's the pre-state for the block?

Hi @ChihChengLiang. Unfortunately not. I will need to repeat the test since the environment has been redeployed. How do i collect the block pre_state info?

@ChihChengLiang
Copy link
Collaborator

I have no idea currently. Maybe @pinkiebell has some info. I can look into the prover code.

@pinkiebell
Copy link
Contributor

The prestateTracer

{"id":0, "jsonrpc":"2.0","method":"debug_traceBlockByNumber", "params":["0x1", {"tracer":"prestateTracer"}]}

Returns for example:

{"jsonrpc":"2.0","id":0,"result":[{"result":{"0xdf08f82de32b8d460adbe8d72043e3a7e25a3b39":{"balance":"0x16345785d8a0000","nonce":0,"code":"0x","storage":{}}}}]}

@CPerezz
Copy link
Member

CPerezz commented Sep 12, 2022

I think this should do @ChihChengLiang right? Since the failure is inside the MSTORE checks, this should allow to debug the entire thing. Specially since we already know the randomness.

@AronisAt79
Copy link
Contributor Author

thanks @pinkiebell!
@ChihChengLiang i am attaching debug trace for block4 with prestate tracer enabled, plus debug traces for block4 transactions.
Block4_preStateTrace.zip
Mockprover log turned out huge, in case it is needed please let me know

@ChihChengLiang
Copy link
Collaborator

Thank you @AronisAt79 for the prestate. Is there a chance we can get the block info? It would have the content of the block header and the transactions inside it.

@pinkiebell
Copy link
Contributor

@ChihChengLiang I looked your PR. I think the error must have something todo with not having the proper block/tx setup. So the data of the block is needed and (easier) to reproduce anyway.

I think it's best to pull-in a script that:
i) get the block data w/ full transactions
ii) fetches all history hashes
Iii) fetches prestate for the block in question

@pinkiebell
Copy link
Contributor

I added a helper script: privacy-scaling-explorations/zkevm-chain@0dc460a

@AronisAt79 You have to retry this again. Sorry for the inconvenience 🙏

@AronisAt79
Copy link
Contributor Author

Hello,

@pinkiebell thanks for the script. I will add a debug option into our test tool to fetch that info. Can you please explain why you go back specifically 255 blocks in history?

block27.zip

@ChihChengLiang i am attaching fresh set of info plus tx traces. Will you need the prover log as well?

@ChihChengLiang
Copy link
Collaborator

@AronisAt79 thanks for the data. Yeah prover log might help if it's not too much trouble for you.

@pinkiebell
Copy link
Contributor

Hello,

@pinkiebell thanks for the script. I will add a debug option into our test tool to fetch that info. Can you please explain why you go back specifically 255 blocks in history?

block27.zip

@ChihChengLiang i am attaching fresh set of info plus tx traces. Will you need the prover log as well?

The max. supported lookup (by EVM convention) is 256 blocks. And otherwise are just zero hash. I didn't take a closer look ad the circuit input builder and just padded the array accordingly.

@ChihChengLiang
Copy link
Collaborator

@pinkiebell the bug seems to be fixed on main. Can you try the zkevm-main on zkevm-chain prover?

@pinkiebell
Copy link
Contributor

@pinkiebell the bug seems to be fixed on main. Can you try the zkevm-main on zkevm-chain prover?

Will do.

@pinkiebell
Copy link
Contributor

@AronisAt79 Can we do a run with privacy-scaling-explorations/zkevm-chain@d01a5c0 and see if it goes trough without errors?

@AronisAt79
Copy link
Contributor Author

@AronisAt79 Can we do a run with privacy-scaling-explorations/zkevm-chain@d01a5c0 and see if it goes trough without errors?

sure! on it

@ChihChengLiang
Copy link
Collaborator

Close in favor of #790

@ChihChengLiang ChihChengLiang moved this from 🏗 In progress to ✅ Done in zkEVM Community Edition Sep 22, 2022
lispc added a commit that referenced this issue Aug 28, 2023
* try

* fix: returndatalength for precompiles

* fix: add restorecontext to modexp

* return data length for precompiles using BaseGadget

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bug Type: bug
Projects
Status: Done
Development

No branches or pull requests

4 participants