CommitBlock - same block height collisions between BlockSync and Consensus should not cause error

[ronnno]

When blocks are committed during block sync they might be rejected if already in block storage.

This usually does not happen but for instance if a block is accepted by the network and something causes delay in the last part where ConsensusAlgo commits the bloc to BlockStorage. A Block sync might be triggered during the delay causing the block arrive form external source before ConsensusAlgo commits it. Later ConsensusAlgo will continue with committing the same block but it will be rejected by BlockStorage.

In this case, we should not have to log an error. because there is no error here. But, currently the API does not allow communicating back to Consensus Algo that the block was not committed although there was no error.

Suggest: adding an indication inside CommitBlockOutput such as a success boolean or current top height.

[OdedWx]

Correct, in a normal operation there can be a race between node sync and consensus commit.
When block storage receives a block as part of the sync it requests the consensus to validate it and update its current height.
This means that if the block storage rejected a committed block since it was already committed as part of the sync it implies that the consensus algo is already updated.

[ronnno]

The problem is that whoever is calling CommitBlock does not know if an error occurred or if the error indicates a correct state where the block was previously committed.

we can:

• return a success status in CommitBlockOutput
• return a nil error in this case - implicating that the commit succeeded without indicating to the caller that the block had been previously committed.

Which is it?

[OdedWx]

The consensus algo cant react to an error in the commit, so I'm not sure that the data helps it.