HAL-01 | panics in beginblock and endblock

[juniuszhou]

Description

BeginBlocker and EndBlocker are optional methods module developers can implement in their module. They will be triggered at the beginning and at the end of each block, respectively, when the BeginBlock and EndBlock ABCI messages are received from the underlying consensus engine. Making use of panics for error handling in the BeginBlock and EndBlock methods may cause the chain to halt if an error does occur.

Code Location:
x/clp/abci.go, Line 130
 130 panic ( err )

x/dispensation/abci.go, Lines 54-60
54 if err != nil {
55 panic ( fmt . Sprintf (" Unable to send %s coins to address %s", mintCoins . String () , ecoPoolAddress . String () ) )
56 }
57 err = k. AddMintAmount (ctx , mintCoins [0])
58 if err != nil {
59 panic ( err )
60 }

x/dispensation/abci.go, Lines 54-60
if err != nil {
55 panic ( fmt . Sprintf (" Unable to send %s coins to address %s", mintCoins . String () , ecoPoolAddress . String () ) )
56 }
57 err = k. AddMintAmount (ctx , mintCoins [0])
58 if err != nil {
59 panic ( err )
60 }

Recommendation

According to suggestion in the report, it is better to use error instead of panic. I don't agree on it. If the Sifchain runs into the situation that we shouldn't continue, panic and exit could be the right behavior. Otherwise the chain will be running at the wrong context. Stop it and fix the issue could be the right thing to do.

[juniuszhou]

The same with #3263, I think it make sense to panic and stop the node or chain if serious error.