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

Implement relay with builder REST API spec #26

Merged
merged 49 commits into from
May 11, 2022
Merged

Implement relay with builder REST API spec #26

merged 49 commits into from
May 11, 2022

Conversation

metachris
Copy link
Contributor

Copy link
Collaborator

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Left a few comments, but mostly on api/builder.go side of things. I think this is in a good enough place to merge and we sort that stuff out in a subsequent PR. Nice work!

BlockHash common.Hash `json:"blockHash" gencodec:"required"`
TransactionsRoot common.Hash `json:"transactionsRoot" gencodec:"required"`
}
func BuilderGetHeader(ctx context.Context, log logrus.Ext1FieldLogger, sk bls.SecretKey, builderAddr string, blockHash common.Hash) (*types.ExecutionPayloadHeader, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func BuilderGetHeader(ctx context.Context, log logrus.Ext1FieldLogger, sk bls.SecretKey, builderAddr string, blockHash common.Hash) (*types.ExecutionPayloadHeader, error) {
func BuilderGetHeader(ctx context.Context, log logrus.Ext1FieldLogger, builderAddr string, slot uint64, blockHash common.Hash, sk bls.SecretKey) (*types.ExecutionPayloadHeader, error) {

Needs slot parameter. Also makes sense to order in the same order they are in the path.

ExtraData hexutil.Bytes
LogsBloom hexutil.Bytes
}
path := fmt.Sprintf("/eth/v1/builder/header/%d/%s/0x%x", 1, blockHash.Hex(), sk.PublicKey().Marshal())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
path := fmt.Sprintf("/eth/v1/builder/header/%d/%s/0x%x", 1, blockHash.Hex(), sk.PublicKey().Marshal())
path := fmt.Sprintf("/eth/v1/builder/header/%d/%s/0x%x", slot, blockHash.Hex(), sk.PublicKey().Marshal())

type BlindedBeaconBlockBody struct {
ExecutionPayload ExecutionPayloadHeaderV1 `json:"execution_payload_header"`
e.Debug("Received payload")
return bid.Data.Message.Header, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably verify builder signature before returning. Maybe also add a TODO that we should eventually add a list of "trusted" builders to cross-reference the builder pubkey against.

Message *BuilderReceipt `json:"message"`
Signature string `json:"signature"`
}
func BuilderGetPayload(ctx context.Context, log logrus.Ext1FieldLogger, sk bls.SecretKey, builderAddr string, header *types.ExecutionPayloadHeader) (*types.ExecutionPayloadV1, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The api interface probably shouldn't take bls.SecretKey, rather SignedBlindedBeaconBlock instead.

}

// func (r *RelayBackend) GetHeaderV1(ctx context.Context, slot hexutil.Uint64, pubkey hexutil.Bytes, parentHash common.Hash) (*types.SignedBuilderBidV1, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can probably just remove this stuff now?

@lightclient lightclient merged commit 5647c41 into protolambda:master May 11, 2022
@metachris metachris mentioned this pull request May 12, 2022
@metachris metachris deleted the rest branch May 13, 2022 13:03
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.

3 participants