New filter restricting delegations in txpool #8022
Conversation
ak88
changed the title
| return AcceptTxResult.Accepted; | ||
| Transaction[] currentTxs; | ||
|
|
||
| if (standardPool.TryGetBucket(tx.SenderAddress!, out currentTxs) || blobPool.TryGetBucket(tx.SenderAddress!, out currentTxs)) |
There was a problem hiding this comment.
TryGetBucket locks and then copies all the transactions. Maybe we can have some kind of SearchInBucket(Predicate) where we could avoid creating the array?
There was a problem hiding this comment.
Alternatively returning a pooled array instead? Keeping the lock while searching inside the bucket, could have other side effects.
@benaadams what do you think?
There was a problem hiding this comment.
Should keep the lock if changing the txs in the bucket? What if another tx came in from same sender doing same thing?
Why is it or?
If there are regular txs does it not matter if are blob txs? And do the blob txs only matter if not regular ones?
There was a problem hiding this comment.
Though probably lock at bucket level rather than whole pool
There was a problem hiding this comment.
Should keep the lock if changing the txs in the bucket? What if another tx came in from same sender doing same thing?
Why is it or?
If there are regular txs does it not matter if are blob txs? And do the blob txs only matter if not regular ones?
-
The idea is not to change the bucket at all, only to check if the incoming transaction is a replacement, in which case it should be accepted.
-
Its not possible to have a blob transaction and another type in the pool at the same time.
There was a problem hiding this comment.
Do you have any suggestions for Lukasz comment?
There was a problem hiding this comment.
The search below is very fast. I would rather have this search under the lock as I think it won't be much slower than allocating an array?
| if (tx.HasAuthorizationList) | ||
| { | ||
| foreach (var auth in tx.AuthorizationList) | ||
| { | ||
| if (auth.Authority is not null) | ||
| _pendingDelegations.IncrementDelegationCount(auth.Authority!, auth.Nonce); | ||
| } | ||
| } |
There was a problem hiding this comment.
Let's have AddPendingDelegations for symmetry
| return AcceptTxResult.Accepted; | ||
| Transaction[] currentTxs; | ||
|
|
||
| if (standardPool.TryGetBucket(tx.SenderAddress!, out currentTxs) || blobPool.TryGetBucket(tx.SenderAddress!, out currentTxs)) |
There was a problem hiding this comment.
The search below is very fast. I would rather have this search under the lock as I think it won't be much slower than allocating an array?
Co-authored-by: Lukasz Rozmej <[email protected]>
| @@ -530,6 +530,14 @@ public bool TryGetBucketsWorstValue(TGroupKey groupKey, out TValue? item) | |||
| return false; | |||
| } | |||
|
|
|||
| public bool BucketEmptyExcept(TGroupKey groupKey, Func<TValue, bool> predicate) | |||
There was a problem hiding this comment.
Maybe better name BucketAny?
This is a new filter for tx pool, introducing limitations to delegated accounts.
Originally mitigations are described in the EIP itself:
https://eips.ethereum.org/EIPS/eip-7702#transaction-propagation
As the EIP suggests this will limit delegated accounts to only one transaction in the pool at a time.
Also will restrict any transaction with the same nonce and source as a pending authorization tuple in the pool.