Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor!: move transaction error out of block payload #5118

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Oct 3, 2024

closes #5001

  • move transaction error out of block payload

Context

since transaction error is not part of the block hash or protected by the block signature it's better to move it outside of block payload to avoid confusion. The main reason is that we don't want a change in transaction error that happens after block creation to affect block header

I didn't go that far in this PR, but I think it might be ok to keep this information in the state instead of inside a block. This could be explored in future work

Future work

even though error will no longer be a part of the block header it can still be protected with block signatures. We will just do this explicitly. This is to be handled in #5144

@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Oct 3, 2024
@mversic mversic force-pushed the move_tx_error branch 4 times, most recently from 9c9186e to c3fcacc Compare October 8, 2024 05:13
@mversic mversic assigned dima74 and unassigned nxsaken Oct 8, 2024
@mversic mversic force-pushed the move_tx_error branch 5 times, most recently from a95d78d to 0a7a0c4 Compare October 8, 2024 15:59
@s8sato s8sato self-assigned this Oct 8, 2024
@mversic mversic force-pushed the move_tx_error branch 3 times, most recently from 0b03730 to b93c621 Compare October 9, 2024 08:13
@mversic mversic marked this pull request as draft October 9, 2024 16:04
@mversic mversic force-pushed the move_tx_error branch 2 times, most recently from 84685b4 to e768e22 Compare October 14, 2024 12:29
dima74
dima74 previously approved these changes Oct 17, 2024
@mversic mversic enabled auto-merge (squash) October 18, 2024 04:27
@mversic mversic force-pushed the move_tx_error branch 3 times, most recently from 88f108d to de63dec Compare October 21, 2024 08:38
@mversic mversic requested a review from dima74 October 21, 2024 08:41
s8sato
s8sato previously approved these changes Oct 23, 2024
dima74
dima74 previously approved these changes Oct 23, 2024
@mversic mversic dismissed stale reviews from dima74 and s8sato via 2e248a1 October 24, 2024 07:12
@mversic mversic requested review from dima74 and s8sato October 24, 2024 07:13
s8sato
s8sato previously approved these changes Oct 28, 2024
dima74
dima74 previously approved these changes Oct 28, 2024
@mversic mversic disabled auto-merge October 28, 2024 08:41
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic dismissed stale reviews from dima74 and s8sato via e01b0cb October 28, 2024 15:00
@mversic mversic requested review from dima74 and s8sato October 28, 2024 15:16
@mversic mversic enabled auto-merge (squash) October 28, 2024 15:17
@mversic mversic merged commit b01fce0 into hyperledger-iroha:main Oct 28, 2024
16 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api-changes Changes in the API for client libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CommittedTransaction::error is not part of the block hash
5 participants