paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.62k stars 561 forks source link

message queue regression compare to dmp queue #3709

Open xlc opened 3 months ago

xlc commented 3 months ago

We detected a minor regression when migrate from using dmpQueue to messageQeueue that the incoming XCM are executed one block later.

A brief check of the code shows that previously with dmpQueue pallet, it will try to execute the message immediately in the message handler. However, for messageQueue, it only enqueue the message. The messages in the queue are only dispatched in on_initialize hook, but the messages are enqueued in the inherent, after the on_initialize hook.

This means all the incoming XCM are going to be stored in storage for at least one block before they are dispatched. This seems inefficient. As it have to write the XCM and then read and remove it on the next block. Compare to what we have currently that it will just dispatch it and no need those extra storage access. And the message get dispatched faster.

This behaviour also makes e2e test lot more complicated as there will be some uncertainty on when the message will be dispatched. Hardcoded it to be 2 blocks later just sounds like a poor workaround.

I am not sure what's the best way to address such issue. Make it try to dispatch messages with on_idle hook may work. Or better just restore the old behaivour and dispatch the messages after they are queued. Another way will need to what for the new post_inherent hook and service the queue there.

ggwpez commented 3 months ago

This is a known change - yes. The MQ pallet has to be careful with PoV weight: just executing all XCM at the beginning of a block can overload the PoV weight and produce overweight blocks. It therefore queues them.

You can call ServiceQueues::service_queues(weight_limit: Weight) -> Weight to nudge the pallet into processing messages whenever you want.

I guess it still makes sense to add a config flag to enable it to also process messages in the on_poll hook though, so it would try to process as early as possible. You are right that this should produce a smaller PoV for low amounts of messages.