staking miner: Check the queue one last time before submission #4819
Conversation
github-actions
bot
added
the
A3-in_progress
niklasad1
added
A0-please_review
niklasad1
changed the title
|
@kianenigma can you take another look at this? I tried to address your grumbles and I hope I got it right this time |
| .map_err::<Error<T>, _>(Into::into)? | ||
| .unwrap_or_default(); | ||
|
|
||
| for (_score, idx) in indices { |
There was a problem hiding this comment.
I kinda wanted to rewrite this with .any() but I realized async code in closures it still a PITA right?
There was a problem hiding this comment.
it's possible but then we need convert the futures to a stream or batch them together.
Something like FuturesOrdered or JSON-RPC batch request might make this is little bit nicer to read.
Running it right now, I'll keep you posted. EDIT: works just won the election on westend |
|
@emostov can you take another look at this as well? |
| let rpc2 = rpc.clone(); | ||
| let account = signer.account.clone(); | ||
|
|
||
| let signed_phase_fut = tokio::spawn(async move { |
There was a problem hiding this comment.
I assume we do this here to prevent heavy network usage from create_election_ext and then again right before submitting to make sure its not pointless to submit?
I think its fine for now, but in the future I think we should look into only doing it right before submitting. Even if it ends up with a lot of wasted network usage it should be fine as long as we are not executing this function righter before the signed phase, since that is when we want to be really fast to get the solution in. But if there fear is that this will be executing at the end of the sign phase I think its fine because we will have awhile until the next signed phase, so the extra network usage won't slow itself down. (Assuming its just traffic to a node on the local network)
There was a problem hiding this comment.
I assume we do this here to prevent heavy network usage from create_election_ext and then again right before submitting to make sure its not pointless to submit?
Yeah
Even if it ends up with a lot of wasted network usage it should be fine as long as we are not executing this function righter before the signed phase, since that is when we want to be really fast to get the solution in.
I'm just afraid that it will flood the JSON-RPC client and it can't keep up with all pending calls because fetching the entire pallet data is quite large (even if the future is dropped the client has to process to responses to check whether it's a valid call). This will occur on every block if we try to do everything in parallel.
Thus, I think we could try to make a subscription to the Phase and only spawn these tasks if it's Phase::Signed then
I'd be okay in spawning the huge calls in parallel but we could try to benchmark this just to be on the safe side. I could be wrong :)
|
bot merge |
Builds on #4716
Closing #3740