Balance change after new block is delayed by several seconds - Closes #2403

[lsilvs]

What was the problem?

New blocks were been broadcasted before cache for blocks and transactions been cleared.

How did I fix it?

Changed the broadcastBlock operation to be executed after applyBlock

How to test it?

Run the tests.
Follow the steps described in the issue #2403

Review checklist

• The PR resolves Balance change after new block is delayed by several seconds #2403
• All new code is covered with unit tests
• All new code was formatted with Prettier
• Linting passes
• Tests pass
• Commit messages follow the commit guidelines
• Documentation has been added/updated

[SargeKhan]

This PR is changing the behavior of broadcasting of blocks. By which I mean that earlier the node was broadcasting blocks even they were invalid whereas after this change node will only broadcast blocks which it considers to be valid. Therefore, this change will affect how the network behaves during fork recovery.

[4miners]

I agree with @SargeKhan, we should not change the behavior when we don't need to.

[lsilvs]

Good point @SargeKhan
I'll come up with something else then.

[4miners]

@lsilvs So what is currently wrong with the logic here is that blocks/change event is emitted from the wrong place. It should be emited after validation/verification, when block is actually accepted by the node. The simplest valid solution would be to add a new function, like:

Blocks.prototype.onNewBlock = block => {
    // Emit notification
    library.network.io.sockets.emit('blocks/change', block);
};

And remove emitting this event from Transport->onBroadcastBlock.

[lsilvs]

As discussed with @4miners and @nazarhussain , just adding onNewBlock inside Block module is not enough as we can't guarantee that the cache will be cleared before emitting blocks/change message.

The solution was to refactor onNewBlock inside Cache module to a regular method and call it from onNewBlock inside Block module making sure we emit blocks/change message after the cache function is finished.

[nazarhussain]

Please change this log message. As its not Cache - onNewBlock now.

[nazarhussain]

There is already two methods removeByPattern and deleteJsonForKey in general. I would rather suggest to develop a generic method clearCacheFor(scope), scope can be blocks, transactions or in future any thing. This way we can make that single method useable on different cases.

[4miners]

Where this is used?

[lsilvs]

You mean the delegatesApi?
If so, I meant to use it in onFinishRound and onTransactionsSaved to keep consistency but it seems I've forgotten.
Good catch! I will added it.

[4miners]

Yep, thanks.

[MaciejBaj]

It might solve the #955 as well.