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

feat: store skipped transactions in local db #467

Merged
merged 27 commits into from
Aug 25, 2023

Conversation

Thegaram
Copy link

@Thegaram Thegaram commented Aug 17, 2023

1. Purpose or design rationale of this PR

This PR addresses the following problems:

  • It is hard to detect which transactions were skipped.
  • We need an easy way to check why a transaction was skipped. Now we log this information but the logs will be lost after a node is restarted.
  • When we skip an L2 transaction, it's dropped completely. This makes it very hard to inspect this transaction.

To solve this, this PR adds the following:

  • Store skipped transactions in the node's local db. We store 3 things: (1) For each skipped tx, we store hash => Transaction. (2) For each skipped transaction, we store skip index => TxHash (where skip index is a simple counter). (3) We store the count of skipped transactions.
  • Expose 3 new APIs: scroll_getNumSkippedTransactions(), scroll_getSkippedTransaction(txHash), scroll_getSkippedTransactionHashes(fromIndex, toIndex). These are also conveniently exposed on the geth console.
  • Populate the DB in worker for signer nodes, here we have detailed skip reasons. Populate the DB in block_validator for follower nodes, here we don't have the skip reason.

Caveats:

  • If a node upgrades to this version, it will not re-index old skipped messages. For example, NumSkippedTransactions will be lower than the exact count.
  • Signer and follower nodes will have different views. This includes different skip reasons. The information about which block skipped the message will also be different.

That said, these APIs are mainly for easier debugging, so these potential inconsistencies are acceptable.

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?

  • 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

HAOYUatHZ
HAOYUatHZ previously approved these changes Aug 20, 2023
Copy link
Member

@icemelon icemelon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. just one minor comment

miner/worker.go Outdated Show resolved Hide resolved
mask-pp and others added 9 commits August 25, 2023 21:13
* fix bug when calculate l2 tx count

* Update version
* fix: exclude L1 message from block payload size validation

* fix the bug when calculating l2TxCount. (#479)

* fix bug when calculate l2 tx count

* Update version

* bump version

---------

Co-authored-by: maskpp <maskpp266@gmail.com>
* Fix row estimation.

* Update libzkp.

* more

* prepare

* finish

* upgrade

* bump version

* fix tests

* Update to scroll-prover `v0.7.2`.

* fix tests

* Update miner/worker.go

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

* Update miner/worker.go

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

* Reset ccc when skips first tx.

* do not unnecessarily skip L1 message

* fix ccc reset and improve code readability

* seal block on circuitcapacitychecker.ErrUnknown

---------

Co-authored-by: HAOYUatHZ <haoyu@protonmail.com>
Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
* simplify ccc revert to snapshot

* Update version.go

---------

Co-authored-by: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com>
* feat: use --gcmode=archive and --cache.noprefetch=true by default

* refuse to start with invalid config

* lint

* lint
* enable use compression algorithm

* fix ci

* Just enable decode compressed content at ethclient

* fix comments

---------

Co-authored-by: Haichen Shen <shenhaichen@gmail.com>
Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>
* Update libzkp to use scroll-prover `v0.7.5`.

* Update version.
@Thegaram Thegaram merged commit 7de261b into develop Aug 25, 2023
5 checks passed
@Thegaram Thegaram deleted the store-skipped-transactions branch August 25, 2023 13:41
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.

6 participants