Skip to content

feat(rollup): add fallback if TransactionByHash fails in getting commitBatch calldata #601

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

Conversation

colinlyguo
Copy link
Member

@colinlyguo colinlyguo commented Dec 14, 2023

1. Purpose or design rationale of this PR

The transaction indexing of a geth node is controlled by --txlookuplimit flag, which indicates the number of recent blocks to maintain transactions index, thus in some cases, the l2geth node cannot successfully fetch old transactions to get commitBatch's calldata. To solve this issue, this PR:

  1. Add fallback if TransactionByHash fails in getting commitBatch calldata.
  2. Add more runtime logs for easier debugging.

Related issues and discussions:
TransactionByHash returns null
TransactionReceipt returns null

Local test (you may need to tweak some code to go into fallback path):

make nccc_geth
./build/bin/geth --scroll --verbosity 3 --datadir "./datadir" \
    --http --http.addr "0.0.0.0" --http.port 8545 --http.api "eth,net,web3,debug,scroll" \
    --l1.endpoint "L1 URL" --rollup.verify

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:

  • feat: A new feature

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 added bug Something isn't working feature New feature or request bump-version labels Dec 14, 2023
@colinlyguo colinlyguo self-assigned this Dec 14, 2023
Thegaram
Thegaram previously approved these changes Dec 15, 2023
@colinlyguo colinlyguo requested a review from Thegaram December 15, 2023 13:46
@Thegaram Thegaram merged commit b235afb into develop Dec 18, 2023
@Thegaram Thegaram deleted the feat-add-fallback-if-rollup-sync-service-get-tx-by-hash-failed branch December 18, 2023 13:51
0xmountaintop pushed a commit that referenced this pull request Jun 26, 2024
…itBatch calldata (#601)

* feat(rollup): add fallback if TransactionByHash fails in getting commitBatch calldata

* Update rollup/rollup_sync_service/rollup_sync_service.go

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* change log.Warn to log.Debug

---------

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
0xmountaintop added a commit that referenced this pull request Jun 26, 2024
* feat(rollup): add fallback if TransactionByHash fails in getting commitBatch calldata (#601)

* feat(rollup): add fallback if TransactionByHash fails in getting commitBatch calldata

* Update rollup/rollup_sync_service/rollup_sync_service.go

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* change log.Warn to log.Debug

---------

Co-authored-by: Péter Garamvölgyi <peter@scroll.io>

* fix(rollup sync service): remove syscall.Kill (#636)

* fix(rollup sync service): remove syscall.Kill

* bump version

* fix(api-backend): disable finalized tag when rollup verify is disabled (#722)

---------

Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>
Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bump-version feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants