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

[Utility] trustless relay e2e happy case (POC PR) #869

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,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
Expand Down Expand Up @@ -466,7 +471,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. <KEYWORD>: <Description of follow up work>;
Expand Down
15 changes: 8 additions & 7 deletions app/client/cli/servicer.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,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)
}
Expand Down Expand Up @@ -183,13 +183,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{
Expand All @@ -200,6 +200,7 @@ func buildRelay(payload string, appPrivateKey crypto.PrivateKey, session *rpc.Se
},
ServicerPubKey: servicer.PublicKey,
// TODO(#697): Geozone
ApplicationAddress: appAddr,
}

relay := &rpc.RelayRequest{
Expand Down
3 changes: 2 additions & 1 deletion build/localnet/manifests/configs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,8 @@ data:
"address": "001022b138896c4c5466ac86b24a9bbe249905c2",
"public_key": "56915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495",
"chains": ["0001"],
"service_url": "servicer-001-pocket:42069",
# ADD_IN_THIS_PR: update all RPC ports and add the protocol, i.e. HTTPS
"service_url": "http://servicer-001-pocket:50832",
Copy link
Member

Choose a reason for hiding this comment

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

why did we update the port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Port 50832 is the configured RPC port, so 42069 seems incorrect (encountered this while running the E2E test introduced in this PR)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should update all the RPC ports then to avoid a discrepancy:

Screenshot 2023-07-05 at 4 07 04 PM

If you want to do it in a small separate PR (while this one is being reviewed), I support that!

Copy link
Member

Choose a reason for hiding this comment

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

Is the intention to do it in this PR (given the comment)?

I still think a different small "micro PR" could be simpler & faster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think a different small "micro PR" could be simpler & faster

Yes, I will open a separate, small PR for this shortly.

"staked_amount": "1000000000000",
"paused_height": -1,
"unstaking_height": -1,
Expand Down
14 changes: 14 additions & 0 deletions charts/pocket/pocket-servicer-overrides.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# - This is an override of the shared config: <https://github.com/pokt-network/pocket/blob/40f325305c1756bbfd069bf139fa67545419981c/charts/pocket/values.yaml#L116C12-L117>
# - 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:
adshmh marked this conversation as resolved.
Show resolved Hide resolved
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"
adshmh marked this conversation as resolved.
Show resolved Hide resolved
services:
"0001":
url: "https://eth-mainnet.gateway.pokt.network"
adshmh marked this conversation as resolved.
Show resolved Hide resolved
142 changes: 141 additions & 1 deletion e2e/tests/steps_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,15 @@ const (
// validatorB maps to suffix ID 002 and receives in the Send test.
validatorB = "002"
chainId = "0001"

servicerA = "001"
appA = "000"
serviceA = "0001"

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 {
adshmh marked this conversation as resolved.
Show resolved Hide resolved
gocuke.TestingT

Expand All @@ -45,6 +52,27 @@ type rootSuite struct {
// validator holds command results between runs and reports errors to the test suite
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() {
Expand All @@ -59,12 +87,43 @@ func (s *rootSuite) Before() {
s.validator = new(validatorPod)
s.clientset = clientSet
s.validatorKeys = vkmap

// ADD_IN_THIS_PR: use pocketk8s to populate
s.servicerKeys = map[string]string{
// 000 servicer NOT in session
adshmh marked this conversation as resolved.
Show resolved Hide resolved
// The list of servicers in the session is decided by the 'servicers' section of the genesis, from 'build/localnet/manifest/configs.yaml' file
"000": "acbca21f295caefdfe480ceba85f3fed31a50915162f94867f9c23d8f474f4c6d1130c5eb920af8edd5b6bfa39d33aa787f421c8ba0786de4ca4e7703553bb97",
// 001 servicer in session
"001": "eec4072b095acf60be9d6be4093b14a24e2ddb6e9d385d980a635815961d025856915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495",
}

s.appKeys = map[string]string{
"000": "468cc03083d72f2440d3d08d12143b9b74cca9460690becaa2499a4f04fddaa805a25e527bf6f51676f61f2f1a96efaa748218ac82f54d3cdc55a4881389eb60",
}

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()
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
Expand Down Expand Up @@ -158,6 +217,87 @@ func (s *rootSuite) getPrivateKey(
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)
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
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.servicer = 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
// ADD_IN_THIS_PR: Verify the response: correct id and correct jsonrpc
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.servicer)
appPrivateKey := s.getAppPrivateKey(appA)

s.sendTrustlessRelay(checkBalanceRelay, servicerPrivateKey.Address().String(), appPrivateKey.Address().String())
}

func (s *rootSuite) TheRelayResponseContains(relayResponse string) {
require.Contains(s, s.validator.result.Stdout, relayResponse)
}

func (s *rootSuite) sendTrustlessRelay(relayPayload string, servicerAddr, appAddr string) {
args := []string{
"Servicer",
"Relay",
appAddr,
servicerAddr,
// IMPROVE: add ETH_Goerli as a chain/service to genesis
serviceA,
relayPayload,
}

// TECHDEBT: run the command from a client, i.e. not a validator, pod.
res, err := s.validator.RunCommand(args...)

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) {
Expand Down
18 changes: 18 additions & 0 deletions e2e/tests/trustless_relays.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Feature: Trustless Relays
Olshansk marked this conversation as resolved.
Show resolved Hide resolved
# 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
adshmh marked this conversation as resolved.
Show resolved Hide resolved
# 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
# TECHDEBT: replace validator with client
And the validator should have exited without error
adshmh marked this conversation as resolved.
Show resolved Hide resolved

# 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.

# TODO: add an E2E test for a trustless relay, where the application retrieves the session first, using a new fetch session command
18 changes: 10 additions & 8 deletions rpc/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -220,7 +221,7 @@ 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,
},
}
Expand All @@ -229,8 +230,9 @@ func buildJsonRPCRelayPayload(body *RelayRequest) *coreTypes.Relay {
payload.JsonRpcPayload.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 {
Expand Down
12 changes: 8 additions & 4 deletions rpc/v1/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1699,16 +1699,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:
Expand Down Expand Up @@ -1762,6 +1763,7 @@ components:
- geozone
- token
- signature
- application_address
properties:
block_height:
type: integer
Expand All @@ -1776,6 +1778,8 @@ components:
$ref: "#/components/schemas/AAT"
signature:
type: string
application_address:
type: string
Signature:
type: object
required:
Expand Down
5 changes: 3 additions & 2 deletions shared/core/types/proto/relay.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@ message JSONRPCPayload {
// 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<string, string> headers = 5;
}

Expand Down
4 changes: 2 additions & 2 deletions shared/core/types/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions shared/k8s/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ func FetchValidatorPrivateKeys(clientset *kubernetes.Clientset) (map[string]stri
return validatorKeysMap, nil
}

// ADD_IN_THIS_PR: add the following functions in a separate PR: FetchServicerPrivateKeys and FetchAppPrivateKeys

func getNamespace() (string, error) {
_, err := os.Stat(kubernetesServiceAccountNamespaceFile)
if err == nil {
Expand Down
Loading