diff --git a/Makefile b/Makefile index 9ec1c636a..df1f0c0ab 100644 --- a/Makefile +++ b/Makefile @@ -383,6 +383,11 @@ test_e2e: kubectl_check ## Run all E2E tests echo "IMPROVE(#759): Make sure you ran 'make localnet_up' in case this fails with infrastructure related errors." go test ${VERBOSE_TEST} -count=1 -tags=test,e2e ./e2e/tests/... +.PHONY: test_e2e_relay +test_e2e_relay: kubectl_check + echo "IMPROVE(#759): Make sure you ran 'make localnet_up' in case this fails with infrastructure related errors." + go test ${VERBOSE_TEST} -count=1 -tags=test,e2e -run TestRelay ./e2e/tests/... + .PHONY: test_all_with_json_coverage test_all_with_json_coverage: generate_rpc_openapi ## Run all go unit tests, output results & coverage into json & coverage files go test -p=1 -count=1 -tags=test -json ./... -covermode=count -coverprofile=coverage.out | tee test_results.json | jq @@ -502,7 +507,8 @@ benchmark_p2p_peerstore: ## Run P2P peerstore benchmarks # BUG - There is a known existing bug in this code # DISCUSS_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way for the reviewer of a PR to start / reply to a discussion. # TODO_IN_THIS_COMMIT - SHOULD NEVER BE COMMITTED TO MASTER. It is a way to start the review process while non-critical changes are still in progress -TODO_KEYWORDS = -e "TODO" -e "DECIDE" -e "TECHDEBT" -e "IMPROVE" -e "OPTIMIZE" -e "DISCUSS" -e "INCOMPLETE" -e "INVESTIGATE" -e "CLEANUP" -e "HACK" -e "REFACTOR" -e "CONSIDERATION" -e "TODO_IN_THIS_COMMIT" -e "DISCUSS_IN_THIS_COMMIT" -e "CONSOLIDATE" -e "DEPRECATE" -e "ADDTEST" -e "RESEARCH" -e "BUG" +# ADD_IN_THIS_PR - SHOULD NEVER BE COMMITTED TO MASTER. It is a way to mark functionality that should be added, possibly in a different PR, to allow this comment to be removed. +TODO_KEYWORDS = -e "TODO" -e "DECIDE" -e "TECHDEBT" -e "IMPROVE" -e "OPTIMIZE" -e "DISCUSS" -e "INCOMPLETE" -e "INVESTIGATE" -e "CLEANUP" -e "HACK" -e "REFACTOR" -e "CONSIDERATION" -e "TODO_IN_THIS_COMMIT" -e "DISCUSS_IN_THIS_COMMIT" -e "ADD_IN_THIS_PR" -e "CONSOLIDATE" -e "DEPRECATE" -e "ADDTEST" -e "RESEARCH" -e "BUG" # How do I use TODOs? # 1. : ; diff --git a/app/client/cli/servicer.go b/app/client/cli/servicer.go index 0ed35dff4..4f9040123 100644 --- a/app/client/cli/servicer.go +++ b/app/client/cli/servicer.go @@ -104,7 +104,7 @@ Will prompt the user for the *application* account passphrase`, return fmt.Errorf("error getting servicer for the relay: %w", err) } - relay, err := buildRelay(relayPayload, pk, session, servicer) + relay, err := buildRelay(relayPayload, pk, session, servicer, applicationAddr) if err != nil { return fmt.Errorf("error building relay from payload: %w", err) } @@ -243,13 +243,13 @@ func sendTrustlessRelay(ctx context.Context, servicerUrl string, relay *rpc.Rela return client.PostV1ClientRelayWithResponse(ctx, *relay) } -func buildRelay(payload string, appPrivateKey crypto.PrivateKey, session *rpc.Session, servicer *rpc.ProtocolActor) (*rpc.RelayRequest, error) { +func buildRelay(payload string, appPrivateKey crypto.PrivateKey, session *rpc.Session, servicer *rpc.ProtocolActor, appAddr string) (*rpc.RelayRequest, error) { // TECHDEBT: This is mostly COPIED from pocket-go: we should refactor pocket-go code and import this functionality from there instead. - relayPayload := rpc.Payload{ - // INCOMPLETE(#803): need to unmarshal into JSONRPC and other supported relay formats once proto-generated custom types are added. - Jsonrpc: "2.0", - Method: payload, - // INCOMPLETE: set Headers for HTTP relays + var relayPayload rpc.Payload + // INCOMPLETE(#803): need to unmarshal into JSONRPC and other supported relay formats once proto-generated custom types are added. + // INCOMPLETE: set Headers for HTTP relays + if err := json.Unmarshal([]byte(payload), &relayPayload); err != nil { + return nil, fmt.Errorf("error unmarshalling relay payload %s: %w", payload, err) } relayMeta := rpc.RelayRequestMeta{ @@ -260,6 +260,7 @@ func buildRelay(payload string, appPrivateKey crypto.PrivateKey, session *rpc.Se }, ServicerPubKey: servicer.PublicKey, // TODO(#697): Geozone + ApplicationAddress: appAddr, } relay := &rpc.RelayRequest{ diff --git a/build/localnet/manifests/configs.yaml b/build/localnet/manifests/configs.yaml index da8a091e7..18a648865 100644 --- a/build/localnet/manifests/configs.yaml +++ b/build/localnet/manifests/configs.yaml @@ -1665,7 +1665,7 @@ data: { "address": "00001fff518b1cdddd74c197d76ba5b5dedc0301", "public_key": "05a25e527bf6f51676f61f2f1a96efaa748218ac82f54d3cdc55a4881389eb60", - "chains": ["0001"], + "chains": ["0001", "9999"], "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, @@ -1698,7 +1698,7 @@ data: { "address": "001022b138896c4c5466ac86b24a9bbe249905c2", "public_key": "56915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495", - "chains": ["0001"], + "chains": ["0001", "9999"], "service_url": "http://servicer-001-pocket:50832", "staked_amount": "1000000000000", "paused_height": -1, diff --git a/charts/pocket/pocket-servicer-overrides.yaml b/charts/pocket/pocket-servicer-overrides.yaml new file mode 100644 index 000000000..63b545086 --- /dev/null +++ b/charts/pocket/pocket-servicer-overrides.yaml @@ -0,0 +1,19 @@ +# - This is an override of the shared config: +# - This is a reference of how we can utilize Helm to configure the service +# - Configs can also be overrident without an explicit config file: https://github.com/pokt-network/pocket/blob/main/build/localnet/Tiltfile#L216 +# - Settings specific to a single instance of the servicer, e.g. public key and address, are a good fit for this file +config: + servicer: + enabled: true + # The address and public_key fields are taken from the genesis section of the config file: + # https://github.com/pokt-network/pocket/blob/40f325305c1756bbfd069bf139fa67545419981c/build/localnet/manifests/configs.yaml#L1699 + address: "001022b138896c4c5466ac86b24a9bbe249905c2" + public_key: "56915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495" + services: + "0001": + url: "https://eth-mainnet.gateway.pokt.network" + # 9999 is chosen as the ID for a service that always times out when responding to relays + "9999": + # Port 22222 is used by the mock service node to support timeout test scenarios + url: "http://localhost:22222/timing_out_service" + TimeoutMsec: 50 diff --git a/e2e/tests/steps_init_test.go b/e2e/tests/steps_init_test.go index ee680cd82..f3ec575f5 100644 --- a/e2e/tests/steps_init_test.go +++ b/e2e/tests/steps_init_test.go @@ -4,10 +4,12 @@ package e2e import ( "fmt" + "net/http" "os" "path/filepath" "strings" "testing" + "time" pocketLogger "github.com/pokt-network/pocket/logger" "github.com/pokt-network/pocket/runtime/defaults" @@ -32,8 +34,18 @@ const ( // validatorB maps to suffix ID 002 and receives in the Send test. validatorB = "002" chainId = "0001" + + // 001 servicer is in session 0 for applicatio 000 + // The list of servicers in the session is decided by the 'servicers' section of the genesis, from 'build/localnet/manifest/configs.yaml' file + servicerA = "001" + appA = "000" + serviceA = "0001" + timeoutService = "9999" + + relaychainEth = "RelayChainETH" // used to refer to Ethereum chain when retrieving relaychain settings ) +// TODO(#874, olshansky): Populate the app & servicer keys with the full set type rootSuite struct { gocuke.TestingT @@ -46,6 +58,27 @@ type rootSuite struct { // TECHDEBT: Rename `validator` to something more appropriate validator *validatorPod // validatorA maps to suffix ID 001 of the kube pod that we use as our control agent + + // servicerKeys is hydrated by the clientset with credentials for all servicers. + // servicerKeys maps servicer IDs to their private key as a hex string. + servicerKeys map[string]string + + // appKeys is hydrated by the clientset with credentials for all apps. + // appKeys maps app IDs to their private key as a hex string. + appKeys map[string]string + + // relaychains holds settings for all relaychains used in the tests + // the map key is a constant selected as the identifier for the relaychain, e.g. "RelayChainETH" represented as "0001" in other parts of the codebase for Ethereum + relaychains map[string]*relaychainSettings + + // servicer holds the key for the servicer that should received the relay + servicerKey string +} + +// relaychainSettings holds the settings for a specific relaychain +type relaychainSettings struct { + account string + height string } func (s *rootSuite) Before() { @@ -57,15 +90,50 @@ func (s *rootSuite) Before() { e2eLogger.Fatal().Err(err).Msg("failed to get validator key map") } + skmap, err := pocketk8s.FetchServicerPrivateKeys(clientSet) + if err != nil { + e2eLogger.Fatal().Err(err).Msg("failed to get validator key map") + } + + akmap, err := pocketk8s.FetchApplicationPrivateKeys(clientSet) + if err != nil { + e2eLogger.Fatal().Err(err).Msg("failed to get validator key map") + } + s.validator = new(validatorPod) s.clientset = clientSet s.validatorKeys = vkmap -} + s.servicerKeys = skmap + s.appKeys = akmap + s.relaychains = map[string]*relaychainSettings{ + relaychainEth: {}, + } + } // TestFeatures runs the e2e tests specified in any .features files in this directory // * This test suite assumes that a LocalNet is running that can be accessed by `kubectl` func TestFeatures(t *testing.T) { - gocuke.NewRunner(t, &rootSuite{}).Path("*.feature").Run() + // setup a mock service node that causes a timeout by sleeping for the specified duration + // 22222 is the port used for service ID "0004" in charts/pocket/pocket-servicer-overrides.yaml + // 100 is the delay in milliseconds, selected to be more than the timeout value for service "0004" in charts/pocket/pocket-servicer-overrides.yaml + // This setup is done here to ensure the http path is registered exactly once. + setupMockServiceNodeWithTimeOut(22222, 100) + + runner := gocuke.NewRunner(t, &rootSuite{}).Path("*.feature") + runTests(runner) +} + +// TestRelay builds a test runner which only includes relay tests +func TestRelay(t *testing.T) { + runner := gocuke.NewRunner(t, &rootSuite{}).Path("*_relays.feature") + runTests(runner) +} + +// runTests adds steps that need to be registered manually and runs the tests +func runTests(runner *gocuke.Runner) { + // DISCUSS: is there a better way to make gocuke pickup the balance, i.e. a hexadecimal, as a string in function argument? + runner.Step(`^the\srelay\sresponse\scontains\s([[:alnum:]]+)$`, (*rootSuite).TheRelayResponseContains) + runner.Run() } // InitializeScenario registers step regexes to function handlers @@ -157,6 +225,112 @@ func (s *rootSuite) getPrivateKey(validatorId string) cryptoPocket.PrivateKey { return privateKey } +// TheApplicationHasAValidEthereumRelaychainAccount fullfils the following condition from feature file: +// +// "Given the application has a valid ethereum relaychain account" +func (s *rootSuite) TheApplicationHasAValidEthereumRelaychainAccount() { + // Account: 0x8315177aB297bA92A06054cE80a67Ed4DBd7ed3a (Arbitrum Bridge) + s.relaychains[relaychainEth].account = "0x8315177aB297bA92A06054cE80a67Ed4DBd7ed3a" +} + +// TheApplicationHasAValidEthereumRelaychaindHeight fullfils the following condition from feature file: +// +// "Given the application has a valid ethereum relaychain height" +func (s *rootSuite) TheApplicationHasAValidEthereumRelaychainHeight() { + // Ethereum relaychain BlockNumber: 17605670 = 0x10CA426 + s.relaychains[relaychainEth].height = "0x10CA426" +} + +// TheApplicationHasAValidServicer fullfils the following condition from feature file: +// +// "Given the application has a valid servicer" +func (s *rootSuite) TheApplicationHasAValidServicer() { + s.servicerKey = servicerA +} + +// An Application requests the account balance of a specific address at a specific height +func (s *rootSuite) TheApplicationSendsAGetBalanceRelayAtASpecificHeightToAnEthereumServicer() { + // ADD_IN_THIS_PR: Add a servicer staked for the Ethereum RelayChain + params := fmt.Sprintf("%q: [%q, %q]", "params", s.relaychains[relaychainEth].account, s.relaychains[relaychainEth].height) + checkBalanceRelay := fmt.Sprintf("{%s, %s}", `"method": "eth_getBalance", "id": "1", "jsonrpc": "2.0"`, params) + + servicerPrivateKey := s.getServicerPrivateKey(s.servicerKey) + appPrivateKey := s.getAppPrivateKey(appA) + + s.sendTrustlessRelay(checkBalanceRelay, servicerPrivateKey.Address().String(), appPrivateKey.Address().String(), serviceA, true) +} + +// An Application requests the account balance of a specific address at a specific height on "ServiceWithTimeout", i.e. timing out, service +func (s *rootSuite) TheApplicationSendsAGetBalanceRelayAtASpecificHeightToTheServicewithtimeoutService() { + params := fmt.Sprintf("%q: [%q, %q]", "params", s.relaychains[relaychainEth].account, s.relaychains[relaychainEth].height) + checkBalanceRelay := fmt.Sprintf("{%s, %s}", `"method": "eth_getBalance", "id": "1", "jsonrpc": "2.0"`, params) + + servicerPrivateKey := s.getServicerPrivateKey(s.servicerKey) + appPrivateKey := s.getAppPrivateKey(appA) + + s.sendTrustlessRelay(checkBalanceRelay, servicerPrivateKey.Address().String(), appPrivateKey.Address().String(), timeoutService, false) +} + +// Then the request times out without a response +func (s *rootSuite) TheRequestTimesOutWithoutAResponse() { + require.Contains(s, s.validator.result.Stdout, "HTTP status code: 500") +} + +func (s *rootSuite) TheRelayResponseContains(relayResponse string) { + require.Contains(s, s.validator.result.Stdout, relayResponse) +} + +func (s *rootSuite) TheRelayResponseIsValidJsonRpc() { + require.Contains(s, s.validator.result.Stdout, `"jsonrpc":"2.0"`) +} + +func (s *rootSuite) TheRelayResponseHasValidId() { + require.Contains(s, s.validator.result.Stdout, `"id":1`) +} + +func (s *rootSuite) sendTrustlessRelay(relayPayload string, servicerAddr, appAddr, serviceId string, shouldSucceed bool) { + args := []string{ + "Servicer", + "Relay", + appAddr, + servicerAddr, + // IMPROVE: add ETH_Goerli as a chain/service to genesis + serviceId, + relayPayload, + } + + // TECHDEBT: run the command from a client, i.e. not a validator, pod. + res, err := s.validator.RunCommand(args...) + + if shouldSucceed { + require.NoError(s, err) + } + + s.validator.result = res +} + +// getAppPrivateKey generates a new keypair from the application private hex key that we get from the clientset +func (s *rootSuite) getAppPrivateKey( + appId string, +) cryptoPocket.PrivateKey { + privHexString := s.appKeys[appId] + privateKey, err := cryptoPocket.NewPrivateKey(privHexString) + require.NoErrorf(s, err, "failed to extract privkey for app with id %s", appId) + + return privateKey +} + +// getServicerPrivateKey generates a new keypair from the servicer private hex key that we get from the clientset +func (s *rootSuite) getServicerPrivateKey( + servicerId string, +) cryptoPocket.PrivateKey { + privHexString := s.servicerKeys[servicerId] + privateKey, err := cryptoPocket.NewPrivateKey(privHexString) + require.NoErrorf(s, err, "failed to extract privkey for servicer with id %s", servicerId) + + return privateKey +} + // getClientset uses the default path `$HOME/.kube/config` to build a kubeconfig // and then connects to that cluster and returns a *Clientset or an error func getClientset(t gocuke.TestingT) (*kubernetes.Clientset, error) { @@ -190,3 +364,20 @@ func inClusterConfig(t gocuke.TestingT) *rest.Config { return config } + +// setupMockServiceNodeWithTimeout sets up an http server on localhost that causes a timeout by delaying the response +// +// delay is the desired delay in milliseconds +func setupMockServiceNodeWithTimeOut(port int, delay int64) { + http.HandleFunc("/timing_out_service", func(http.ResponseWriter, *http.Request) { + time.Sleep(time.Millisecond * time.Duration(delay)) + return + }) + + go func() { + err := http.ListenAndServe(fmt.Sprintf(":%d", port), nil) + if err != nil { + e2eLogger.Fatal().Err(err).Msg("unexpected error in mock service") + } + }() +} diff --git a/e2e/tests/trustless_relays.feature b/e2e/tests/trustless_relays.feature new file mode 100644 index 000000000..0fcf5f818 --- /dev/null +++ b/e2e/tests/trustless_relays.feature @@ -0,0 +1,27 @@ +Feature: Trustless Relays + # Happy test case: An Application requests the account balance of a specific address at a specific height from a Servicer staked for the Ethereum RelayChain, and receives a successful response. + + # ADD_IN_THIS_PR: Add a servicer staked for the Ethereum relaychain to the genesis file + Scenario: Application can send a trustless relay to a relaychain to get an account's balance at a specific height + Given the application has a valid ethereum relaychain account + Given the application has a valid ethereum relaychain height + Given the application has a valid servicer + # INCOMPLETE: GeoZone + When the application sends a get balance relay at a specific height to an Ethereum Servicer + # Balance: 1,160,126.46817237178258965 ETH = 0xf5aa94f49d4fd1f8dcd2 + Then the relay response contains 0xf5aa94f49d4fd1f8dcd2 + And the relay response is valid json rpc + And the relay response has valid id + # TECHDEBT: replace validator with client + And the validator should have exited without error + + + # ADD_IN_THIS_PR: Sad test case: An Application requests the account balance of a specific address at a specific height from a Servicer staked for the Ethereum RelayChain in the same GeoZone, and the request times out without a response. + # Note: to test the timeout scenario, a local http server is setup which simply sleeps on receiving a request to trigger a timeout + Scenario: An Application requests the account balance of a specific address at a specific height from a Servicer staked for the "TimeoutService" RelayChain in the same GeoZone, and the request times out without a response. + Given the application has a valid servicer + When the application sends a get balance relay at a specific height to the ServiceWithTimeout Service + Then the request times out without a response + + # TODO: add an E2E test for a trustless relay, where the application retrieves the session first, using a new fetch session command + # TODO: add an E2E test for a trustless relay, where the application is not staked for a service but requests a relay for it, and gets rejected diff --git a/rpc/handlers.go b/rpc/handlers.go index e222b3e66..18bea8caf 100644 --- a/rpc/handlers.go +++ b/rpc/handlers.go @@ -105,11 +105,12 @@ func (s *rpcServer) PostV1ClientRelay(ctx echo.Context) error { Name: body.Meta.Geozone.Name, } relayMeta := &coreTypes.RelayMeta{ - BlockHeight: body.Meta.BlockHeight, - ServicerPublicKey: body.Meta.ServicerPubKey, - RelayChain: chain, - GeoZone: geozone, - Signature: body.Meta.Signature, + BlockHeight: body.Meta.BlockHeight, + ServicerPublicKey: body.Meta.ServicerPubKey, + RelayChain: chain, + GeoZone: geozone, + Signature: body.Meta.Signature, + ApplicationAddress: body.Meta.ApplicationAddress, } relayRequest := buildJsonRPCRelayPayload(&body) @@ -220,17 +221,18 @@ func (s *rpcServer) GetV1P2pStakedActorsAddressBook(ctx echo.Context, params Get func buildJsonRPCRelayPayload(body *RelayRequest) *coreTypes.Relay { payload := &coreTypes.Relay_JsonRpcPayload{ JsonRpcPayload: &coreTypes.JSONRPCPayload{ - JsonRpc: body.Payload.Jsonrpc, + Jsonrpc: body.Payload.Jsonrpc, Method: body.Payload.Method, }, } if body.Payload.Id != nil { - payload.JsonRpcPayload.Id = []byte(*body.Payload.Id) + payload.JsonRpcPayload.Id = &coreTypes.JSONRPCId{Id: []byte(*body.Payload.Id)} } - if body.Payload.Parameters != nil { - payload.JsonRpcPayload.Parameters = *body.Payload.Parameters + if body.Payload.Params != nil { + // DISCUSS: Need a decision and implementation on Params field and conversion from rpc to proto + payload.JsonRpcPayload.Params = *body.Payload.Params } if body.Payload.Headers != nil { diff --git a/rpc/v1/openapi.yaml b/rpc/v1/openapi.yaml index c068b4238..14a9053e9 100644 --- a/rpc/v1/openapi.yaml +++ b/rpc/v1/openapi.yaml @@ -1710,16 +1710,17 @@ components: - jsonrpc - method properties: + # INCOMPLETE: need to support string, number, and NULL values, according to JSONRPC spec id: type: string - format: byte jsonrpc: type: string method: type: string - parameters: - type: string - format: byte + params: + type: array + items: + type: string headers: $ref: "#/components/schemas/Headers" Pool: @@ -1773,6 +1774,7 @@ components: - geozone - token - signature + - application_address properties: block_height: type: integer @@ -1787,6 +1789,8 @@ components: $ref: "#/components/schemas/AAT" signature: type: string + application_address: + type: string Signature: type: object required: diff --git a/shared/core/types/proto/relay.proto b/shared/core/types/proto/relay.proto index 6061a3a62..ff9f12c4e 100644 --- a/shared/core/types/proto/relay.proto +++ b/shared/core/types/proto/relay.proto @@ -31,21 +31,26 @@ enum RESTRequestType { RESTRequestTypeDELETE = 3; } +message JSONRPCId { + bytes id = 1; +} + message JSONRPCPayload { // JSONRPC version 2 expected a field named "id". // See the JSONRPC spec in the following link for more details: // https://www.jsonrpc.org/specification#request_object - bytes id = 1; + JSONRPCId id = 1; // JSONRPC version 2 expects a field named "jsonrpc" with a value of "2.0". // See the JSONRPC spec in the following link for more details: // https://www.jsonrpc.org/specification#request_object - string json_rpc = 2; + string jsonrpc = 2; string method = 3; // The parameters field can be empty, an array or a structure. It is on the server to decide which one // has been sent to it and whether the supplied value is valid. // See the JSONRPC spec in the following link for more details: // https://www.jsonrpc.org/specification#parameter_structures - bytes parameters = 4; + // INCOMPLETE: decide the type for params field, considering the above description and interaction with OpenAPI + repeated string params = 4; map headers = 5; } diff --git a/shared/core/types/relay.go b/shared/core/types/relay.go index 5d9f5b65f..c9d7d7273 100644 --- a/shared/core/types/relay.go +++ b/shared/core/types/relay.go @@ -34,8 +34,8 @@ func (r *Relay) Validate() error { // 1. The JSONRPC field is set to "2.0" as per the JSONRPC spec requirement, and // 2. The Method field is not empty func (p *JSONRPCPayload) Validate() error { - if p.JsonRpc != jsonRpcVersion { - return fmt.Errorf("%w: %s", errInvalidJSONRPC, p.JsonRpc) + if p.Jsonrpc != jsonRpcVersion { + return fmt.Errorf("%w: %s", errInvalidJSONRPC, p.Jsonrpc) } // DISCUSS: do we need/want chain-specific validation? Potential for reusing the existing logic of Portal V2/pocket-go @@ -54,3 +54,10 @@ func (p *RESTPayload) Validate() error { } return nil } + +// MarshalJSON is a custom marshaller for JSONRPCId type to return the byte array as-is. +// +// This is to ensure the specified ID gets sent correctly when serializing the relay that contains the ID. +func (i *JSONRPCId) MarshalJSON() ([]byte, error) { + return i.Id, nil +} diff --git a/shared/core/types/relay_test.go b/shared/core/types/relay_test.go index db34d8dec..a0c1bb47c 100644 --- a/shared/core/types/relay_test.go +++ b/shared/core/types/relay_test.go @@ -1,6 +1,7 @@ package types import ( + "encoding/json" "testing" "github.com/stretchr/testify/require" @@ -16,7 +17,7 @@ func TestRelay_Validate(t *testing.T) { name: "valid Relay: JSONRPC", relay: &Relay{ RelayPayload: &Relay_JsonRpcPayload{ - JsonRpcPayload: &JSONRPCPayload{JsonRpc: "2.0", Method: "eth_blockNumber"}, + JsonRpcPayload: &JSONRPCPayload{Jsonrpc: "2.0", Method: "eth_blockNumber"}, }, }, }, @@ -37,7 +38,7 @@ func TestRelay_Validate(t *testing.T) { name: "invalid Relay: invalid JSONRPC Payload", relay: &Relay{ RelayPayload: &Relay_JsonRpcPayload{ - JsonRpcPayload: &JSONRPCPayload{JsonRpc: "foo"}, + JsonRpcPayload: &JSONRPCPayload{Jsonrpc: "foo"}, }, }, expected: errInvalidJSONRPC, @@ -69,16 +70,16 @@ func TestRelay_ValidateJsonRpc(t *testing.T) { }{ { name: "valid JSONRPC", - payload: &JSONRPCPayload{JsonRpc: "2.0", Method: "eth_blockNumber"}, + payload: &JSONRPCPayload{Jsonrpc: "2.0", Method: "eth_blockNumber"}, }, { - name: "invalid JSONRPC: invalid JsonRpc field value", - payload: &JSONRPCPayload{JsonRpc: "foo", Method: "eth_blockNumber"}, + name: "invalid JSONRPC: invalid Jsonrpc field value", + payload: &JSONRPCPayload{Jsonrpc: "foo", Method: "eth_blockNumber"}, expected: errInvalidJSONRPC, }, { name: "invalid JSONRPC: Method field not set", - payload: &JSONRPCPayload{JsonRpc: "2.0"}, + payload: &JSONRPCPayload{Jsonrpc: "2.0"}, expected: errInvalidJSONRPCMissingMethod, }, } @@ -115,3 +116,16 @@ func TestRelay_ValidateREST(t *testing.T) { }) } } + +func TestRelay_MarshalJSONRPC(t *testing.T) { + payload := JSONRPCPayload{ + Id: &JSONRPCId{Id: []byte(`"1"`)}, + Jsonrpc: "2.0", + Method: "eth_blockNumber", + } + expected := []byte(`{"id":"1","jsonrpc":"2.0","method":"eth_blockNumber"}`) + + bz, err := json.Marshal(payload) + require.NoError(t, err) + require.Equal(t, bz, expected) +} diff --git a/utility/servicer/module.go b/utility/servicer/module.go index dcd4c48f1..621ea7dd2 100644 --- a/utility/servicer/module.go +++ b/utility/servicer/module.go @@ -2,7 +2,9 @@ package servicer import ( "bytes" + "crypto/tls" "encoding/hex" + "encoding/json" "errors" "fmt" "io" @@ -228,8 +230,15 @@ func (s *servicer) validateRelayChainSupport(relayChain *coreTypes.Identifiable, } defer readCtx.Release() //nolint:errcheck // We only need to make sure the readCtx is released - // DISCUSS: should we update the GetServicer signature to take a string instead? - servicer, err := readCtx.GetServicer([]byte(s.config.Address), currentHeight) + + // The servicer address needs to be passed to persistence module, which uses hex.EncodeToString to convert the byte array to string. + // Therefore, the address needs to be decoded as a byte array before passing it to the persistence module. + servicerAddrBz, err := hex.DecodeString(s.config.Address) + if err != nil { + return fmt.Errorf("error decoding servicer address %s: %w", s.config.Address, err) + } + + servicer, err := readCtx.GetServicer(servicerAddrBz, currentHeight) if err != nil { return fmt.Errorf("error reading servicer from persistence: %w", err) } @@ -424,7 +433,8 @@ func (s *servicer) executeJsonRPCRelay(meta *coreTypes.RelayMeta, payload *coreT return nil, fmt.Errorf("Chain %s not found in servicer configuration: %w", meta.RelayChain.Id, errValidateRelayMeta) } - relayBytes, err := codec.GetCodec().Marshal(payload) + // JSONRPC endpoints expect json-encoded payload, so codec package would not work here as it uses proto serialization + relayBytes, err := json.Marshal(payload) if err != nil { return nil, fmt.Errorf("Error marshalling payload %s: %w", payload.String(), err) } @@ -466,7 +476,9 @@ func (s *servicer) executeHTTPRelay(serviceConfig *configs.ServiceConfig, payloa } // INCOMPLETE(#837): Optimize usage of HTTP client, e.g. connection reuse, depending on the volume of relays a servicer is expected to handle - resp, err := (&http.Client{Timeout: time.Duration(serviceConfig.TimeoutMsec) * time.Millisecond}).Do(req) + // INCOMPLETE(#887): allow configuration of TLS Settings for HTTPS services + tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} + resp, err := (&http.Client{Transport: tr, Timeout: time.Duration(serviceConfig.TimeoutMsec) * time.Millisecond}).Do(req) if err != nil { return nil, fmt.Errorf("Error performing the HTTP request for relay: %w", err) }