search

hyperledger-iroha / iroha

Iroha - A simple, enterprise-grade decentralized ledger
https://wiki.hyperledger.org/display/iroha
Apache License 2.0
438 stars 280 forks source link

fix: put `CommittedTransaction::error` in block hash #5095

Closed Erigara closed 4 weeks ago

Erigara commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 4 weeks ago

Will update after merge of #4967