Add block setters for consensus type #11751
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 reverts commit 1ae5db7.
terencechain
force-pushed
the
add-block-getters
branch
2 times, most recently
from
a565392 to
1df6f2e
Compare
terencechain
force-pushed
the
add-block-getters
branch
from
9cddfd2 to
490d110
Compare
terencechain
force-pushed
the
add-block-getters
branch
from
490d110 to
f7142a6
Compare
nisdas
reviewed
Dec 12, 2022
consensus-types/blocks/getters.go
Outdated
| @@ -751,6 +786,11 @@ func (b *BeaconBlockBody) Graffiti() [field_params.RootLength]byte { | |||
| return b.graffiti | |||
| } | |||
|
|
|||
| // SetGraffiti sets the graffiti in the block. | |||
| func (b *BeaconBlockBody) SetGraffiti(g []byte) { | |||
Big typo for the title. It's meant to write block setters lol |
potuz
reviewed
Dec 12, 2022
| @@ -39,7 +39,9 @@ func TestExecuteStateTransitionNoVerify_FullProcess(t *testing.T) { | |||
| require.NoError(t, err) | |||
| require.NoError(t, beaconState.SetSlot(beaconState.Slot()-1)) | |||
|
|
|||
| nextSlotState, err := transition.ProcessSlots(context.Background(), beaconState.Copy(), beaconState.Slot()+1) | |||
| @@ -95,7 +97,9 @@ func TestExecuteStateTransitionNoVerifySignature_CouldNotVerifyStateRoot(t *test | |||
| require.NoError(t, err) | |||
| require.NoError(t, beaconState.SetSlot(beaconState.Slot()-1)) | |||
|
|
|||
| nextSlotState, err := transition.ProcessSlots(context.Background(), beaconState.Copy(), beaconState.Slot()+1) | |||
There was a problem hiding this comment.
Sorry. It's meant for another PR
consensus-types/blocks/execution.go
Outdated
| @@ -155,6 +155,16 @@ func (e executionPayload) WithdrawalsRoot() ([]byte, error) { | |||
| return nil, ErrUnsupportedGetter | |||
| } | |||
|
|
|||
| // PbV1 -- | |||
| func (e executionPayload) PbV1() (*enginev1.ExecutionPayload, error) { | |||
There was a problem hiding this comment.
All these PbVx from the different targets are not tested.
| @@ -731,6 +731,41 @@ func (b *BeaconBlock) AsSignRequestObject() (validatorpb.SignRequestObject, erro | |||
| } | |||
| } | |||
|
|
|||
| func (b *BeaconBlock) Copy() (interfaces.BeaconBlock, error) { | |||
rkapka
reviewed
Dec 12, 2022
consensus-types/blocks/execution.go
Outdated
| @@ -155,6 +155,16 @@ func (e executionPayload) WithdrawalsRoot() ([]byte, error) { | |||
| return nil, ErrUnsupportedGetter | |||
| } | |||
|
|
|||
| // PbV1 -- | |||
| func (e executionPayload) PbV1() (*enginev1.ExecutionPayload, error) { | |||
There was a problem hiding this comment.
Do we really want to name things PbV1/PbV2 instead of Pb/PbCapella or PbBellatrix/PbCapella? It might be misleading because all protobufs are under the enginev1 package. I understand this convention comes from the builder API?
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
…to add-block-getters
rkapka
approved these changes
Dec 12, 2022
roberto-bayardo
pushed a commit
to roberto-bayardo/prysm
that referenced
this pull request
Dec 17, 2022
* Add block getters * Rm changes * Rm changes * Revert "Rm changes" This reverts commit 1ae5db7. * Fix tests * Set graffiti right place * Potuz feedback * Update consensus-types/blocks/setters.go Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Radek feedback * Fix comments Co-authored-by: Radosław Kapka <rkapka@wp.pl>
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.
Pat of #11629
This PR adds block setters which is very useful for block construction for validators. We no longer need to duplicate every protobuf-specific functions for every hard fork.
Note: Setting a field in a block as currently designed is not thread-safe, the only caller is the proposer at the block construction stage. This is similar to what we have today with setting state root. I'll be happy to change this to a thread-safe model with lock if there's feedback