fix: put CommittedTransaction::error in block hash

[Erigara]

Context

Error in CommittedTransaction was not reflected in the block hash which allowed for this value to be tempered result in block false rejection.

Closes #5001.

Solution

Include error in block hash calculation.

[mversic]

we specifically don't want to do this. This stands in the way of #4967 as noted here and there is no reason to do this because it has no effect on blockchain integrity

[mversic]

in other words, it's important that we don't include CommittedTransaction::error in block hash or signature so that leader can send block before validation and update that value afterwards

[Erigara]

it has no effect on blockchain integrity

i think by checking this error we can assure that peers are on the same page at least to some extent (they all agree on outcome of each transaction in the block)

[mversic]

it has no effect on blockchain integrity

i think by checking this error we can assure that peers are on the same page at least to some extent (they all agree on outcome of each transaction in the block)

we can do this without making this value a part of the block hash. The same way we circulate some other messages between peers

however, if leader will not categorize instructions (for performance reasons), what good is this to us since there is no value to be agreed upon? In the case of block sync, yes we can reject the block if the value is incorrect. But we still don't have to protect the value with a hash

[Erigara]

what good is this to us since there is no value to be agreed upon?

not sure i got your point but, if we don't somehow check result of block execution peers have a risk of silently diverge

[mversic]

what good is this to us since there is no value to be agreed upon?

not sure i got your point but, if we don't somehow check result of block execution peers have a risk of silently diverge

I'm talking in the context of #4967 where we don't categorize transactions ahead of time. I don't see how we can have both.

[Erigara]

Will update after merge of #4967