Add a "How-To" example for performing HTTP requests#3509
Add a "How-To" example for performing HTTP requests#3509jvff merged 32 commits intolinera-io:mainfrom
Conversation
## Motivation For security reasons, we would like to initially limit the hosts that applications can send HTTP requests to. ## Proposal Add an allow-list of hosts to the `ResourceControlPolicy` configured by a committee. Only perform HTTP requests in the execution state actor if the target host is in the list. ## Test Plan Tests that exercise the allow list were added in PR #3509. ## Release Plan - Nothing to do / These changes follow the usual release cycle, because this is a new feature that should be included in the next release and testnet. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
2d7b1b5 to
f6778af
Compare
971dcb8 to
ce3adba
Compare
| impl Secp256k1PublicKey { | ||
| /// A fake public key used for testing. | ||
| #[cfg(with_testing)] | ||
| #[cfg(all(with_testing, not(target_arch = "wasm32")))] |
There was a problem hiding this comment.
Did you encounter problems with compilation here?
There was a problem hiding this comment.
Yes, I believe when it tried to compile the tests for wasm32-unknown-unknown.
|
|
||
| /// Tests if service query can't perform HTTP requests to hosts that aren't allowed. | ||
| #[test_log::test(tokio::test)] | ||
| #[should_panic(expected = "UnauthorizedHttpRequest")] |
There was a problem hiding this comment.
Does it expect a "Rust panic" in the service part? How will that behave on the validator node (client)? Will it propagate the panic and shut down the node?
There was a problem hiding this comment.
Not exactly. This expects a panic from the ActiveChain used as the test helper:
linera-protocol/linera-sdk/src/test/chain.rs
Line 217 in 524c61a
To help with writing tests, the types in linera_sdk::test usually don't return Result and panic in case of an error, because that's usually what the test does anyway. There are exceptions with try_* methods, and I think the cases in this example would make more sense to assert the error type instead.
| chain | ||
| .graphql_mutation(application_id, "mutation { performHttpRequest }") | ||
| .await; | ||
|
|
||
| Ok(()) |
There was a problem hiding this comment.
It looks like it's strictly less test than service_query_performs_http_request?
There was a problem hiding this comment.
Not exactly, because this uses mutation { performHttpRequest } while the other test uses query { performHttpRequest }, so the entrypoints are completely different. Maybe the names should be different to avoid confusion. Either having the mutation something like performHttpRequestAndSendToContract (but shorter 😅) or having the query be performLocalHttpQuery.
| .await; | ||
|
|
||
| chain | ||
| .graphql_mutation(application_id, "mutation { performHttpRequest }") |
There was a problem hiding this comment.
It'd be nice to be able to really see that the block proposal failed for exactly the expected reason here.
There was a problem hiding this comment.
I agree, I'll see if I can add a try_graphql_mutation method that returns a Result instead.
There was a problem hiding this comment.
A more general question:
Why do we need all these ways to perform an HTTP query? If only the service was allowed to make queries then it could be used both on~ and off~ chain:
- A client node calls the service only and consumes the results to construct a block
- A contract calls into its service and consumes the result (acting as an oracle)
ma2bd
left a comment
There was a problem hiding this comment.
see comment
Also the example is not run in CI, is it?
…3540) ## Motivation Epoch changes were recently changed to use events (#3432), but the `TestValidator::change_resource_control_policy` was still trying to receive epoch change system messages instead of creating an operation to process the new epoch. ## Proposal Use the new `SystemOperation::ProcessNewEpoch` to update the chains to use the new epoch, and skip updating the admin chain because it fails to try to process the same epoch twice. ## Test Plan Tested with a rebased version of the tests in #3509. The tests stop working after #3432 but pass again after this PR. ## Release Plan - These changes follow the usual release cycle, because it fixes an issue with SDK that is not present in any released version. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation HTTP requests can take a long time to complete, or attempt to stay open indefinitely. This should not be allowed because it may stall a chain. ## Proposal Add a `http_request_timeout` policy to `ResourceControlPolicy`, and configure all HTTP requests to use that timeout. ## Test Plan Wrote some [tests](https://github.com/jvff/linera-protocol-archive/compare/95da70cabb..test-http-timeouts-p1) out of this branch, possibly to be added to #3509. ## Release Plan - Backporting is not possible but we may want to deploy a new `devnet` and `testnet` and release a new SDK soon, because this contains backwards incompatible changes to the chain state. ## Links - Closes #3522 - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
## Motivation Applications could cause excessive bandwidth costs to validators by performing HTTP requests to obtain large amounts of data as fast as possible. ## Proposal Ensure that HTTP responses are limited in size to a value agreed among the validators. ## Test Plan Wrote some unit tests in a separate branch, using the code from #3509: https://github.com/jvff/linera-protocol-archive/blob/test-http-response-limits-p1/examples/how-to/perform-http-requests/tests/response_size_tests.rs ## Release Plan - Backporting is not possible but we may want to deploy a new `devnet` and release a new SDK soon, because this contains changes to the consensus critical `ResourceControlPolicy`. ## Links - Closes #3521 - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
The contract should successfully execute an operation with a valid off-chain HTTP response.
Ensure that the contract panics if the off-chain HTTP response can't be trusted.
The received response is sent to the contract to validate.
Ensure that the service schedules the operation with a received HTTP response.
Ensure that it forwards the received HTTP response to the contract to verify.
Ensure that if the service reaches an untrusted HTTP server, the contract rejects the HTTP response.
Demonstrate how an HTTP request is made in the contract.
Add a unit test showing how to mock the response.
Ensure that the response is being verified.
Allow the service to request the contract to perform an HTTP request.
Ensure that the mutation schedules the operation for the contract.
And that it is accepted if the response is valid.
It should reject a response that it does not trust.
Request the service to perform an HTTP request for the contract, returning only the relevant deterministic output.
Demonstrate how to mock the expected service query.
A reminder that the oracle's HTTP response also needs to be verified.
Provide a mutation to create an operation that uses the service as an oracle.
Ensure that it produces the operation for the contract to use the service as an oracle.
Ensure that the service performs an HTTP request when it is called as an oracle.
A reminder that the oracle's HTTP response also needs to be verified.
04bba32 to
681f405
Compare
Describe the different ways HTTP requests can be made, and compare their usages.
Run a very simple HTTP server for executing the README example.
Ensure that the README instructions are up-to-date.
681f405 to
43c5a08
Compare
## Motivation The integration tests for the HTTP Requests How-To example (#3509) used the `#[should_panic]` attribute for testing failures. However, as [pointed out during review](#3509 (comment)), these are quite broad and may lead to mistakes. Being able to assert the exact error leads to better tests and a better example for external developers. ## Proposal Add `try_query`, `try_graphql_query` and `try_graphql_mutation` helper methods to the `ActiveChain` test interface. Use the new methods to replace usages of `#[should_panic]` with error type assertions. To reduce boilerplate code, a `WorkerError::expect_execution_error` test helper method was added, that dives into the internal `ExecutionError` stored inside a `WorkerError`. ## Test Plan CI should catch any regressions caused by this refactor to the tests. ## Release Plan - These changes follow the usual release cycle, because this only includes small backward-compatible API additions to the public API and SDK. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
Motivation
Performing HTTP requests from applications is an important feature for Linera. There are a few different ways for applications to perform them, and there are tradeoffs between these ways. Therefore, this feature should be documented and a "How-To" example demonstrating how to use and test the feature would be useful.
Proposal
Add an example that performs HTTP requests in all different ways, and test them.
Test Plan
Unit and integration tests were added to test not just the application, but also to serve as testing examples and to also test the HTTP allow-list.
Release Plan
Links