Skip to content

Fix miner.TestDAFilters #399

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.

Sign up for GitHub

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

Jump to bottom
Merged
piersy merged 1 commit into from
Jul 2, 2025
Merged

Fix miner.TestDAFilters #399

piersy merged 1 commit into from
Jul 2, 2025

Conversation

@piersy
Copy link

@piersy piersy commented Jul 2, 2025 •
edited
Loading

This fixes a synchronisation error in miner.TestDAFilters transactions were being added to the pool asynchronously and this introduced race conditions that could cause the test to fail randomly, running with a count of 1000 is enough to generate a few failures:

go test ./miner/ -run TestDAFilters -count 1000
--- FAIL: TestDAFilters (0.00s)
    --- FAIL: TestDAFilters/with-tx-filter-max-too-high (0.02s)
        payload_building_test.go:345: Unexpected transaction set: got 1, expected 257
--- FAIL: TestDAFilters (0.00s)
    --- FAIL: TestDAFilters/with-block-filter-max-too-high (0.03s)
        payload_building_test.go:345: Unexpected transaction set: got 1, expected 257
--- FAIL: TestDAFilters (0.00s)
    --- FAIL: TestDAFilters/with-tx-filter-max-too-high (0.02s)
        payload_building_test.go:345: Unexpected transaction set: got 1, expected 257
--- FAIL: TestDAFilters (0.00s)
    --- FAIL: TestDAFilters/with-tx-filter-max-too-high (0.02s)
        payload_building_test.go:345: Unexpected transaction set: got 1, expected 257
--- FAIL: TestDAFilters (0.00s)
    --- FAIL: TestDAFilters/with-nil-tx-filters (0.05s)
        payload_building_test.go:345: Unexpected transaction set: got 1, expected 257
FAIL
FAIL    github.com/ethereum/go-ethereum/miner   28.241s
FAIL

Fixes https://github.com/celo-org/celo-blockchain-planning/issues/1027

@piersy piersy requested a review from karlb July 2, 2025 13:44
karlb
karlb approved these changes Jul 2, 2025
View reviewed changes
Copy link

@karlb karlb left a comment

Choose a reason for hiding this comment

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

Sounds reasonable. We should upstream that fix, since it is not yet fixed there either.

@piersy
Copy link
Author

piersy commented Jul 2, 2025

Sounds reasonable. We should upstream that fix, since it is not yet fixed there either.

Yep, upstream fix is pending - ethereum-optimism#637

@piersy piersy merged commit 0196a3b into celo-rebase-13 Jul 2, 2025
7 checks passed
@piersy piersy deleted the piersy/fix_TestDAFilters branch July 2, 2025 14:28
Kourin1996 pushed a commit that referenced this pull request Jul 20, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
e5c79fb
Kourin1996 pushed a commit that referenced this pull request Jul 21, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
880f303
Kourin1996 pushed a commit that referenced this pull request Jul 26, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
74eede5
Kourin1996 pushed a commit that referenced this pull request Jul 26, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
4f30e0c
Kourin1996 pushed a commit that referenced this pull request Jul 28, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
a23ecf6
Kourin1996 pushed a commit that referenced this pull request Jul 30, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
2d29f95
Kourin1996 pushed a commit that referenced this pull request Jul 30, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
7f4a7c6
Kourin1996 pushed a commit that referenced this pull request Jul 30, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
7bace84
Kourin1996 pushed a commit that referenced this pull request Jul 31, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
288a036
Kourin1996 pushed a commit that referenced this pull request Jul 31, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
e0d4026
Kourin1996 pushed a commit that referenced this pull request Jul 31, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
76af691
Kourin1996 pushed a commit that referenced this pull request Aug 1, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
73ff39a
Kourin1996 pushed a commit that referenced this pull request Aug 1, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
c39c305
Kourin1996 pushed a commit that referenced this pull request Aug 1, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
b2b00e5
Kourin1996 pushed a commit that referenced this pull request Aug 3, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
9aab200
Kourin1996 pushed a commit that referenced this pull request Aug 4, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
d77c311
Kourin1996 pushed a commit that referenced this pull request Aug 4, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
d321bd2
Kourin1996 pushed a commit that referenced this pull request Aug 4, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
2cb7607
Kourin1996 pushed a commit that referenced this pull request Aug 4, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
5b743f8
Kourin1996 pushed a commit that referenced this pull request Aug 5, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
de255ca
Kourin1996 pushed a commit that referenced this pull request Aug 5, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
9368e98
Kourin1996 pushed a commit that referenced this pull request Aug 7, 2025
@piersy @Kourin1996
miner: Ensure test adds transactions synchronously (#399)
0dff50f
karlb pushed a commit that referenced this pull request Nov 5, 2025
@piersy @karlb
miner: Ensure test adds transactions synchronously (#399)
794a1e4
karlb pushed a commit that referenced this pull request Dec 5, 2025
@piersy @karlb
miner: Ensure test adds transactions synchronously (#399)
545d7f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@karlb karlb karlb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@piersy @karlb