core/types: fix immutability guarantees in Block #27844
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change rearranges the accessor methods in block.go and fixes some minor issues with the copy-on-write logic of block data. As a refresher, the rules around block immutability are as follows: - We copy all data when the block is constructed. This makes the references held inside the block independent of whatever value was passed in. - We copy all header data on access. This is because any change to the header would mess up the cached hash and size values in the block. Calling code is expected to take advantage of this to avoid over-allocating! - When new body data is attached to the block, we create a shallow copy of the block. This ensures block modifications are race-free. - We do not copy body data on access because it does not affect the caches, and also because it would be too expensive. The change corrects these issues: - Block.WithWithdrawals did not create a shallow copy of the block. - Block.WithBody copied the header unnecessarily, and did not preserve withdrawals. However, the bugs did not affect any code in go-ethereum because blocks are *always* created using NewBlockWithHeader().WithBody().WithWithdrawals().
fjl
force-pushed
the
core-types-block-imm
branch
from
1e99de3
to
a34b90c
Compare
rjl493456442
approved these changes
Aug 4, 2023
devopsbo3
pushed a commit
to HorizenOfficial/go-ethereum
that referenced
this pull request
Nov 10, 2023
This change rearranges the accessor methods in block.go and fixes some minor issues with the copy-on-write logic of block data. Fixed issues: - Block.WithWithdrawals did not create a shallow copy of the block. - Block.WithBody copied the header unnecessarily, and did not preserve withdrawals. However, the bugs did not affect any code in go-ethereum because blocks are *always* created using NewBlockWithHeader().WithBody().WithWithdrawals()
devopsbo3
added a commit
to HorizenOfficial/go-ethereum
that referenced
this pull request
Nov 10, 2023
)" This reverts commit d8807c4.
devopsbo3
added a commit
to HorizenOfficial/go-ethereum
that referenced
this pull request
Nov 10, 2023
)" This reverts commit d8807c4.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change rearranges the accessor methods in block.go and fixes some minor issues with the copy-on-write logic of block data. As a refresher, the rules around block immutability are as follows:
We copy all data when the block is constructed. This makes the references held inside the block independent of whatever value was passed in.
We copy all header data on access. This is because any change to the header would mess up the cached hash and size values in the block. Calling code is expected to take advantage of this to avoid over-allocating!
When new body data is attached to the block, we create a shallow copy of the block. This ensures block modifications are race-free.
We do not copy body data on access because it does not affect the caches, and also because it would be too expensive.
The change corrects these issues:
Block.WithWithdrawals did not create a shallow copy of the block.
Block.WithBody copied the header unnecessarily, and did not preserve withdrawals.
However, the bugs did not affect any code in go-ethereum because blocks are always created using NewBlockWithHeader().WithBody().WithWithdrawals().