-
Notifications
You must be signed in to change notification settings - Fork 303
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
7390 consensus block value #7574
7390 consensus block value #7574
Conversation
139e44c
to
43828c8
Compare
data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java
Dismissed
Show dismissed
Hide dismissed
...main/java/tech/pegasys/teku/infrastructure/json/types/DeserializableOneOfTypeDefinition.java
Dismissed
Show dismissed
Hide dismissed
fbf04d2
to
357d56d
Compare
357d56d
to
a39b192
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.
You need to set consensus value somewhere in similar way you did it for local payload value, extend this method definition with consensusBlockValueResult https://github.com/mehdi-aouadi/teku/blob/fa84b508f6a978201d33e0628da3bc59dd0e8651/ethereum/spec/src/main/java/tech/pegasys/teku/spec/executionlayer/ExecutionLayerChannel.java#L134-L140 and set it in similar way you did local value above this line https://github.com/mehdi-aouadi/teku/blob/0b65bfac2ba63a4091e583889a90701607e926e3/ethereum/executionlayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/ExecutionBuilderModule.java#L209 with builderBidValue and filling it empty in all other cases (assuming it == localPayloadValue in all other cases). And extend ExecutionPayloadResult
as well. RewardCalculator
is designed to calculate CL side rewards from inclusion etc, it didn't count EL side rewards and vice versa.
I'm stupid, I didn't read the spec carefully |
data/provider/src/main/java/tech/pegasys/teku/api/DataProvider.java
Outdated
Show resolved
Hide resolved
data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java
Outdated
Show resolved
Hide resolved
data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java
Outdated
Show resolved
Hide resolved
data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java
Show resolved
Hide resolved
data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java
Outdated
Show resolved
Hide resolved
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.
LGTM, just I think we need to add a minor fix to handle exception in RewardCalculator
data/provider/src/main/java/tech/pegasys/teku/api/ValidatorDataProvider.java
Outdated
Show resolved
Hide resolved
99d72a0
to
570efea
Compare
3aaf7f1
to
5791009
Compare
PR Description
Add the consensus block value to the Block V3 API:
Eth-Consensus-Block-Value
headerconsensus_block_value
fieldFixed Issue(s)
fixes #7390
Documentation
doc-change-required
label to this PR if updates are required.Changelog