Prevent crashing the application when a block is received and node is syncing or forging - Closes #3896

[2snEM6]

What was the problem?

When receiving a block from the network in receiveBlockFromNetwork function, the application was crashing with a fatal error if the node was syncing or forging at that moment (aka blocks.js#_isActive was set to true)

How did I fix it?

By adding back forger.#forge call to the Sequence. Somehow it was removed from it in 2.2
Added a unit test case to cover this particular scenario.

How to test it?

Run these tests npm run mocha:unit framework/test/mocha/unit/modules/chain/chain.js

Review checklist

• The PR resolves Fatal error Block process cannot be executed in parallel #3896
• 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

[shuse2]

https://github.com/LiskHQ/lisk-sdk/blob/release/2.0.0/framework/src/modules/chain/submodules/blocks/process.js#L560-L565

I think we shouldn't change the receiveBlockFromNetwork in block, but it should be rejected in transport layer as it was before

[pablitovicente]

https://github.com/LiskHQ/lisk-sdk/blob/release/2.0.0/framework/src/modules/chain/submodules/blocks/process.js#L560-L565

I think we shouldn't change the receiveBlockFromNetwork in block, but it should be rejected in transport layer as it was before

I like this idea if the changes are not massive - due to the possible accessibility issues of the active variable - it would be nice for this to happen in network. Maybe through ApplicationState? But I think that if the changes for this are too big it could be ok if we create an issue to solve it in a minor version.

[2snEM6]

/2.0.0/framework/src/modules/chain/submodules/blocks/process.js@release#L560-L565

I think we shouldn't change the receiveBlockFromNetwork in block, but it should be rejected in transport layer as it was before

@shuse2 you mean transport.js? That file points to process.js

[2snEM6]

Also @shuse2 @pablitovicente shouldn't we add the task to the Sequence queue no matter what, and then the task itself could check if active = true. This way we don't miss received blocks. Doing it the way you propose I think could change the behavior.

[shuse2]

If the current status is syncing it shouldn't add to the sequence. If we do, the process queue will be something like

height: 10 (sync)
height: 10 (sync)
height: 200000 (received)
height: 10 (sync)
height: 10 (sync)
height: 10 (sync)

but this received is unnecessary, and only useful in case where it's pushed exactly when the sync is finishing