Total Difficulty in IBFT

[dbrajovic]

Description

Defining total difficulty in our IBFT implementation.
Addresses #179

Update:

Hardcoding GenesisDifficulty to 1 breaks backwards compatibility for existing chains due to new nodes having to generate the genesis block with difficulty 1.

Changes include

• Bugfix (non-breaking change that solves an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

• I have assigned this PR to myself
• I have added at least 1 reviewer
• I have tested this code
• I have updated the README and other relevant documents (guides...)
• I have added sufficient documentation both in code, as well as in the READMEs

Additional comments

This PR is about coming to terms on what Total Difficulty means in IBFT.

In other consensuses, TD is what helps identify the correct chain (fork) based on the sum of all blocks' difficulty field (which varies from block to block).
In IBFT, TD has nothing to do with the difficulty field, but serves the same purpose, in a sense that it decides what is the latest block (number) - hence this value should represent the block's number field.

In short (our IBFT impl):

Block's difficulty -> Block's number
Block's TD -> Sum of all previous block numbers (up to current)
Chain TD -> TD of the latest block in the chain

Related issues:
https://github.com/0xPolygon/polygon-sdk/issues/179

This fix should provide the Syncer with the correct value (fetched with CurrentTD()) in order to resolve its best peer.

[dbrajovic]

After tuning the unit tests, I discovered that TD is calculated in other places as well (advanceHead()). At all points, there is a fetching of the parent difficulty used only to calculate new TD (which is redundant in this new approach).

I've left _ = variableName to suppress compiler warnings - I intend to remove these once proven that they were not meant for anything else.

Regarding the TD field in the units test (blockchain_test.go), this is the value we would like to see hardcoded to 1, in which case, all that is needed is to hardcode GetChainTD() to always return 1.

[dbrajovic]

After revising the aforementioned calculation of TD, I decided to go with the less invasive approach of hardcoding each block's difficulty to 1, keeping the existing logic (summing of difficulties) intact, such that the following are true:

Block difficulty = 1
Block TD = Block number (1 + 1 + ... + 1)
ChainTD = TD of the latest block (its number)

[zivkovicmilos]

Looks great to me 💯

Left a minor question that needs clarification

[zivkovicmilos]

Thank you for making the requested changes 🙏

Looks great now 💯

[Kourin1996]

Thank you for the PR. I like renaming variable to proper one. I've left small comments. I'd appreciated if you could check and fix if need.

[Kourin1996]

Thank you for fixing. LGTM👍

[zivkovicmilos]

Approving for a second time because Github reset my approval for some reason 🙂

[lazartravica]

@dbrajovic feel free to close this PR and delete the branch if you plan on opening a new one 👍

[lazartravica]

Closing this PR in favour of #208

cc @dbrajovic