Skip to content

Fix restoring txpool from disk when blob transactions are present#8481

Merged
fab-10 merged 1 commit intobesu-eth:mainfrom
fab-10:fix-txpool-restore-with-blobs
Mar 28, 2025
Merged

Fix restoring txpool from disk when blob transactions are present#8481
fab-10 merged 1 commit intobesu-eth:mainfrom
fab-10:fix-txpool-restore-with-blobs

Conversation

@fab-10
Copy link
Copy Markdown
Contributor

@fab-10 fab-10 commented Mar 27, 2025

PR description

Fix to use the POOLED_TRANSACTION format also when reading from disk to fix the issue related to saving blob tx with that format but reading them with the BLOCK_BODY format.

Writing a test for this, I found that we where not correctly supporting the enable/disable feature in the unit tests, so that required a bit of refactoring on the test code.

Fixed Issue(s)

fixes #8449

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@fab-10 fab-10 changed the title Fix restoring txpool from disk with blob transactions Fix restoring txpool from disk when blob transactions are present Mar 27, 2025
@fab-10 fab-10 force-pushed the fix-txpool-restore-with-blobs branch from abc86ca to e8fbfc7 Compare March 27, 2025 15:10
@fab-10 fab-10 marked this pull request as ready for review March 27, 2025 15:11

clearInvocations(transactionValidatorFactory);

// trying to readd the same tx should return transaction already known and no
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// trying to readd the same tx should return transaction already known and no
// trying to read the same tx should return transaction already known and no

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is re-add, done


// re-enabling the txpool restores from file
transactionPool.setEnabled().get(10, TimeUnit.SECONDS);
transactionPool.setEnabled().get(10000, TimeUnit.SECONDS);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

10000 seconds?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch, debug left over reverted

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 force-pushed the fix-txpool-restore-with-blobs branch from e8fbfc7 to db6ac73 Compare March 28, 2025 09:12
@fab-10 fab-10 enabled auto-merge (squash) March 28, 2025 09:13
@fab-10 fab-10 merged commit 303c345 into besu-eth:main Mar 28, 2025
43 checks passed
@fab-10 fab-10 deleted the fix-txpool-restore-with-blobs branch March 28, 2025 09:45
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.

Regression in txpool dump and restore on v25.3.0

3 participants