Counterintuitive warp error for Unsupported endpoint version
#3404
Comments
|
It may be that we want to avoid
|
|
I'll look into this one and follow up, thx |
@jmcph4 and I got this working on #4316. Our approach avoids returning a lighthouse/beacon_node/http_api/src/lib.rs Lines 1232 to 1264 in 73b30a6 Jack will fix just the new v2/blocks endpoints in that PR, and this issue can remain open until we roll out the fix across the codebase. We might want to write a small abstraction to minimise the amount of code duplication (something like |
|
@michaelsproul Thanks for helping get this one moving. Been a bit busy lately but should be freeing up more in the immediate future. I'm not sure how a rejection handler solves our initial issue of warp mixing up endpoint specific logic... If you've got any advice/guidance there it would be much appreciated. Also, do you mind clarifying how this would look? |
|
Hey @muang0, the pattern we found that works is this one: lighthouse/beacon_node/http_api/src/lib.rs Lines 1248 to 1274 in dfcb336 i.e. change the The complication is that, then_or_error(|| {
// this function `f` returns a `Result<T, Rejection>`, which `then_or_error` will convert to a
// `Response`. We can hopefully do a find and replace of `and_then` for `then_or_error`,
// with minimal other changes.
f()
})The implementation of use warp::{filters::BoxedFilter, Filter, reply::Reply};
// Mixin trait (add methods to all warp types implementing Filter)
pub trait ThenOrError {
fn then_or_error<G, R>(self, g: G) -> BoxedFilter<(Response,)>
where G: Future<Item = Result<R, Rejection>>,
R: Reply,
{
self.then(|| {
match f.await {
Ok(result) => result.into_response(),
Err(e) => match warp_utils::reject::handle_rejection(e).await {
Ok(reply) => reply.into_response(),
Err(_) => warp::reply::with_status(
StatusCode::INTERNAL_SERVER_ERROR,
eth2::StatusCode::INTERNAL_SERVER_ERROR,
)
.into_response(),
},
}
})
.boxed()
}
}
impl<F> ThenOrError for F: Filter {}This might require some tweaks to get working. We are also working on a big overhaul of the HTTP API here: #4462, so it might make sense for that to stabilise before rolling out this change (or we can raise a PR based on #4462 and merge this afterwards). I'm away for the next two weeks, but if you feel like working on it in the meantime I hope this is enough info to make progress |
|
Thanks for the clarification @michaelsproul I think we may be able to solve this using warp-native constructs (see example here). I'll play around with it a bit tomorrow. In the meantime, can anyone provide context around the choice of a custom |
|
Hey @muang0, I don't think lighthouse/beacon_node/http_api/src/lib.rs Line 3945 in dfcb336 The problem is the
I don't think there's a better solution, but willing to be proved wrong. The boxing greatly simplifies the types and prevents us from hitting the compiler's recursion limit on massive types like |
|
Assuming no concerns, I'll update an endpoint to use |
|
|
|
|
|
Update: Adding recover() to each filter didn't solve the backtracking issue. I'll try the Mixin approach next. |
## Issue Addressed Closes #3404 (mostly) ## Proposed Changes - Remove all uses of Warp's `and_then` (which backtracks) in favour of `then` (which doesn't). - Bump the priority of the `POST` method for `v2/blocks` to `P0`. Publishing a block needs to happen quickly. - Run the new SSZ POST endpoints on the beacon processor. I think this was missed in between merging #4462 and #4504/#4479. - Fix a minor issue in the validator registrations endpoint whereby an error from spawning the task on the beacon processor would be dropped. ## Additional Info I've tested this manually and can confirm that we no longer get the dreaded `Unsupported endpoint version` errors for queries like: ``` $ curl -X POST -H "Content-Type: application/json" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: WeakSubjectivityConflict", "stacktraces": [] } ``` ``` $ curl -X POST -H "Content-Type: application/octet-stream" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: invalid SSZ: OffsetOutOfBounds(572530811)", "stacktraces": [] } ``` ``` $ curl "http://localhost:5052/eth/v2/validator/blocks/7067595" {"code":400,"message":"BAD_REQUEST: invalid query: Invalid query string","stacktraces":[]} ``` However, I can still trigger it by leaving off the `Content-Type`. We can re-test this aspect with #4575.
## Issue Addressed Closes #3404 (mostly) ## Proposed Changes - Remove all uses of Warp's `and_then` (which backtracks) in favour of `then` (which doesn't). - Bump the priority of the `POST` method for `v2/blocks` to `P0`. Publishing a block needs to happen quickly. - Run the new SSZ POST endpoints on the beacon processor. I think this was missed in between merging #4462 and #4504/#4479. - Fix a minor issue in the validator registrations endpoint whereby an error from spawning the task on the beacon processor would be dropped. ## Additional Info I've tested this manually and can confirm that we no longer get the dreaded `Unsupported endpoint version` errors for queries like: ``` $ curl -X POST -H "Content-Type: application/json" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: WeakSubjectivityConflict", "stacktraces": [] } ``` ``` $ curl -X POST -H "Content-Type: application/octet-stream" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: invalid SSZ: OffsetOutOfBounds(572530811)", "stacktraces": [] } ``` ``` $ curl "http://localhost:5052/eth/v2/validator/blocks/7067595" {"code":400,"message":"BAD_REQUEST: invalid query: Invalid query string","stacktraces":[]} ``` However, I can still trigger it by leaving off the `Content-Type`. We can re-test this aspect with #4575.
|
@michaelsproul |
Closes sigp#3404 (mostly) - Remove all uses of Warp's `and_then` (which backtracks) in favour of `then` (which doesn't). - Bump the priority of the `POST` method for `v2/blocks` to `P0`. Publishing a block needs to happen quickly. - Run the new SSZ POST endpoints on the beacon processor. I think this was missed in between merging sigp#4462 and sigp#4504/sigp#4479. - Fix a minor issue in the validator registrations endpoint whereby an error from spawning the task on the beacon processor would be dropped. I've tested this manually and can confirm that we no longer get the dreaded `Unsupported endpoint version` errors for queries like: ``` $ curl -X POST -H "Content-Type: application/json" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: WeakSubjectivityConflict", "stacktraces": [] } ``` ``` $ curl -X POST -H "Content-Type: application/octet-stream" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: invalid SSZ: OffsetOutOfBounds(572530811)", "stacktraces": [] } ``` ``` $ curl "http://localhost:5052/eth/v2/validator/blocks/7067595" {"code":400,"message":"BAD_REQUEST: invalid query: Invalid query string","stacktraces":[]} ``` However, I can still trigger it by leaving off the `Content-Type`. We can re-test this aspect with sigp#4575.
Closes sigp#3404 (mostly) - Remove all uses of Warp's `and_then` (which backtracks) in favour of `then` (which doesn't). - Bump the priority of the `POST` method for `v2/blocks` to `P0`. Publishing a block needs to happen quickly. - Run the new SSZ POST endpoints on the beacon processor. I think this was missed in between merging sigp#4462 and sigp#4504/sigp#4479. - Fix a minor issue in the validator registrations endpoint whereby an error from spawning the task on the beacon processor would be dropped. I've tested this manually and can confirm that we no longer get the dreaded `Unsupported endpoint version` errors for queries like: ``` $ curl -X POST -H "Content-Type: application/json" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: WeakSubjectivityConflict", "stacktraces": [] } ``` ``` $ curl -X POST -H "Content-Type: application/octet-stream" --data @block.json "http://localhost:5052/eth/v2/beacon/blocks" | jq { "code": 400, "message": "BAD_REQUEST: invalid SSZ: OffsetOutOfBounds(572530811)", "stacktraces": [] } ``` ``` $ curl "http://localhost:5052/eth/v2/validator/blocks/7067595" {"code":400,"message":"BAD_REQUEST: invalid query: Invalid query string","stacktraces":[]} ``` However, I can still trigger it by leaving off the `Content-Type`. We can re-test this aspect with sigp#4575.
Description
When hitting the
/eth/v2/validators/blocks/{slot}endpoint with an invalidrandao_reveal(andverify_randao=trueor empty) Lighthouse will return this error:This makes zero sense, as the endpoint is cleared declared supporting v2 here:
lighthouse/beacon_node/http_api/src/lib.rs
Lines 1928 to 1935 in 2983235
Adding a panic to
unsupported_endpoint_versionto get a backtrace shows some funkywarpstuff going on:i.e. it seems that
warpis doing some counterintuitive backtracking through a fewand_then/and/oruntil it hits theunsupported_version_endpointfor a different endpoint (one that isv1only). This makes little sense to me, and would probably require someone to dig pretty deep intowarpto understand why this happens. I think this is similar to another open issue (#3112) related to our apparent misuse ofwarpcombinators.Finally, if
verify_randao=falseis set or the randao reveal is valid then no error occurs:Version
v2.5.0
Related issues
The text was updated successfully, but these errors were encountered: