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

fix: seal block if single tx overflows #468

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Conversation

Thegaram
Copy link

@Thegaram Thegaram commented Aug 17, 2023

1. Purpose or design rationale of this PR

After we run ccc and get an error, we revert all state changes:

go-ethereum/miner/worker.go

Lines 893 to 898 in 626ae08

accRows, err := w.circuitCapacityChecker.ApplyTransaction(traces)
if err != nil {
// `w.current.traceEnv.State` & `w.current.state` share a same pointer to the state, so only need to revert `w.current.state`
w.current.state.RevertToSnapshot(snap)
return nil, err
}

However, ccc internally maintains some state that is not reverted. As a result, if we continue to add txs to the block, we get such errors:

thread '<unnamed>' panicked at 'inconsistent sload: step proof 1766847170092282960410376565589572629199724816441630535569796515255295180, local statedb 1766847170092282936596459892185702160975285250786472294303038209972465637, result 1766847170092282960410376565589572629199724816441630535569796515255295180 in contract 0x4a23db3b8ddc60cde5f90f0734c9fd15cbadd27c, key 0', /root/.cargo/git/checkouts/zkevm-circuits-462c2cba34f9b301/87cae11/bus-mapping/src/evm/opcodes/sload.rs:67:17
...
TRACE[08-17|03:29:28.667] Unknown circuit capacity checker error for L2MessageTx tx=0x3ab635dce7c255a976ff48e7bd0e6490d25bd27abec496efae9a1d398ea833a1

This leads to discarding transactions unnecessarily. This PR changes the behaviour so that the worker does not continue to block any more transactions after a ccc error. We can change back to the original behaviour once this is fixed in the ccc.

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • fix: A bug fix

3. Deployment tag versioning

Has the version in params/version.go been updated?

  • This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change
  • Yes

@Thegaram Thegaram merged commit 81dc364 into develop Aug 18, 2023
@Thegaram Thegaram deleted the seal-on-overflow branch August 18, 2023 05:09
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.

3 participants