-
Notifications
You must be signed in to change notification settings - Fork 595
TxPool: refactoring for blob txs #5953
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
Changes from all commits
d366741
53fcffa
2bc9be5
79588f0
1a25885
414fb8f
c508930
45115fa
c0db165
15087f4
c6ea9fe
d1c66c1
a6a65c5
5cf90f5
2e91dcb
b38a40d
7f3df36
a3d86d7
109cca4
0b8f72d
5a20486
5a6b7fe
a6e1e16
6596c1d
07870f1
73872e1
b11a27d
5da9488
3071be3
9cc03cc
761a6c5
028f650
1b47921
8145f44
1d3ec8d
6201cae
56d9dd4
091bde6
d7daff3
fb54eb9
d8cf261
647873f
5fdff5a
12e9b9a
53f4415
e646940
dbd8890
82a5110
44b145e
5a6cf30
771858c
a822d6f
f4df112
965a003
6f045a6
c3fbde1
4aa0e0e
a91b8d8
0ea2bc5
17e997a
c41717f
7c71180
ccc93e0
3d3fe14
78de5be
4923d2e
e515f32
58fc2fd
edd4fca
f8801b9
c98b83c
001fa5c
0fbc778
185b22d
06c3b53
5fbf208
93d00b1
b8030ce
b1b2c15
fc8e258
a0bd04b
ff01c02
40456ec
58e8b96
a026341
bb6a6b4
768ba88
2901068
69cbeba
4eee1cf
a35b8af
29c30b9
6b4a438
2ff8309
890ee24
9b36393
288b0e3
e43ad49
0154927
b3a5ffe
22d8de7
2488460
5b78cf4
ee4dfb1
d0bc319
6ffeb44
127e1b5
59fb8a3
416ab31
c236326
056fb15
1b5c1cc
051c89b
d16bf52
6f211b6
96352c8
8f6b3f4
b100533
7de7fe6
bbe23e3
cc33167
8173c81
6195466
ee1891b
d2131bd
ae671d4
ccc1799
d535ae1
0bb5cf0
3040dfa
5413f84
84e8e79
f2b284d
75163e7
8f9980e
ce6be6c
0ade121
a4d3600
918c7b6
3b63ce5
f43c8a0
e6d1e46
1a928e6
d2e5904
c50c44b
16d99b3
188de15
a19c4ce
a804f51
f9e7030
7cd6d1b
ca5a37e
8e2ce35
1542e2e
574725d
91d420c
eeb9b9b
76b6e25
3b150cd
29f73f1
2df4ae2
d90e4a2
544dc68
9700c48
222cd92
70babe9
4e26565
fb97b14
112fb91
001be21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Linq; | ||
| using System.Runtime.CompilerServices; | ||
| using Nethermind.Consensus.Comparers; | ||
|
|
@@ -46,26 +47,26 @@ public TxPoolTxSource( | |
| public IEnumerable<Transaction> GetTransactions(BlockHeader parent, long gasLimit) | ||
| { | ||
| long blockNumber = parent.Number + 1; | ||
| IEip1559Spec specFor1559 = _specProvider.GetSpecFor1559(blockNumber); | ||
| UInt256 baseFee = BaseFeeCalculator.Calculate(parent, specFor1559); | ||
| IReleaseSpec spec = _specProvider.GetSpec(parent); | ||
| UInt256 baseFee = BaseFeeCalculator.Calculate(parent, spec); | ||
| IDictionary<Address, Transaction[]> pendingTransactions = _transactionPool.GetPendingTransactionsBySender(); | ||
| IDictionary<Address, Transaction[]> pendingBlobTransactionsEquivalences = _transactionPool.GetPendingLightBlobTransactionsBySender(); | ||
| IComparer<Transaction> comparer = GetComparer(parent, new BlockPreparationContext(baseFee, blockNumber)) | ||
| .ThenBy(ByHashTxComparer.Instance); // in order to sort properly and not lose transactions we need to differentiate on their identity which provided comparer might not be doing | ||
|
|
||
| IEnumerable<Transaction> transactions = GetOrderedTransactions(pendingTransactions, comparer); | ||
| IEnumerable<Transaction> blobTransactions = GetOrderedTransactions(pendingBlobTransactionsEquivalences, comparer); | ||
| if (_logger.IsDebug) _logger.Debug($"Collecting pending transactions at block gas limit {gasLimit}."); | ||
|
|
||
| int checkedTransactions = 0; | ||
| int selectedTransactions = 0; | ||
| int i = 0; | ||
| using ArrayPoolList<Transaction> selectedBlobTxs = new(Eip4844Constants.MaxBlobsPerBlock); | ||
marcindsobczak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // TODO: removing transactions from TX pool here seems to be a bad practice since they will | ||
| // not come back if the block is ignored? | ||
| int blobsCounter = 0; | ||
| UInt256 blobGasPrice = UInt256.Zero; | ||
| SelectBlobTransactions(blobTransactions, parent, spec, selectedBlobTxs); | ||
|
|
||
| foreach (Transaction tx in transactions) | ||
| { | ||
| i++; | ||
| checkedTransactions++; | ||
|
|
||
| if (tx.SenderAddress is null) | ||
| { | ||
|
|
@@ -77,52 +78,130 @@ public IEnumerable<Transaction> GetTransactions(BlockHeader parent, long gasLimi | |
| bool success = _txFilterPipeline.Execute(tx, parent); | ||
| if (!success) continue; | ||
|
|
||
| if (tx.SupportsBlobs) | ||
| foreach (Transaction blobTx in PickBlobTxsBetterThanCurrentTx(selectedBlobTxs, tx, comparer)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we can do it in more efficient/simpler way - let's discuss it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's already simplified a lot comparing to initial version 😄 But I'm open for discussion. What is important here, we need to have all transactions (mix of blob and non-blob) in correct order, so we can't add blobs at the beginning or at the end.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if it's needed cos is already fast enough. But if we could sort SelectBlobTransactions firstly and pass it as iterator to PickBlobTxsBetterThanCurrentTx it might save some cycles, because it's seems more efficient to merge already sorted datas.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is already sorted. |
||
| { | ||
| if (blobGasPrice.IsZero) | ||
| { | ||
| ulong? excessBlobGas = BlobGasCalculator.CalculateExcessBlobGas(parent, _specProvider.GetSpec(parent)); | ||
| if (excessBlobGas is null) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {tx.ToShortString()}, the specification is not configured to handle shard blob transactions."); | ||
| continue; | ||
| } | ||
| if (!BlobGasCalculator.TryCalculateBlobGasPricePerUnit(excessBlobGas.Value, out blobGasPrice)) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {tx.ToShortString()}, failed to calculate blob gas price."); | ||
| continue; | ||
| } | ||
| } | ||
| yield return blobTx; | ||
| } | ||
|
|
||
| int txAmountOfBlobs = tx.BlobVersionedHashes?.Length ?? 0; | ||
| if (_logger.IsTrace) _logger.Trace($"Selected {tx.ToShortString()} to be potentially included in block."); | ||
|
|
||
| if (blobGasPrice > tx.MaxFeePerBlobGas) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {tx.ToShortString()}, blob gas fee is too low."); | ||
| continue; | ||
| } | ||
| selectedTransactions++; | ||
| yield return tx; | ||
| } | ||
|
|
||
| if (BlobGasCalculator.CalculateBlobGas(blobsCounter + txAmountOfBlobs) > | ||
| Eip4844Constants.MaxBlobGasPerBlock) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {tx.ToShortString()}, no more blob space."); | ||
| continue; | ||
| } | ||
| if (selectedBlobTxs.Count > 0) | ||
| { | ||
| foreach (Transaction blobTx in selectedBlobTxs) | ||
| { | ||
| yield return blobTx; | ||
LukaszRozmej marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| if (_logger.IsDebug) _logger.Debug($"Potentially selected {selectedTransactions} out of {checkedTransactions} pending transactions checked."); | ||
| } | ||
|
|
||
| blobsCounter += txAmountOfBlobs; | ||
| if (_logger.IsTrace) _logger.Trace($"Selected shard blob tx {tx.ToShortString()} to be potentially included in block, total blobs included: {blobsCounter}."); | ||
| private IEnumerable<Transaction> PickBlobTxsBetterThanCurrentTx(ArrayPoolList<Transaction> selectedBlobTxs, Transaction tx, IComparer<Transaction> comparer) | ||
| { | ||
| while (selectedBlobTxs.Count > 0) | ||
| { | ||
| Transaction blobTx = selectedBlobTxs[0]; | ||
| if (comparer.Compare(blobTx, tx) > 0) | ||
| { | ||
| yield return blobTx; | ||
| selectedBlobTxs.Remove(blobTx); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you have to shift all the blob transactions each time, would be better to keep blob transactions in reverse order and remove last |
||
| } | ||
| else | ||
| { | ||
| if (_logger.IsTrace) | ||
| _logger.Trace($"Selected {tx.ToShortString()} to be potentially included in block."); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| selectedTransactions++; | ||
| yield return tx; | ||
| private void SelectBlobTransactions(IEnumerable<Transaction> blobTransactions, BlockHeader parent, IReleaseSpec spec, ArrayPoolList<Transaction> selectedBlobTxs) | ||
| { | ||
| int checkedBlobTransactions = 0; | ||
| int selectedBlobTransactions = 0; | ||
| int blobsCounter = 0; | ||
| UInt256 blobGasPrice = UInt256.Zero; | ||
|
|
||
| foreach (Transaction blobTx in blobTransactions) | ||
| { | ||
| if (blobsCounter == Eip4844Constants.MaxBlobsPerBlock) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {blobTx.ToShortString()}, no more blob space. Block already have {blobsCounter} which is max value allowed."); | ||
| break; | ||
| } | ||
|
|
||
| checkedBlobTransactions++; | ||
|
|
||
| int txAmountOfBlobs = blobTx.BlobVersionedHashes?.Length ?? 0; | ||
| if (blobsCounter + txAmountOfBlobs > Eip4844Constants.MaxBlobsPerBlock) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {blobTx.ToShortString()}, not enough blob space."); | ||
| continue; | ||
| } | ||
|
|
||
| if (blobGasPrice.IsZero && !TryUpdateBlobGasPrice(blobTx, parent, spec, out blobGasPrice)) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {blobTx.ToShortString()}, failed to get full version of this blob tx from TxPool."); | ||
| continue; | ||
| } | ||
|
|
||
| if (blobGasPrice > blobTx.MaxFeePerBlobGas) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {blobTx.ToShortString()}, data gas fee is too low."); | ||
| continue; | ||
| } | ||
|
|
||
| bool success = _txFilterPipeline.Execute(blobTx, parent); | ||
| if (!success) continue; | ||
|
|
||
| if (!TryGetFullBlobTx(blobTx, out Transaction fullBlobTx)) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {blobTx.ToShortString()}, failed to get full version of this blob tx from TxPool."); | ||
| continue; | ||
| } | ||
|
|
||
| blobsCounter += txAmountOfBlobs; | ||
| if (_logger.IsTrace) _logger.Trace($"Selected shard blob tx {fullBlobTx.ToShortString()} to be potentially included in block, total blobs included: {blobsCounter}."); | ||
|
|
||
| selectedBlobTransactions++; | ||
| selectedBlobTxs.Add(fullBlobTx); | ||
| } | ||
|
|
||
| if (_logger.IsDebug) _logger.Debug($"Potentially selected {selectedBlobTransactions} out of {checkedBlobTransactions} pending blob transactions checked."); | ||
| } | ||
|
|
||
| private bool TryGetFullBlobTx(Transaction blobTx, [NotNullWhen(true)] out Transaction? fullBlobTx) | ||
| { | ||
| if (blobTx.NetworkWrapper is not null) | ||
| { | ||
| fullBlobTx = blobTx; | ||
| return true; | ||
| } | ||
|
|
||
| fullBlobTx = null; | ||
| return blobTx.Hash is not null && _transactionPool.TryGetPendingBlobTransaction(blobTx.Hash, out fullBlobTx); | ||
| } | ||
|
|
||
| private bool TryUpdateBlobGasPrice(Transaction lightBlobTx, BlockHeader parent, IReleaseSpec spec, out UInt256 blobGasPrice) | ||
| { | ||
| ulong? excessDataGas = BlobGasCalculator.CalculateExcessBlobGas(parent, spec); | ||
| if (excessDataGas is null) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {lightBlobTx.ToShortString()}, the specification is not configured to handle shard blob transactions."); | ||
| blobGasPrice = UInt256.Zero; | ||
| return false; | ||
| } | ||
|
|
||
| if (!BlobGasCalculator.TryCalculateBlobGasPricePerUnit(excessDataGas.Value, out blobGasPrice)) | ||
| { | ||
| if (_logger.IsTrace) _logger.Trace($"Declining {lightBlobTx.ToShortString()}, failed to calculate data gas price."); | ||
| blobGasPrice = UInt256.Zero; | ||
| return false; | ||
| } | ||
|
|
||
| if (_logger.IsDebug) _logger.Debug($"Potentially selected {selectedTransactions} out of {i} pending transactions checked."); | ||
| return true; | ||
| } | ||
|
|
||
| protected virtual IEnumerable<Transaction> GetOrderedTransactions(IDictionary<Address, Transaction[]> pendingTransactions, IComparer<Transaction> comparer) => | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.