-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Unqueue invalid transactions when skipping #13918
Unqueue invalid transactions when skipping #13918
Conversation
The check was intially added by: #5121 But a later pr fixed it properly: #9789 TLDR: We don't need to check for skipped anymore, as we already remove all transactions that are being unlocked by an invalid transactions and thus, we will not tag them as invalid directly because the nonce for example is incorrect. Fixes: #13911
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 modification looks good.
I understand that we don't require to check if skipped > 0
before removal as all the dependent txs were already skipped by the iterator when calling report_invalid
for a dependency. I.e. the iterator doesn't return dependent txs (introduced by this #9789).
But I don't get (probably for some lacks of my knowledge on the inner machinery) how this fixes : #13911
I mean, if we have a bunch of validated txs in the pool and we upgrade the runtime. How this modification ends up removing the ones that would panic?
Thank you
Because they would panic while applying them, aka return an error here and then we unqueue them. |
I see. So we remove them after the fact. Thank you |
The check was intially added by: paritytech#5121 But a later pr fixed it properly: paritytech#9789 TLDR: We don't need to check for skipped anymore, as we already remove all transactions that are being unlocked by an invalid transactions and thus, we will not tag them as invalid directly because the nonce for example is incorrect. Fixes: paritytech#13911
The check was intially added by: #5121
But a later pr fixed it properly: #9789
TLDR: We don't need to check for skipped anymore, as we already remove all transactions that are being unlocked by an invalid transactions and thus, we will not tag them as invalid directly because the nonce for example is incorrect.
Fixes: #13911