TOP pool synchronization

[kziemianek]

Context

resolves: #1935
resolves: P-100

This PR attempts to add TOP pool synchronization through direct trusted rpc server. Trusted calls should be eventually broadcasted to other registered workers and optimistically picked and executed by next block producer.

Changes:

• author_submitAndWatchExtrinsic JSON-RPC method broadcasts trusted call to other peers
• adds author_submitAndWatchBroadcastedExtrinsic JSON-RPC method which acts as author_submitAndWatchExtrinsic but doesn't broadcast trusted calls
• adds TrustedOperationStatus::TopExecuted which is fired upon TOP execution (breaking change)
• adds ocall get_trusted_peers_urls in order pull trusted peers urls from worker service to enclave runtime

[linear]

P-100 Linear processing of DI requests

Context

Following up on the discussion in #1920 (comment)

Imagine there are 100 workers and we send a request to worker0, worker0 happens to just finish the its block production so the newly submitted request won't be included in the block for now. Since we have an aura authorship mechanism, the request can be processed after 100 blocks at the earliest, this sounds terrifying.

The root cause is the tx pool of sidechain is not synchronised among nodes, so a DI request can only be stored and processed by the node, to which the request was submitted.

It's a critical issue for multi-worker setup, but I don't think we have time to solve it soon, hence the medium prior.

---

✔️ Please set appropriate labels and assignees if applicable.

[Kailai-Wang]

I haven't completed the review yet - only took a rough look. Will go into more details later

The animation is well made btw!

[Kailai-Wang]

Overall looks good to me, nicely done!

[clangenb]

Hey, we definitely want to gossip the top pool. I had a superficial look and this PR looks very nice and clean!

I noticed that you gossip the executed TOP to peers, but this is probably not the mechanism to remove it from the synchronized pool? It is done via the sidechain block import, isn't it?

Maybe @brenzi also wants to chime in here.

[kziemianek]

Hey, we definitely want to gossip the top pool. I had a superficial look and this PR looks very nice and clean!

I noticed that you gossip the executed TOP to peers, but this is probably not the mechanism to remove it from the synchronized pool? It is done via the sidechain block import, isn't it?

Maybe @brenzi also wants to chime in here.

Thanks @clangenb for your time and taking a look at this 👍🏼 .

Yes, this change doesn't affect how TOP pool is managed (when TOPs are added or removed). It basically only gossips trusted calls to other known peers and routes back selected responses to the caller. All the rest remains the same.

RPC client could submit trusted call to all peers in the network separately and the efect would be pretty the same.

[brenzi]

I think you should take care not to gossip getters or inherents.

Top pool holds tops (TrustedOperation) which can be getters, indirect or direct TrustedCalls

Also, a trusted call can be an inherent/intrinsic(signed by enclave itself) i.e. shield_funds(). These could be gossipped in principle, but we need to think about edge cases

[kziemianek]

I think you should take care not to gossip getters or inherents.

Top pool holds tops (TrustedOperation) which can be getters, indirect or direct TrustedCalls

Also, a trusted call can be an inherent/intrinsic(signed by enclave itself) i.e. shield_funds(). These could be gossipped in principle, but we need to think about edge cases

Hi @brenzi, thanks for your advices, here are my thoughts:

• Only trusted operations submitted through json-rpc method author_submitAndWatch* are gossiped - this means that indirect calls submitted by indirect executor directly to AuthorApi wouldn't be gossiped
• Only succesfully submitted trusted operations are gossiped. This means that as long as we have correct Author's filter (CallsOnlyFilter) applied getters wouldn't be gossiped.

We can add filtering logic to DirectRpcBroadcaster similar to the one in Author but that would require going through the whole ceremony of decrypting/decoding or move gossiping to Author - might be worth to make sure only direct calls are broadcasted. What do you think @Kailai-Wang ?

[kziemianek]

I've moved broadcasting triggering point from json-rpc method to Author, it gives following benefits:

• broadcasting is encapsulated in Author so other components don't need to be aware about how broadcasting should be performed
• possibility to filter TrustedCalls so only direct calls are broadcasted

[Kailai-Wang]

I've moved broadcasting triggering point from json-rpc method to Author, it gives following benefits:

• broadcasting is encapsulated in Author so other components don't need to be aware about how broadcasting should be performed
• possibility to filter TrustedCalls so only direct calls are broadcasted

Thank you - I like the incorporation of BroadcastedRequest

And yes I don't think we need to broadcast all kinds of trusted operations:

• direct trusted call => sure
• trusted getters => theoretically we should be able to submit getters via author_submitAndWatch* but practically we don't do that. Maybe we should simply disallow submitting getters from this rpc method - or apply filtering. Anyways it makes little sense to gossip it
• indirect calls => we don't need to gossip it, all workers will get them by syncing the parachain

So I'd say for now we only broadcast direct trusted calls.

[Kailai-Wang]

Looks good, let's merge it.

BroadcastedTopFilter is passed in as DirectCallsOnlyFilter to initialisation AFAIU, isn't it

[kziemianek]

Looks good, let's merge it.

BroadcastedTopFilter is passed in as DirectCallsOnlyFilter to initialisation AFAIU, isn't it

Yes, this will ensure only direct calls are broadcasted to peers.

[kziemianek]

I've moved broadcasting triggering point from json-rpc method to Author, it gives following benefits:

• broadcasting is encapsulated in Author so other components don't need to be aware about how broadcasting should be performed
• possibility to filter TrustedCalls so only direct calls are broadcasted

Thank you - I like the incorporation of BroadcastedRequest

And yes I don't think we need to broadcast all kinds of trusted operations:

• direct trusted call => sure
• trusted getters => theoretically we should be able to submit getters via author_submitAndWatch* but practically we don't do that. Maybe we should simply disallow submitting getters from this rpc method - or apply filtering. Anyways it makes little sense to gossip it
• indirect calls => we don't need to gossip it, all workers will get them by syncing the parachain

So I'd say for now we only broadcast direct trusted calls.

Trusted getters are not added to TOP pool due to filters applied :

#[cfg(feature = "sidechain")]
pub type AuthorTopFilter = crate::top_filter::CallsOnlyFilter;