-
Notifications
You must be signed in to change notification settings - Fork 844
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
Keep track of blobs that are part of multiple transactions #7723
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically what you need is to count how many txs reference this blob, so it will be simpler and more performant to just keep a counter instead of a list
@@ -107,8 +108,8 @@ public class TransactionPool implements BlockAddedObserver { | |||
private final SaveRestoreManager saveRestoreManager = new SaveRestoreManager(); | |||
private final Set<Address> localSenders = ConcurrentHashMap.newKeySet(); | |||
private final EthScheduler.OrderedProcessor<BlockAddedEvent> blockAddedEventOrderedProcessor; | |||
private final Map<VersionedHash, BlobsWithCommitments.BlobQuad> mapOfBlobsInTransactionPool = | |||
new HashMap<>(); | |||
private final Map<VersionedHash, List<BlobsWithCommitments.BlobQuad>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need to support concurrency, since notifications are sent async by potentially multiple threads
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The access to the map was synchronized, but using a map implementation that does the synchronization for me is probably a better idea!
My first idea was to have a reference count, but this has the drawback that you potentially keep an additional blob in memory, while the list has the drawback of using a little more storage and it is a little slower. |
ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java
Outdated
Show resolved
Hide resolved
…ransactions/TransactionPool.java initial size to 1 Signed-off-by: Stefan Pingel <[email protected]>
PR description
The same blob can be part of multiple transactions. This PR makes sure that these blobs can be returned by
eth_getBlobsV1
even when only one transaction containing that blob is still in the transaction pool.