Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Removes fork-specific concrete type getters #13941

Merged
merged 23 commits into from
May 2, 2024
Merged

Removes fork-specific concrete type getters #13941

merged 23 commits into from
May 2, 2024

Conversation

kasey
Copy link
Contributor

@kasey kasey commented Apr 30, 2024

What type of PR is this?

Other

What does this PR do? Why is it needed?

Maintaining backwards compatible interfaces across forks is a huge pain and causing geometric growth in the amount of bolilerplate spaghetti to add at each hard fork. This PR is part of a process to simplify the consensus type getters so that we can migrate to a more maintainable type scheme.

@kasey kasey force-pushed the rm-pb-getters branch from d30336c to 313085c Compare May 1, 2024 01:10
@kasey kasey marked this pull request as ready for review May 1, 2024 05:13
@kasey kasey requested a review from a team as a code owner May 1, 2024 05:13
@kasey kasey force-pushed the rm-pb-getters branch from bc017af to fc87233 Compare May 1, 2024 05:19
// massage the proto struct type data into the api response type.
mj, err := structs.SignedBeaconBlockMessageJsoner(sb)
if err != nil {
return nil, nil, errors.Wrap(err, "error generating blinded beacon block post request")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this message looks incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is correct? This method is posting a blinded beacon block to the builder in order to reveal the execution payload.

api/client/builder/client.go Outdated Show resolved Hide resolved
api/client/builder/client.go Outdated Show resolved Hide resolved
api/client/builder/client.go Outdated Show resolved Hide resolved
api/client/builder/client.go Outdated Show resolved Hide resolved
api/client/builder/client.go Outdated Show resolved Hide resolved
api/client/builder/client_test.go Outdated Show resolved Hide resolved
beacon-chain/rpc/eth/beacon/handlers.go Outdated Show resolved Hide resolved
beacon-chain/rpc/eth/beacon/handlers.go Outdated Show resolved Hide resolved
api/client/builder/client.go Show resolved Hide resolved
kasey and others added 13 commits May 1, 2024 08:31
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@kasey kasey added this pull request to the merge queue May 2, 2024
Merged via the queue into develop with commit c312a88 May 2, 2024
17 checks passed
@kasey kasey deleted the rm-pb-getters branch May 2, 2024 21:54
nisdas pushed a commit that referenced this pull request Jul 4, 2024
* removing typed pb accessors

* refactor ssz api resp handlers to avoid typed pbs

* json get block handler refactor

* SubmitBlindedBlock to use generic json handling

* update SubmitBlindedBlock

* clear out more usages of PbForkname methods

* remove fork-specific getters from block interface

* remove usages of payload pb methods

* remove pb helpers from execution payload interface

* Update beacon-chain/rpc/eth/beacon/handlers.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update beacon-chain/rpc/eth/beacon/handlers.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/builder/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/builder/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/builder/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/builder/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/builder/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Radek review

* fix error message

* deal with wonky builder responses

* ✂️

* gaz

* lint

* tweaks for deep source

---------

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants