Skip to content
This repository has been archived by the owner on Sep 23, 2023. It is now read-only.

Txpool upgrades for EIP-4844 Blob Transactions #1075

Merged
merged 32 commits into from
Sep 10, 2023

Conversation

somnathb1
Copy link
Collaborator

@somnathb1 somnathb1 commented Aug 11, 2023

EIP-4844 proposed the new (type-3) Blob Transactions which are expensive (because blobs are big etc.). The tx-pool should be tightened to alleviate spam and dos attacks by way of these new transactions.

Changes

Keep blob txs cached
During new block, don't delete mined blobTxs, instead cache it till they are finalized. This is to avoid fetching them again for unwind. During unwind, if it's a previously seen blobTxn, try to fetch it from memory.

Pool config
BlobSlots: Similar to AccountSlots - limits how many blobs a single account is allowed to have until it's marked as spamming.
Blob txn priceBump: It's an added field in pool config (with a flag). Defaults to 100%, since blob txs of equal fee are costlier to process/execute

@somnathb1 somnathb1 marked this pull request as ready for review September 8, 2023 17:03
@somnathb1 somnathb1 requested review from yperbasis and AskAlexSharov and removed request for yperbasis September 8, 2023 17:06
@somnathb1
Copy link
Collaborator Author

somnathb1 commented Sep 8, 2023

Depends on ledgerwatch/interfaces#194

@somnathb1 somnathb1 added this pull request to the merge queue Sep 10, 2023
Merged via the queue into ledgerwatch:main with commit adef971 Sep 10, 2023
3 checks passed
@somnathb1 somnathb1 deleted the txpool-4844-upgrades branch September 10, 2023 10:24
@AskAlexSharov
Copy link
Collaborator

please don't merge erigon-lib PR if Erigon's CI is red. see: https://github.com/ledgerwatch/erigon-lib#dev-workflow

AskAlexSharov pushed a commit to ledgerwatch/erigon that referenced this pull request Sep 11, 2023
PriceBump: 10, // Price bump percentage to replace an already existing transaction
MinFeeCap: 1,
AccountSlots: 16, //TODO: to choose right value (16 to be compatible with Geth)
BlobSlots: 48, // Allows 8 txs of 6 blobs each - for hive tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO: double check if we really need than many for Hive

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It turns out we do, but I added a flag for this. So will reduce the default. Not sure if normally many users should be sending many blobs and node operators in need could tune their nodes accordingly.


//Get the config of the pool for BlobPriceBump and bump all prices
requiredPriceBump := pool.cfg.BlobPriceBump
blobTxn.Tip.MulDivOverflow(tip, uint256.NewInt(requiredPriceBump+100), uint256.NewInt(100))
Copy link
Collaborator

@yperbasis yperbasis Sep 12, 2023

Choose a reason for hiding this comment

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

Probably FeeCap & BlobFeeCap should be bumped by different amounts

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This two-dimensionality for replace txs could a little confusing, albeit not for target users. The rationale for continuing with a one-dimensional price bump (for 2D pricing) is that blob fee is burn-only and not paid to the node, while bumping both fields just makes the requirement to "go aggressive, or wait".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants