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(l2geth-verifier): only check last batch of the bundle #904

Merged

Conversation

colinlyguo
Copy link
Member

@colinlyguo colinlyguo commented Jul 15, 2024

1. Purpose or design rationale of this PR

In "finalize by bundle", only the last batch of each bundle is fully verified. This check still ensures the correctness of all batch hashes in the bundle due to the parent-child relationship between batch hashes.

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?

  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change

@colinlyguo colinlyguo force-pushed the fix-l2geth-verifier-only-check-last-batch-of-the-bundle branch from d6ad174 to 600ea40 Compare July 15, 2024 16:12
@colinlyguo colinlyguo marked this pull request as ready for review July 15, 2024 16:21
@colinlyguo colinlyguo marked this pull request as draft July 16, 2024 05:35
@colinlyguo colinlyguo force-pushed the fix-l2geth-verifier-only-check-last-batch-of-the-bundle branch from 42780a5 to df6a93e Compare July 16, 2024 06:37
@colinlyguo colinlyguo marked this pull request as ready for review July 16, 2024 07:57
0xmountaintop
0xmountaintop previously approved these changes Jul 16, 2024
Thegaram
Thegaram previously approved these changes Jul 16, 2024
@colinlyguo
Copy link
Member Author

colinlyguo commented Jul 16, 2024

tested in a local testnet with Darwin transition:

curl -s -X POST 'http://localhost:8888' -H 'Content-Type: application/json' -d '{"jsonrpc": "2.0","id": 54321,"method": "scroll_syncStatus"}' | jq
{
  "jsonrpc": "2.0",
  "id": 54321,
  "result": {
    "l2BlockSyncHeight": 475,
    "l1RollupSyncHeight": 5209,
    "l1MessageSyncHeight": 5237,
    "l2FinalizedBlockHeight": 329
  }
}

@colinlyguo colinlyguo dismissed stale reviews from Thegaram and 0xmountaintop via 6cb8a76 July 17, 2024 05:42
@colinlyguo colinlyguo requested review from amoylan2 and georgehao July 17, 2024 08:19
Thegaram
Thegaram previously approved these changes Jul 17, 2024
rollup/rollup_sync_service/rollup_sync_service.go Outdated Show resolved Hide resolved
@0xmountaintop 0xmountaintop merged commit 0360eba into develop Jul 17, 2024
8 checks passed
@0xmountaintop 0xmountaintop deleted the fix-l2geth-verifier-only-check-last-batch-of-the-bundle branch July 17, 2024 12:01
achmand pushed a commit to draganm/go-ethereum that referenced this pull request Aug 13, 2024
…h#904)

* fix(l2geth-verifier): only check the last batch of each bundle

* refactor function comments

* add more logs to debug

* fix the bug

* Revert "add more logs to debug"

This reverts commit 4befe9f.

* tweak

* TestValidateBatchInFinalizeByBundle

* update da-codec dependency

* move rawdb.WriteFinalizedBatchMeta outside the loop

* use batch writer

* change log.Crit to log.Error
lwedge99 pushed a commit to sentioxyz/scroll-geth that referenced this pull request Aug 27, 2024
…h#904)

* fix(l2geth-verifier): only check the last batch of each bundle

* refactor function comments

* add more logs to debug

* fix the bug

* Revert "add more logs to debug"

This reverts commit 4befe9f.

* tweak

* TestValidateBatchInFinalizeByBundle

* update da-codec dependency

* move rawdb.WriteFinalizedBatchMeta outside the loop

* use batch writer

* change log.Crit to log.Error
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