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

Add PostAttestationV2 API #8448

Merged
merged 11 commits into from
Aug 1, 2024

Conversation

mehdi-aouadi
Copy link
Contributor

@mehdi-aouadi mehdi-aouadi commented Jul 16, 2024

PR Description

  • Add PostAttestationV2 API
  • Handle arrays with different item types deserialisation
  • Add a header based schema selector

Fixed Issue(s)

#8424

Documentation

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

Changelog

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

@mehdi-aouadi mehdi-aouadi force-pushed the array-type-definition branch 2 times, most recently from 8e1b367 to e7094ad Compare July 16, 2024 21:03
@mehdi-aouadi mehdi-aouadi changed the title wip array type definition Add PostAttestationV2 API Jul 17, 2024
@mehdi-aouadi mehdi-aouadi marked this pull request as ready for review July 17, 2024 13:11
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added new PostAttestationV2 API endpoint (/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/JsonTypeDefinitionBeaconRestApi.java, /data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostAttestationV2.java)
  • Introduced header-based schema selector (/data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/beacon/MilestoneDependentTypesUtil.java)
  • Enhanced deserialization for arrays with different item types (/infrastructure/json/src/main/java/tech/pegasys/teku/infrastructure/json/types/DeserializableOneOfTypeDefinition.java, /infrastructure/restapi/src/main/java/tech/pegasys/teku/infrastructure/restapi/endpoints/EndpointMetadata.java)
  • Added comprehensive tests for new API and schema selector (/data/beaconrestapi/src/test/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostAttestationV2Test.java, /data/beaconrestapi/src/test/java/tech/pegasys/teku/beaconrestapi/schema/MilestoneDependentTypesUtilTest.java)
  • Introduced new JSON files for testing and error responses (/data/beaconrestapi/src/test/resources/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/errorListBadRequest.json, /data/beaconrestapi/src/test/resources/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/postAttestationRequestBodyElectra.json, /data/beaconrestapi/src/test/resources/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/postAttestationRequestBodyPhase0.json)

14 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Added postAttestationRequestBodyELECTRA.json for PostAttestationV2 API testing
  • Added postAttestationRequestBodyPHASE0.json for PostAttestationV2 API testing
  • Ensure deserialization of arrays with different item types
  • Verify header-based schema selector functionality

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Introduced PostAttestationV2 API in data/beaconrestapi/src/integration-test/resources/tech/pegasys/teku/beaconrestapi/beacon/paths/_eth_v2_beacon_pool_attestations.json
  • Added header parameter 'Eth-Consensus-Version' for version handling
  • Supported multiple attestation schemas based on 'Eth-Consensus-Version'

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@mehdi-aouadi mehdi-aouadi self-assigned this Jul 17, 2024
@mehdi-aouadi mehdi-aouadi force-pushed the array-type-definition branch 2 times, most recently from 7f7abcd to c295f56 Compare July 18, 2024 10:50
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Added PostAttestationV2 API in data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostAttestationV2.java
  • Implemented header-based schema selection in MilestoneDependentTypesUtil.java
  • Added support for arrays with different item types in EndpointMetadata.java
  • Introduced OneOfArrayJsonRequestContentTypeDefinition.java for complex request body handling
  • Added IPv6 support in DiscV5Service.java and NodeRecordConverter.java

18 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Added PostAttestationV2 API in data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v2/beacon/PostAttestationV2.java
  • Implemented header-based schema selection for attestation deserialization
  • Simplified error handling in MilestoneDependentTypesUtil.java
  • Removed listOf method from DeserializableOneOfTypeDefinition.java
  • Renamed requestBodyTypeForList to requestBodyType in EndpointMetadata.java

4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@mehdi-aouadi mehdi-aouadi mentioned this pull request Jul 19, 2024
2 tasks
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Added integration tests for PostAttestationsV2 API in data/beaconrestapi/src/integration-test/java/tech/pegasys/teku/beaconrestapi/v2/beacon/PostAttestationsV2IntegrationTest.java
  • Updated JsonTypeDefinitionBeaconRestApi.java to use PostAttestationsV2 class
  • Introduced PostAttestationsV2.java to handle new API endpoint /eth/v2/beacon/pool/attestations
  • Renamed and updated tests in PostAttestationsV2Test.java to reflect new API endpoint

4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@mehdi-aouadi mehdi-aouadi mentioned this pull request Jul 19, 2024
2 tasks
@mehdi-aouadi mehdi-aouadi force-pushed the array-type-definition branch 2 times, most recently from 4f1c076 to 4b28766 Compare July 29, 2024 09:06
@mehdi-aouadi mehdi-aouadi force-pushed the array-type-definition branch from 4b28766 to 0472e2a Compare July 30, 2024 14:25
Copy link
Contributor

@gfukushima gfukushima left a comment

Choose a reason for hiding this comment

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

lgtm with some minor asks

@mehdi-aouadi mehdi-aouadi enabled auto-merge (squash) August 1, 2024 07:49
@mehdi-aouadi mehdi-aouadi merged commit 38d2635 into Consensys:master Aug 1, 2024
16 checks passed
@mehdi-aouadi mehdi-aouadi deleted the array-type-definition branch August 1, 2024 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants