-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Prysm rpc: Get local header #14378
Prysm rpc: Get local header #14378
Conversation
1da49d3
to
2df5f30
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Happy path" unit test fails for me. Not sure if it's just me but I attached some solutions.
}) | ||
t.Run("Happy case", func(t *testing.T) { | ||
params.SetupTestConfigCleanup(t) | ||
cfg := params.BeaconConfig().Copy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses minimal config, producing the following error: Unexpected error: rpc error: code = Internal desc = Could not process slots up to 8: could not process slots: could not process epbs epoch: state randao length 65536 different than EpochsPerHistoricalVector 64
.
I came up with two solutions:
- Replace L48 with
cfg := params.MainnetConfig().Copy()
and addOptimisticModeFetcher: &mock.ChainService{Optimistic: false},
tovs := &Server{...}
(L70 or so). - Add
cfg.EpochsPerHistoricalVector = 65536
between L48 and L49.
Applying either of them fixes it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI/CD passes, that's all it matters here. To run beacon-chain/rpc/prysm/v1alpha1/validator
, you should use bazel instead of native golang. bazel test //beacon-chain/rpc/prysm/v1alpha1/validator:go_default_test --test_filter=TestServer_GetLocalHeader
ExecutionEngineCaller: &powtesting.EngineClient{ | ||
GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: executionData, BlobsBundle: &v1.BlobsBundle{ | ||
KzgCommitments: kzgs, | ||
}}, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big of a deal but I think this is easier to read:
ExecutionEngineCaller: &powtesting.EngineClient{ | |
GetPayloadResponse: &blocks.GetPayloadResponse{ExecutionData: executionData, BlobsBundle: &v1.BlobsBundle{ | |
KzgCommitments: kzgs, | |
}}, | |
}, | |
} | |
ExecutionEngineCaller: &powtesting.EngineClient{ | |
GetPayloadResponse: &blocks.GetPayloadResponse{ | |
ExecutionData: executionData, | |
BlobsBundle: &v1.BlobsBundle{ | |
KzgCommitments: kzgs, | |
}}, | |
}, |
2df5f30
to
5d7e86c
Compare
|
||
st, parentRoot, err := vs.getParentState(ctx, slot) | ||
if err != nil { | ||
return nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you return a status here too?
return nil, err | |
return nil, status.Errorf(codes.Internal, "Could not get parent state: %v", err) |
GasLimit: localPayload.ExecutionData.GasLimit(), | ||
BuilderIndex: proposerIndex, | ||
Slot: slot, | ||
Value: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is value meant to be 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is called when a proposer is a builder. Value here means how much do I pay for myself, so making it non-zero is effectively same with zero value.
5d7e86c
to
a21d4c2
Compare
Moved to #14409 |
This PR adds a Prysm RPC endpoint to retrieve the local header for the proposer to sign. The
getLocalHeader
call uses the existinggetLocalPayloadFromEngine
method and the input slot and proposer index to obtain the payload from the execution client.This PR does not include caching the payload, we should implement caching once the end-to-end process is more defined, so we have a clearer understanding of what the cache's API will look like.