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 POST /eth/v1/validator/sync_committee_subscriptions for altair #3918

Merged
merged 12 commits into from
May 4, 2021

Conversation

rolfyone
Copy link
Contributor

fixes #3907

Signed-off-by: Paul Harris [email protected]

Documentation

  • I thought about documentation and added the documentation label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@rolfyone rolfyone marked this pull request as ready for review May 4, 2021 02:24
Signed-off-by: Paul Harris <[email protected]>
Comment on lines 60 to 68
public static SyncCommitteeSubnetSubscription create(
tech.pegasys.teku.api.schema.altair.SyncCommitteeSubnetSubscription schemaSubscription) {
return new SyncCommitteeSubnetSubscription(
schemaSubscription.validatorIndex.intValue(),
schemaSubscription.syncCommitteeIndices.stream()
.map(UInt64::intValue)
.collect(Collectors.toList()),
schemaSubscription.untilEpoch);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not real keen on the REST API objects bleeding into the internal objects like this. Previously we've had a .toInternal() on the API object so it could go the other way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the thing is the validator objects go both ways and this is way simpler than the resulting code if we don't do it. but i guess i'll change it.

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM.

@rolfyone rolfyone merged commit 27dafd2 into Consensys:master May 4, 2021
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.

implement POST /eth/v1/validator/sync_committee_subscriptions
2 participants