fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables #627

vyzo · 2024-01-31T14:35:30Z

1. Purpose or design rationale of this PR

When a user submits a transaction, we check that the user's balance is enough to cover gas fee + L1 data fee in the txpool.

However, if L1GasPriceOracle is updated, it's possible that the user's balance is not sufficient anymore. Txpool does not repeat this check. As a result, such transactions are treated as pending (executable), and are forwarded to the worker for inclusion in the next block. Then we discard them in the worker with an insufficient balance error.

Tracing and processing these transactions is wasted effort. Instead, we should drop such transactions in promoteExecutables.

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:

build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
docs: Documentation-only changes
feat: A new feature
fix: A bug fix
perf: A code change that improves performance
refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
test: Adding missing tests or correcting existing tests

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

vyzo · 2024-01-31T14:35:53Z

Needs a test.

vyzo · 2024-01-31T14:45:46Z

I don't know what's up with this "CI/goimports-lint" check failing, it is acting up; imports have not changed.

colinlyguo · 2024-01-31T16:31:53Z

I don't know what's up with this "CI/goimports-lint" check failing, it is acting up; imports have not changed.

it's a pending CI issue to be solved, can ignore it for now.

colinlyguo

LGTM. why not adding L1DataFee check in pool.demoteUnexecutables(), too?

vyzo · 2024-02-01T17:57:16Z

probably a good idea, @Thegaram thoughts?

vyzo · 2024-02-09T18:00:20Z

Implemented in demoteUnexecutables

colinlyguo

LGTM.

core/tx_list.go

Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>

core/tx_list.go

HAOYUatHZ

LGTM just a nitpick

core/tx_pool.go

core/tx_list.go

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

core/tx_pool.go

* feat(txpool): consider l1 data fee in costcap * fix CI * simplify logic * remove one db read op

colinlyguo · 2024-03-26T17:05:39Z

CI failed in another unit test, can temporarily ignore it.

colinlyguo · 2024-03-27T07:49:13Z

Would we deploy this PR in one of the nodes to see if the executable transactions in tx pool can be pruned? cc @georgehao @Thegaram

…bles (#627) * Check L1DataFee in txpool promoteExecutables * bump version * implement L1 data fee in demoteUnexecutables as well * Update core/tx_list.go Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> * Update core/tx_pool.go Co-authored-by: Péter Garamvölgyi <peter@scroll.io> * feat: consider l1 data fee in txpool costcap (#681) * feat(txpool): consider l1 data fee in costcap * fix CI * simplify logic * remove one db read op * bump version --------- Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> Co-authored-by: Péter Garamvölgyi <peter@scroll.io> Co-authored-by: georgehao <haohongfan@gmail.com>

…bles (#948) fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables (#627) * Check L1DataFee in txpool promoteExecutables * bump version * implement L1 data fee in demoteUnexecutables as well * Update core/tx_list.go * Update core/tx_pool.go * feat: consider l1 data fee in txpool costcap (#681) * feat(txpool): consider l1 data fee in costcap * fix CI * simplify logic * remove one db read op * bump version --------- Co-authored-by: vyzo <vyzo@hackzen.org> Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> Co-authored-by: Péter Garamvölgyi <peter@scroll.io> Co-authored-by: georgehao <haohongfan@gmail.com>

Check L1DataFee in txpool promoteExecutables

edb6474

vyzo requested a review from Thegaram January 31, 2024 14:35

bump version

17de481

vyzo requested a review from colinlyguo January 31, 2024 16:16

colinlyguo previously approved these changes Feb 1, 2024

View reviewed changes

implement L1 data fee in demoteUnexecutables as well

d7028e0

vyzo dismissed colinlyguo’s stale review via d7028e0 February 9, 2024 17:57

vyzo requested a review from colinlyguo February 9, 2024 18:00

Merge branch 'develop' into feat/l1-data-fee-toctou

8410840

colinlyguo changed the title ~~fix: Check L1DataFee in txpool promoteExecutables~~ fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables Feb 29, 2024

colinlyguo previously approved these changes Feb 29, 2024

View reviewed changes

core/tx_list.go Outdated Show resolved Hide resolved

colinlyguo requested review from georgehao and HAOYUatHZ March 1, 2024 07:39

Thegaram dismissed colinlyguo’s stale review via 2f1c7d8 March 4, 2024 13:41

Thegaram and others added 2 commits March 4, 2024 14:41

Update core/tx_list.go

2f1c7d8

Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com>

Merge branch 'develop' into feat/l1-data-fee-toctou

1581c09

colinlyguo previously approved these changes Mar 4, 2024

View reviewed changes

HAOYUatHZ reviewed Mar 11, 2024

View reviewed changes

core/tx_list.go Show resolved Hide resolved

HAOYUatHZ previously approved these changes Mar 11, 2024

View reviewed changes

Thegaram previously approved these changes Mar 25, 2024

View reviewed changes

core/tx_pool.go Outdated Show resolved Hide resolved

core/tx_list.go Outdated Show resolved Hide resolved

vyzo dismissed stale reviews from Thegaram, HAOYUatHZ, and colinlyguo via f8aff6a March 25, 2024 18:08

Update core/tx_pool.go

f8aff6a

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

georgehao reviewed Mar 26, 2024

View reviewed changes

core/tx_pool.go Outdated Show resolved Hide resolved

feat: consider l1 data fee in txpool costcap (#681)

8d383fb

* feat(txpool): consider l1 data fee in costcap * fix CI * simplify logic * remove one db read op

colinlyguo previously approved these changes Mar 26, 2024

View reviewed changes

Merge branch 'develop' into feat/l1-data-fee-toctou

4033cde

georgehao previously approved these changes Mar 26, 2024

View reviewed changes

Merge branch 'develop' into feat/l1-data-fee-toctou

617f792

Thegaram previously approved these changes Mar 27, 2024

View reviewed changes

bump version

02644ee

Thegaram dismissed stale reviews from colinlyguo, georgehao, and themself via 02644ee March 27, 2024 15:23

Thegaram approved these changes Mar 27, 2024

View reviewed changes

Thegaram merged commit d1e4b59 into develop Mar 27, 2024
6 checks passed

Thegaram deleted the feat/l1-data-fee-toctou branch March 27, 2024 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables #627

fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables #627

vyzo commented Jan 31, 2024 •

edited

Loading

vyzo commented Jan 31, 2024

vyzo commented Jan 31, 2024

colinlyguo commented Jan 31, 2024

colinlyguo left a comment

vyzo commented Feb 1, 2024

vyzo commented Feb 9, 2024

colinlyguo left a comment

HAOYUatHZ left a comment

colinlyguo commented Mar 26, 2024

colinlyguo commented Mar 27, 2024 •

edited

Loading

fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables #627

fix: Check L1DataFee in txpool promoteExecutables and demoteUnexecutables #627

Conversation

vyzo commented Jan 31, 2024 • edited Loading

1. Purpose or design rationale of this PR

2. PR title

3. Deployment tag versioning

4. Breaking change label

vyzo commented Jan 31, 2024

vyzo commented Jan 31, 2024

colinlyguo commented Jan 31, 2024

colinlyguo left a comment

Choose a reason for hiding this comment

vyzo commented Feb 1, 2024

vyzo commented Feb 9, 2024

colinlyguo left a comment

Choose a reason for hiding this comment

HAOYUatHZ left a comment

Choose a reason for hiding this comment

colinlyguo commented Mar 26, 2024

colinlyguo commented Mar 27, 2024 • edited Loading

vyzo commented Jan 31, 2024 •

edited

Loading

colinlyguo commented Mar 27, 2024 •

edited

Loading