Skip to content

Conversation

@tgmichel
Copy link
Contributor

In Kind::NewPendingTransactions, current approach of subscribing to storage changes t is not good for many reasons, and will lead to problems because:

  • currently it does not make use of the schema overrides. This is not critical, as subscriptions are "from now on", but still incorrect as may force to do a client upgrade when doing a runtime upgrade that affects that storage item.
  • Moreover, is coupled to the pallet-ethereum storage when this has nothing to do with storage.
  • pallet-ethereum::Pending data is appended when the transact dispatchable is actually dispatched, not when the unsigned extrinsic is preliminarily added to the pool. The pending transaction WS subscription is supposed to notify the subscriber when the transaction is in the ready queue, not when whatever we do in pallet-ethereum.

Instead we want to:

  • Listen for TransactionPool notifications. Internally this will use the ValidatedPool to get transactions that are on the ready queue.
  • This will get us a TxHash, which we can use to get the extrinsic.
  • Then use the runtime to get back a known type we can deal with (ethereum::Transaction).

Then hash it as we do now and notify the subscriber.

Note

This includes some duplicated code (runtime api method) with #403 to illustrate how it works, it will be fused once one of them goes through.

@tgmichel tgmichel marked this pull request as ready for review June 18, 2021 13:44
@tgmichel tgmichel requested a review from sorpaas as a code owner June 18, 2021 13:44
@sorpaas sorpaas merged commit 0d7b34d into polkadot-evm:master Jun 29, 2021
@tgmichel tgmichel deleted the tgm-pubsub-pending-txs-v2 branch April 1, 2022 09:54
abhijeetbhagat pushed a commit to web3labs/frontier that referenced this pull request Jan 11, 2023
* Use `TransactionPool` notification stream in pusbub

* Cleanup

* Update ts test to match new expected behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants