Clarify Fetch candidate heads in reverse sorted order

[jamesray1]

This is for https://github.com/ethereum/sharding/blob/develop/docs/doc.md#fetch-candidate-heads-in-reverse-sorted-order.

This is just initializing the variables to the default values of their type in zero-byte representation.

unchecked_logs = []
current_checking_score = None

Maybe clarify in a comment before the function definition that unchecked_logs is added to by the CollationAdded log.

When reading this section my confusion was compounded when it said after the code:

To re-express in plain language, the idea is to scan backwards through CollationAdded logs (for the correct shard), and wait until you get to one where isNewHead = True. Return that log first, then return all more recent logs with a score equal to that log with isNewHead = False, in order of oldest to most recent. Then go to the previous log with isNewHead = True (this is guaranteed to have score 1 lower than the previous NewHead), then go to all more recent blocks after it with that score, and so forth.

Also the syntax for declaring CollationAdded could use this: https://viper.readthedocs.io/en/latest/structure-of-a-contract.html#events, whereas in the doc it has:

There is also one log type:

• CollationAdded(indexed uint256 shard, bytes collationHeader, bool isNewHead, uint256 score)

[jamesray1]

I worked it out the parts with strikethrough. Initially if the values are inititialized to zero, in the first cycle of the while loop it will get the next log. However, obviously the for loop would be skipped over if unchecked_logs is empty, but fetch_candidate_head() may be called when it isn't empty.

[jamesray1]

for i in range(len(unchecked_logs)-1, -1, -1):

What's the point of having the range stop at -1?

Say that len(unchecked_logs) = 5. This would result in using i as:

...unchecked_logs[4].score...
...unchecked_logs[3].score...
...unchecked_logs[2].score...
...unchecked_logs[1].score...
...unchecked_logs[0].score...
...unchecked_logs[-1].score...

unchecked_logs[-1].score would get the last item in the range, but that would have already been checked with len(unchecked_logs)-1.

[jamesray1]

Return that log first, then return all more recent logs with a score equal to that log with isNewHead = False, in order of oldest to most recent. Then go to the previous log with isNewHead = True (this is guaranteed to have a score that is 1 lower than the previous NewHead), then go to all more recent blocks after it with that score, and so forth.

I can't see how this is done in the preceding code, however I get the principle and the example.

Note that to return in the way listed as follows it seems that the code would need to be different.

If we number the collations A1..A5, B1..B5, C1..C5 and D1..D5, the precise returning order is:

D4 D3 D2 D1 D5 B2 C5 B1 C1 C4 A5 B5 C3 A3 B4 C2 A2 A4 B3 A1

At the moment it seems that the code would return the unchecked logs prior to calling the function, then return o, the collation in the most recent log with the current_checking_score and with isNewHead = True. To return the collations in the above way you would need to not return the unchecked_logs prior to calling the function, iterate the current_checking_score from the maximum score to the minimum score, and return the collations with the current_checking_score but with isNewHead = False.

Let me know if I am misunderstanding something.

[mhchia]

Say that len(unchecked_logs) = 5. This would result in using i as:

I did an experiment in both python2 and python3. It seems correct.

>>> for i in range(5 - 1, -1, -1):
...     print(i)
... 
4
3
2
1
0

[mhchia]

Also the syntax for declaring CollationAdded could use this: https://viper.readthedocs.io/en/latest/structure-of-a-contract.html#events, whereas in the doc it has:

Thank you for the suggestion:) We will implement it that way in vmc.

[hwwhww]

done