Variables renaming on TaskManager

[vncoelho]

This helps for understanding #1148

@erikzhang, please consider this renaming. RestartTasks was a name hard to understand.
In fact, that class was just used at ConsensusService, which forces a restart of global and knowhashes

I believe that comprehension will improve with this.

[vncoelho]

@erikzhang, can you please check this again. I think that this is a good renaming. I closed the other PR because the recent PR #1183 will improve the use of Transaction Verify/Reverify.

[erikzhang]

I don't think this is a good renaming. Because RestartTasks is not related to consensus.

[vncoelho]

@erikzhang, I do not understand. Currently, it is just used by the consensus, in the specific case of asking for Speaker txs.
Is there any other use in mind? I think that the restart might slightly change for each case.
The current design looks quite suitable for the consensus request of txs and is only used there nowadays.

Another idea that just came, maybe the restart could just clear specific consensus hashes, if we move to a set for each type (nowadays we have a knownhashes globally).

[vncoelho]

@doubiliu and @Tommo-L, since you are already playing with this GetData from the Consensus Speaker, take a look here.

I think that this renaming makes the task more clear.
Perhaps, we could even have a specific message instead of GetData, such as GetSpeakerTxsData/GetCurrentBlockData.
In addition, we can even filter that! Imagine that the guy is requesting data from a tx that you already know that is within a block?
I believe that these extra command may give us more flexibility.

I do not see a problem on this because we can have WatchOnly nodes also requesting that.
In this sense, it is more a natural filter for avoiding spam from and mix these request with other normal "daily" requests.

[Tommo-L]

From the perspective of TaskManager, the RestartTasks is ok.
From the perspective of consensus use, the ConsensusTxsTask is ok.
Both for me are ok.

[Tommo-L]

If you want to rename the RestartTasks, plz also consider the TaskSession.AvailableTasks, which only used for inv-block task.

[vncoelho]

I agree, @Tommo-L, I prefer InvBlockPendingTasks than AvaiableTasks as well, which is also aligned with the PR of @yongjiema, #899.

[shargon]

Looks good to me

[vncoelho]

@erikzhang, can we merge? :) aehuaheauheauea

[erikzhang]

#1185 (comment)

My point of view has not changed.

[lock9]

New names are better, they are more explicative than the current ones.
I will approve this because, in my opinion, this is a good change.
I'm not going to merge it until Erik agrees.

[lock9]

@erikzhang we can rename to a different one once we starting using it in a different place. There is no reason to make it generic at this point.

[erikzhang]

RestartTasks is public. In fact it was used in neo-cli. I think it is not related to consensus. This kind of naming is very strange. Its role is to inform the TaskManager to restart some tasks.

[lock9]

It is because I checked the code and the only place where it is used is here: OnPrepareRequestReceived.
Maybe it doesn't have to be public.

[erikzhang]

If it is not public, there is no chance to send it from outside. (example: neo-cli, or plugins)

[vncoelho]

AHeuaheuaea, ok @erikzhang, let's close this.
In fact, it is very bad to have a consensus name on the TaskManager.

However, the availableInvBlocksTasks looks good renaming... ;D

[vncoelho]

@lock9, I think it is better to keep this in mind and let it for the future if that is the case.

My initial rename did not look so good to me after some time, however, the one suggested by @Tommo-L looked better. Thus, maybe it is better to do not rename anything right now.