Do not reuse Endpoint structs between API calls
#9007
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rkapka
requested review from
rauljordan,
terencechain and
nisdas
and removed request for
a team
Codecov Report
@@ Coverage Diff @@
## feature/api-middleware #9007 +/- ##
==========================================================
- Coverage 60.92% 59.83% -1.10%
==========================================================
Files 536 538 +2
Lines 38214 38276 +62
==========================================================
- Hits 23281 22901 -380
- Misses 11614 12081 +467
+ Partials 3319 3294 -25 |
rauljordan
approved these changes
Jun 9, 2021
|
|
||
| func handleGetBeaconStateSsz(m *gateway.ApiProxyMiddleware, endpoint gateway.Endpoint, writer http.ResponseWriter, request *http.Request) (handled bool) { | ||
| config := sszConfig{ | ||
| sszPath: "/eth/v1/debug/beacon/states/{state_id}/ssz", |
There was a problem hiding this comment.
We should move these hardcoded strings as variables at the top of the file or somewhere else for more visibility and easier editing in the future, but no big deal in this PR
rauljordan
added a commit
that referenced
this pull request
Jun 15, 2021
* HTTP proxy server for Eth2 APIs (#8904) * Implement API HTTP proxy server * cleanup + more comments * gateway will no longer be dependent on beaconv1 * handle error during ErrorJson type assertion * simplify handling of endpoint data * fix mux v1 route * use URL encoding for all requests * comment fieldProcessor * fix failing test * change proxy port to not interfere with e2e * gzl * simplify conditional expression * Move appending custom error header to grpcutils package * add api-middleware-port flag * fix documentation for processField * modify e2e port * change field processing error message * better error message for field processing * simplify base64ToHexProcessor * fix json structs * Run several new endpoints through API middleware (#8922) * Implement API HTTP proxy server * cleanup + more comments * gateway will no longer be dependent on beaconv1 * handle error during ErrorJson type assertion * simplify handling of endpoint data * fix mux v1 route * use URL encoding for all requests * comment fieldProcessor * fix failing test * change proxy port to not interfere with e2e * gzl * simplify conditional expression * Move appending custom error header to grpcutils package * add api-middleware-port flag * fix documentation for processField * modify e2e port * change field processing error message * better error message for field processing * simplify base64ToHexProcessor * fix json structs * /eth/v1/beacon/states/{state_id}/validators * /eth/v1/beacon/states/{state_id}/validators/{validator_id} * /eth/v1/beacon/states/{state_id}/validator_balances * /eth/v1/beacon/states/{state_id}/committees * allow skipping base64-encoding for query params * /eth/v1/beacon/pool/attestations * replace break with continue * Remove unused functions (#8924) Co-authored-by: terence tsao <terence@prysmaticlabs.com> * Process SSZ-serialized beacon state through API middleware (#8925) * update field names * Process SSZ-serialized beacon state through API middleware * revert changes to go.mod and go.sum * Revert "Merge branch '__develop' into feature/api-middleware" This reverts commit 7c739a8, reversing changes made to 2d0f8e0. * update ethereumapis * update validator field name * update deps.bzl * update json tags (#8942) * Run `/node/syncing` through API Middleware (#8944) * add IsSyncing field to grpc response * run /node/syncing through the middleware Co-authored-by: Raul Jordan <raul@prysmaticlabs.com> * Return HTTP status codes other than 200 and 500 from node and debug endpoints (#8937) * error codes for node endpoints * error codes for debug endpoints * better comment about headers * gzl * review comments * comment on return value * update fakeChecker used for fuzz tests * fix failing tests * Allow to pass URL params literally, without encoding to base64 (#8938) * Allow to pass URL params literally, without encoding to base64 * fix compile error Co-authored-by: Raul Jordan <raul@prysmaticlabs.com> * Process SSZ-serialized beacon state through API middleware (#8925) * update field names * Process SSZ-serialized beacon state through API middleware * revert changes to go.mod and go.sum * Revert "Merge branch '__develop' into feature/api-middleware" This reverts commit 7c739a8, reversing changes made to 2d0f8e0. * update ethereumapis * update validator field name * update deps.bzl * update json tags (#8942) * Run `/node/syncing` through API Middleware (#8944) * add IsSyncing field to grpc response * run /node/syncing through the middleware Co-authored-by: Raul Jordan <raul@prysmaticlabs.com> * Return HTTP status codes other than 200 and 500 from node and debug endpoints (#8937) * error codes for node endpoints * error codes for debug endpoints * better comment about headers * gzl * review comments * comment on return value * update fakeChecker used for fuzz tests * fix failing tests * Allow to pass URL params literally, without encoding to base64 (#8938) * Allow to pass URL params literally, without encoding to base64 * fix compile error Co-authored-by: Raul Jordan <raul@prysmaticlabs.com> * unused import * Return correct status codes from beacon endpoints (#8960) * Various API Middleware fixes (#8963) * Return correct status codes from `/states` endpoints * better error messages in debug and node * better error messages in state * returning correct error codes from validator endpoints * correct error codes for getting a block header * gzl * fix err variable name * fix nil block comparison * test fixes * make status enum test better * fix ineffectual assignment * make PR unstuck * return proper status codes * return uppercase keys from /config/spec * return lowercase validator status * convert requested enum values to uppercase * validator fixes * Implement `/beacon/headers` endpoint (#8966) * Refactor API Middleware into more manageable code (#8984) * move endpoint registration out of shared package * divide main function into smaller components * return early on error * implement hooks * implement custom handlers and add documentation * fix test compile error * restrict package visibility * remove redundant error checking * rename file * API Middleware unit tests (#8998) * move endpoint registration out of shared package * divide main function into smaller components * return early on error * implement hooks * implement custom handlers and add documentation * fix test compile error * restrict package visibility * remove redundant error checking * rename file * api_middleware_processing * endpoints * gzl * remove gazelle:ignore * merge * Implement SSZ version of `/blocks/{block_id}` (#8970) * Implement SSZ version of `/blocks/{block_id}` * add dependencies back * fix indentation in deps.bzl * parameterize ssz functions * get block ssz * update ethereumapis dependency * gzl * Do not reuse `Endpoint` structs between API calls (#9007) * code refactor * implement endpoint factory * fix test * fmt * include pbs * gaz * test naming fixes * remove unused code * radek comments * revert endpoint test * bring back bytes test case * move `signedBeaconBlock` to `migration` package * change `fmt.Errorf` to `errors.Wrap` * capitalize SSZ * capitalize URL * more review feedback * rename `handleGetBlockSSZ` to `handleGetBeaconBlockSSZ` * rename `IndexOutOfRangeError` to `ValidatorIndexOutOfRangeError` * simplify parameter names * test header * more corrections * properly allocate array capacity Co-authored-by: terence tsao <terence@prysmaticlabs.com> Co-authored-by: Raul Jordan <raul@prysmaticlabs.com> Co-authored-by: Nishant Das <nishdas93@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
The current endpoint registration design results in the same
Endpointstruct being used for all calls to a single URL. Because of this results of one call may be visible to subsequent calls, producing incorrect outcomes.The new design introduces an
EndpointFactoryinterface and a beacon chain implementation of this interface. The factory is responsible for producing a newEndpointinstance on each call tofactory.Create.Which issues(s) does this PR fix?
Part of API Middleware feature
Other notes for review
I split up the
endpoints.gofile into multiple smaller files for better readability.