From 0a005b0d0637a607549a7f25a47bd00e39a56c9c Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Wed, 8 Nov 2023 11:53:18 -0800 Subject: [PATCH] [Session] Add support for `GetSession` via the CLI + deps (#123) - Update the CLI code flow to be able to call `GetSession` - Add make targets to trigger `GetSession` - Replace `[]` with `<>` in all CLI functions to differentiate required vs optional params - Made a couple changes to `SessionHydrator` (and added tests) to account additional error cases https://github.com/pokt-network/poktroll/assets/1892194/56425906-d06f-4c41-b8a5-d210d8a450cd --- Co-authored-by: Bryan White Co-authored-by: Daniel Olshansky Co-authored-by: Dima Kniazev --- Makefile | 59 +++++- app/app.go | 22 +- go.mod | 1 + go.sum | 5 + proto/pocket/shared/service.proto | 4 +- testutil/keeper/session.go | 35 +++- testutil/network/network.go | 3 + x/application/client/cli/query_application.go | 2 +- .../client/cli/tx_delegate_to_gateway.go | 2 +- .../client/cli/tx_stake_application.go | 2 +- .../client/cli/tx_unstake_application.go | 2 +- x/gateway/client/cli/helpers_test.go | 14 +- x/gateway/client/cli/query_gateway.go | 2 +- x/gateway/client/cli/tx_stake_gateway.go | 6 +- x/gateway/client/cli/tx_unstake_gateway.go | 2 +- x/session/client/cli/helpers_test.go | 44 ++++ x/session/client/cli/query_get_session.go | 36 +++- .../client/cli/query_get_session_test.go | 197 ++++++++++++++++++ x/session/keeper/query_get_session.go | 15 ++ x/session/keeper/query_get_session_test.go | 55 ++++- x/session/keeper/session_hydrator.go | 35 +++- x/session/keeper/session_hydrator_test.go | 130 ++++++++---- x/session/types/errors.go | 10 +- x/session/types/query_get_session_request.go | 40 ++++ x/shared/helpers/service.go | 7 +- x/shared/helpers/service_test.go | 107 +++++++--- x/supplier/client/cli/query_supplier.go | 2 +- x/supplier/client/cli/tx_create_claim.go | 5 +- x/supplier/client/cli/tx_stake_supplier.go | 2 +- x/supplier/client/cli/tx_submit_proof.go | 9 +- 30 files changed, 709 insertions(+), 146 deletions(-) create mode 100644 x/session/client/cli/helpers_test.go create mode 100644 x/session/client/cli/query_get_session_test.go create mode 100644 x/session/types/query_get_session_request.go diff --git a/Makefile b/Makefile index de0c67cd2..1860b6156 100644 --- a/Makefile +++ b/Makefile @@ -312,15 +312,18 @@ app_delegate: ## Delegate trust to a gateway (must specify the APP and GATEWAY_A .PHONY: app1_delegate_gateway1 app1_delegate_gateway1: ## Delegate trust to gateway1 - APP=app1 GATEWAY_ADDR=pokt15vzxjqklzjtlz7lahe8z2dfe9nm5vxwwmscne4 make app_delegate + GATEWAY1=$$(make poktrolld_addr ACC_NAME=gateway1) && \ + APP=app1 GATEWAY_ADDR=$$GATEWAY1 make app_delegate .PHONY: app2_delegate_gateway2 app2_delegate_gateway2: ## Delegate trust to gateway2 - APP=app2 GATEWAY_ADDR=pokt15w3fhfyc0lttv7r585e2ncpf6t2kl9uh8rsnyz make app_delegate + GATEWAY2=$$(make poktrolld_addr ACC_NAME=gateway2) && \ + APP=app2 GATEWAY_ADDR=$$GATEWAY2 make app_delegate .PHONY: app3_delegate_gateway3 app3_delegate_gateway3: ## Delegate trust to gateway3 - APP=app3 GATEWAY_ADDR=pokt1zhmkkd0rh788mc9prfq0m2h88t9ge0j83gnxya make app_delegate + GATEWAY3=$$(make poktrolld_addr ACC_NAME=gateway3) && \ + APP=app3 GATEWAY_ADDR=$$GATEWAY3 make app_delegate .PHONY: app_undelegate app_undelegate: ## Undelegate trust to a gateway (must specify the APP and GATEWAY_ADDR env vars). Requires the app to be staked @@ -328,15 +331,18 @@ app_undelegate: ## Undelegate trust to a gateway (must specify the APP and GATEW .PHONY: app1_undelegate_gateway1 app1_undelegate_gateway1: ## Undelegate trust to gateway1 - APP=app1 GATEWAY_ADDR=pokt15vzxjqklzjtlz7lahe8z2dfe9nm5vxwwmscne4 make app_undelegate + GATEWAY1=$$(make poktrolld_addr ACC_NAME=gateway1) && \ + APP=app1 GATEWAY_ADDR=$$GATEWAY1 make app_undelegate .PHONY: app2_undelegate_gateway2 app2_undelegate_gateway2: ## Undelegate trust to gateway2 - APP=app2 GATEWAY_ADDR=pokt15w3fhfyc0lttv7r585e2ncpf6t2kl9uh8rsnyz make app_undelegate + GATEWAY2=$$(make poktrolld_addr ACC_NAME=gateway2) && \ + APP=app2 GATEWAY_ADDR=$$GATEWAY2 make app_undelegate .PHONY: app3_undelegate_gateway3 app3_undelegate_gateway3: ## Undelegate trust to gateway3 - APP=app3 GATEWAY_ADDR=pokt1zhmkkd0rh788mc9prfq0m2h88t9ge0j83gnxya make app_undelegate + GATEWAY3=$$(make poktrolld_addr ACC_NAME=gateway3) && \ + APP=app3 GATEWAY_ADDR=$$GATEWAY3 make app_undelegate ################# ### Suppliers ### @@ -380,6 +386,29 @@ supplier2_unstake: ## Unstake supplier2 supplier3_unstake: ## Unstake supplier3 SUPPLIER=supplier3 make supplier_unstake +############### +### Session ### +############### + +.PHONY: get_session +get_session: ## Retrieve the session given the following env vars: (APP_ADDR, SVC, HEIGHT) + pocketd --home=$(POCKETD_HOME) q session get-session $(APP) $(SVC) $(HEIGHT) --node $(POCKET_NODE) + +.PHONY: get_session_app1_anvil +get_session_app1_anvil: ## Retrieve the session for (app1, anvil, latest_height) + APP1=$$(make poktrolld_addr ACC_NAME=app1) && \ + APP=$$APP1 SVC=anvil HEIGHT=0 make get_session + +.PHONY: get_session_app2_anvil +get_session_app2_anvil: ## Retrieve the session for (app2, anvil, latest_height) + APP2=$$(make poktrolld_addr ACC_NAME=app2) && \ + APP=$$APP2 SVC=anvil HEIGHT=0 make get_session + +.PHONY: get_session_app3_anvil +get_session_app3_anvil: ## Retrieve the session for (app3, anvil, latest_height) + APP3=$$(make poktrolld_addr ACC_NAME=app3) && \ + APP=$$APP3 SVC=anvil HEIGHT=0 make get_session + ################ ### Accounts ### ################ @@ -398,11 +427,13 @@ acc_balance_query_module_app: ## Query the balance of the network level "applica .PHONY: acc_balance_query_module_supplier acc_balance_query_module_supplier: ## Query the balance of the network level "supplier" module - make acc_balance_query ACC=pokt1j40dzzmn6cn9kxku7a5tjnud6hv37vesr5ccaa + SUPPLIER1=$(make poktrolld_addr ACC_NAME=supplier1) + make acc_balance_query ACC=SUPPLIER1 .PHONY: acc_balance_query_app1 acc_balance_query_app1: ## Query the balance of app1 - make acc_balance_query ACC=pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4 + APP1=$$(make poktrolld_addr ACC_NAME=app1) && \ + make acc_balance_query ACC=$$APP1 .PHONY: acc_balance_total_supply acc_balance_total_supply: ## Query the total supply of the network @@ -428,8 +459,20 @@ trigger_ci: ## Trigger the CI pipeline by submitting an empty commit; See https: ##################### ### Documentation ### ##################### + .PHONY: go_docs go_docs: check_godoc ## Generate documentation for the project echo "Visit http://localhost:6060/pkg/pocket/" godoc -http=:6060 +.PHONY: openapi_gen +openapi_gen: ## Generate the OpenAPI spec for the Ignite API + ignite generate openapi --yes + +###################### +### Ignite Helpers ### +###################### + +.PHONY: poktrolld_addr +poktrolld_addr: ## Retrieve the address for an account by ACC_NAME + @echo $(shell poktrolld keys show -a $(ACC_NAME)) diff --git a/app/app.go b/app/app.go index c6a48ee69..b34d4db68 100644 --- a/app/app.go +++ b/app/app.go @@ -575,17 +575,6 @@ func New( ) serviceModule := servicemodule.NewAppModule(appCodec, app.ServiceKeeper, app.AccountKeeper, app.BankKeeper) - app.SessionKeeper = *sessionmodulekeeper.NewKeeper( - appCodec, - keys[sessionmoduletypes.StoreKey], - keys[sessionmoduletypes.MemStoreKey], - app.GetSubspace(sessionmoduletypes.ModuleName), - - app.ApplicationKeeper, - app.SupplierKeeper, - ) - sessionModule := sessionmodule.NewAppModule(appCodec, app.SessionKeeper, app.AccountKeeper, app.BankKeeper) - app.SupplierKeeper = *suppliermodulekeeper.NewKeeper( appCodec, keys[suppliermoduletypes.StoreKey], @@ -618,6 +607,17 @@ func New( ) applicationModule := applicationmodule.NewAppModule(appCodec, app.ApplicationKeeper, app.AccountKeeper, app.BankKeeper) + app.SessionKeeper = *sessionmodulekeeper.NewKeeper( + appCodec, + keys[sessionmoduletypes.StoreKey], + keys[sessionmoduletypes.MemStoreKey], + app.GetSubspace(sessionmoduletypes.ModuleName), + + app.ApplicationKeeper, + app.SupplierKeeper, + ) + sessionModule := sessionmodule.NewAppModule(appCodec, app.SessionKeeper, app.AccountKeeper, app.BankKeeper) + // this line is used by starport scaffolding # stargate/app/keeperDefinition /**** IBC Routing ****/ diff --git a/go.mod b/go.mod index b4658dfb1..51aa32938 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/cosmos/cosmos-sdk v0.47.3 github.com/cosmos/gogoproto v1.4.10 github.com/cosmos/ibc-go/v7 v7.1.0 + github.com/gogo/status v1.1.1 github.com/golang/mock v1.6.0 github.com/golang/protobuf v1.5.3 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index 64f7566f0..b554afc3b 100644 --- a/go.sum +++ b/go.sum @@ -688,10 +688,13 @@ github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRx github.com/gofrs/uuid v4.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= github.com/gofrs/uuid v4.3.1+incompatible h1:0/KbAdpx3UXAx1kEOWHJeOkpbgRFGHVgv+CFIY7dBJI= github.com/gofrs/uuid v4.3.1+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= +github.com/gogo/googleapis v0.0.0-20180223154316-0cd9801be74a/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFGgqEef3s= github.com/gogo/googleapis v1.1.0/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFGgqEef3s= github.com/gogo/googleapis v1.4.1-0.20201022092350-68b0159b7869/go.mod h1:5YRNX2z1oM5gXdAkurHa942MDgEJyk02w4OecKY87+c= github.com/gogo/googleapis v1.4.1 h1:1Yx4Myt7BxzvUr5ldGSbwYiZG6t9wGBZ+8/fX3Wvtq0= github.com/gogo/googleapis v1.4.1/go.mod h1:2lpHqI5OcWCtVElxXnPt+s8oJvMpySlOyM6xDCrzib4= +github.com/gogo/status v1.1.1 h1:DuHXlSFHNKqTQ+/ACf5Vs6r4X/dH2EgIzR9Vr+H65kg= +github.com/gogo/status v1.1.1/go.mod h1:jpG3dM5QPcqu19Hg8lkUhBFBa3TcLs1DG7+2Jqci7oU= github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang-sql/civil v0.0.0-20190719163853-cb61b32ac6fe/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0= github.com/golang-sql/sqlexp v0.0.0-20170517235910-f1bb20e5a188/go.mod h1:vXjM/+wXQnTPR4KqTKDgJukSZ6amVRtWMPEjE6sQoK8= @@ -2610,6 +2613,7 @@ google.golang.org/appengine v1.6.6/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCID google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/genproto v0.0.0-20170818010345-ee236bd376b0/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= +google.golang.org/genproto v0.0.0-20180518175338-11a468237815/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20180831171423-11092d34479b/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20181029155118-b69ba1387ce2/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= @@ -2735,6 +2739,7 @@ google.golang.org/genproto v0.0.0-20221025140454-527a21cfbd71/go.mod h1:9qHF0xnp google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 h1:KpwkzHKEF7B9Zxg18WzOa7djJ+Ha5DzthMyZYQfEn2A= google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1/go.mod h1:nKE/iIaLqn2bQwXBg8f1g2Ylh6r5MN5CmZvuzZCgsCU= google.golang.org/grpc v1.8.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw= +google.golang.org/grpc v1.12.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw= google.golang.org/grpc v1.14.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw= google.golang.org/grpc v1.16.0/go.mod h1:0JHn/cJsOMiMfNA9+DeHDlAU7KAAB5GDlYFpa9MZMio= google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs= diff --git a/proto/pocket/shared/service.proto b/proto/pocket/shared/service.proto index 8d6927515..4700035ce 100644 --- a/proto/pocket/shared/service.proto +++ b/proto/pocket/shared/service.proto @@ -19,7 +19,7 @@ message Service { // ApplicationServiceConfig holds the service configuration the application stakes for message ApplicationServiceConfig { - Service service = 1; // The Service for which the application is configured for + Service service = 1; // The Service for which the application is configured // TODO_RESEARCH: There is an opportunity for applications to advertise the max // they're willing to pay for a certain configuration/price, but this is outside of scope. @@ -28,7 +28,7 @@ message ApplicationServiceConfig { // SupplierServiceConfig holds the service configuration the supplier stakes for message SupplierServiceConfig { - Service service = 1; // The Service for which the supplier is configured for + Service service = 1; // The Service for which the supplier is configured repeated SupplierEndpoint endpoints = 2; // List of endpoints for the service // TODO_RESEARCH: There is an opportunity for supplier to advertise the min // they're willing to earn for a certain configuration/price, but this is outside of scope. diff --git a/testutil/keeper/session.go b/testutil/keeper/session.go index ab0eb277c..8b57ec0c4 100644 --- a/testutil/keeper/session.go +++ b/testutil/keeper/session.go @@ -27,10 +27,15 @@ import ( type option[V any] func(k *keeper.Keeper) var ( - TestServiceId1 = "svc1" - TestServiceId2 = "svc2" + TestServiceId1 = "svc1" // staked for by app1 & supplier1 + TestServiceId11 = "svc11" // staked for by app1 - TestApp1Address = "pokt106grzmkmep67pdfrm6ccl9snynryjqus6l3vct" // Generated via sample.AccAddress() + TestServiceId2 = "svc2" // staked for by app2 & supplier1 + TestServiceId22 = "svc22" // staked for by app2 + + TestServiceId12 = "svc12" // staked for by app1, app2 & supplier1 + + TestApp1Address = "pokt1mdccn4u38eyjdxkk4h0jaddw4n3c72u82m5m9e" // Generated via sample.AccAddress() TestApp1 = apptypes.Application{ Address: TestApp1Address, Stake: &sdk.Coin{Denom: "upokt", Amount: sdk.NewInt(100)}, @@ -39,21 +44,27 @@ var ( Service: &sharedtypes.Service{Id: TestServiceId1}, }, { - Service: &sharedtypes.Service{Id: TestServiceId2}, + Service: &sharedtypes.Service{Id: TestServiceId11}, + }, + { + Service: &sharedtypes.Service{Id: TestServiceId12}, }, }, } - TestApp2Address = "pokt1dm7tr0a99ja232gzt5rjtrl7hj6z6h40669fwh" // Generated via sample.AccAddress() + TestApp2Address = "pokt133amv5suh75zwkxxcq896azvmmwszg99grvk9f" // Generated via sample.AccAddress() TestApp2 = apptypes.Application{ Address: TestApp1Address, Stake: &sdk.Coin{Denom: "upokt", Amount: sdk.NewInt(100)}, ServiceConfigs: []*sharedtypes.ApplicationServiceConfig{ { - Service: &sharedtypes.Service{Id: TestServiceId1}, + Service: &sharedtypes.Service{Id: TestServiceId2}, }, { - Service: &sharedtypes.Service{Id: TestServiceId2}, + Service: &sharedtypes.Service{Id: TestServiceId22}, + }, + { + Service: &sharedtypes.Service{Id: TestServiceId12}, }, }, } @@ -84,6 +95,16 @@ var ( }, }, }, + { + Service: &sharedtypes.Service{Id: TestServiceId12}, + Endpoints: []*sharedtypes.SupplierEndpoint{ + { + Url: TestSupplierUrl, + RpcType: sharedtypes.RPCType_GRPC, + Configs: make([]*sharedtypes.ConfigOption, 0), + }, + }, + }, }, } ) diff --git a/testutil/network/network.go b/testutil/network/network.go index c39983ca6..28f4eaa2f 100644 --- a/testutil/network/network.go +++ b/testutil/network/network.go @@ -113,6 +113,9 @@ func DefaultApplicationModuleGenesisState(t *testing.T, n int) *apptypes.Genesis { Service: &sharedtypes.Service{Id: fmt.Sprintf("svc%d", i)}, }, + { + Service: &sharedtypes.Service{Id: fmt.Sprintf("svc%d%d", i, i)}, + }, }, } // TODO_CONSIDERATION: Evaluate whether we need `nullify.Fill` or if we should enforce `(gogoproto.nullable) = false` everywhere diff --git a/x/application/client/cli/query_application.go b/x/application/client/cli/query_application.go index 61a5eb35b..4fbf32228 100644 --- a/x/application/client/cli/query_application.go +++ b/x/application/client/cli/query_application.go @@ -46,7 +46,7 @@ func CmdListApplication() *cobra.Command { func CmdShowApplication() *cobra.Command { cmd := &cobra.Command{ - Use: "show-application [address]", + Use: "show-application ", Short: "shows a application", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) (err error) { diff --git a/x/application/client/cli/tx_delegate_to_gateway.go b/x/application/client/cli/tx_delegate_to_gateway.go index b27562215..f1363e6db 100644 --- a/x/application/client/cli/tx_delegate_to_gateway.go +++ b/x/application/client/cli/tx_delegate_to_gateway.go @@ -15,7 +15,7 @@ var _ = strconv.Itoa(0) func CmdDelegateToGateway() *cobra.Command { cmd := &cobra.Command{ - Use: "delegate-to-gateway [gateway address]", + Use: "delegate-to-gateway ", Short: "Delegate an application to a gateway", Long: `Delegate an application to the gateway with the provided address. This is a broadcast operation that delegates authority to the gateway specified to sign relays requests for the application, allowing the gateway diff --git a/x/application/client/cli/tx_stake_application.go b/x/application/client/cli/tx_stake_application.go index 86ba62893..2909eaf8e 100644 --- a/x/application/client/cli/tx_stake_application.go +++ b/x/application/client/cli/tx_stake_application.go @@ -21,7 +21,7 @@ func CmdStakeApplication() *cobra.Command { // TODO_HACK: For now we are only specifying the service IDs as a list of of strings separated by commas. // This needs to be expand to specify the full ApplicationServiceConfig. Furthermore, providing a flag to // a file where ApplicationServiceConfig specifying full service configurations in the CLI by providing a flag that accepts a JSON string - Use: "stake-application [amount] [svcId1,svcId2,...,svcIdN]", + Use: "stake-application ", Short: "Stake an application", Long: `Stake an application with the provided parameters. This is a broadcast operation that will stake the tokens and serviceIds and associate them with the application specified by the 'from' address. diff --git a/x/application/client/cli/tx_unstake_application.go b/x/application/client/cli/tx_unstake_application.go index 81011d0e0..c602def16 100644 --- a/x/application/client/cli/tx_unstake_application.go +++ b/x/application/client/cli/tx_unstake_application.go @@ -16,7 +16,7 @@ var _ = strconv.Itoa(0) func CmdUnstakeApplication() *cobra.Command { // fromAddress & signature is retrieved via `flags.FlagFrom` in the `clientCtx` cmd := &cobra.Command{ - Use: "unstake-application [amount]", + Use: "unstake-application", Short: "Unstake an application", Long: `Unstake an application. This is a broadcast operation that will unstake the application specified by the 'from' address. diff --git a/x/gateway/client/cli/helpers_test.go b/x/gateway/client/cli/helpers_test.go index 927212a46..cf2ccf899 100644 --- a/x/gateway/client/cli/helpers_test.go +++ b/x/gateway/client/cli/helpers_test.go @@ -1,14 +1,24 @@ package cli_test import ( + "strconv" "testing" + "github.com/stretchr/testify/require" + + "github.com/pokt-network/poktroll/cmd/pocketd/cmd" "github.com/pokt-network/poktroll/testutil/network" "github.com/pokt-network/poktroll/x/gateway/types" - - "github.com/stretchr/testify/require" ) +// Dummy variable to avoid unused import error. +var _ = strconv.IntSize + +// init initializes the SDK configuration. +func init() { + cmd.InitSDKConfig() +} + // networkWithGatewayObjects creates a network with a populated gateway state of n gateway objects func networkWithGatewayObjects(t *testing.T, n int) (*network.Network, []types.Gateway) { t.Helper() diff --git a/x/gateway/client/cli/query_gateway.go b/x/gateway/client/cli/query_gateway.go index 30076ed22..124c515c8 100644 --- a/x/gateway/client/cli/query_gateway.go +++ b/x/gateway/client/cli/query_gateway.go @@ -46,7 +46,7 @@ func CmdListGateway() *cobra.Command { func CmdShowGateway() *cobra.Command { cmd := &cobra.Command{ - Use: "show-gateway [address]", + Use: "show-gateway ", Short: "shows a gateway", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) (err error) { diff --git a/x/gateway/client/cli/tx_stake_gateway.go b/x/gateway/client/cli/tx_stake_gateway.go index 97e93c812..3c4d29a30 100644 --- a/x/gateway/client/cli/tx_stake_gateway.go +++ b/x/gateway/client/cli/tx_stake_gateway.go @@ -3,20 +3,20 @@ package cli import ( "strconv" - "github.com/pokt-network/poktroll/x/gateway/types" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/tx" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/spf13/cobra" + + "github.com/pokt-network/poktroll/x/gateway/types" ) var _ = strconv.Itoa(0) func CmdStakeGateway() *cobra.Command { cmd := &cobra.Command{ - Use: "stake-gateway [amount]", + Use: "stake-gateway ", Short: "Stake a gateway", Long: `Stake a gateway with the provided parameters. This is a broadcast operation that will stake the tokens and associate them with the gateway specified by the 'from' address. diff --git a/x/gateway/client/cli/tx_unstake_gateway.go b/x/gateway/client/cli/tx_unstake_gateway.go index bd2d38ebe..e97406495 100644 --- a/x/gateway/client/cli/tx_unstake_gateway.go +++ b/x/gateway/client/cli/tx_unstake_gateway.go @@ -16,7 +16,7 @@ var _ = strconv.Itoa(0) func CmdUnstakeGateway() *cobra.Command { // fromAddress & signature is retrieved via `flags.FlagFrom` in the `clientCtx` cmd := &cobra.Command{ - Use: "unstake-gateway [amount]", + Use: "unstake-gateway ", Short: "Unstake a gateway", Long: `Unstake a gateway. This is a broadcast operation that will unstake the gateway specified by the 'from' address. diff --git a/x/session/client/cli/helpers_test.go b/x/session/client/cli/helpers_test.go new file mode 100644 index 000000000..484bca4db --- /dev/null +++ b/x/session/client/cli/helpers_test.go @@ -0,0 +1,44 @@ +// Package cli_test provides unit tests for the CLI functionality. +package cli_test + +import ( + "strconv" + "testing" + + "github.com/pokt-network/poktroll/cmd/pocketd/cmd" + "github.com/pokt-network/poktroll/testutil/network" + apptypes "github.com/pokt-network/poktroll/x/application/types" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" + suppliertypes "github.com/pokt-network/poktroll/x/supplier/types" + "github.com/stretchr/testify/require" +) + +// Dummy variable to avoid unused import error. +var _ = strconv.IntSize + +// init initializes the SDK configuration. +func init() { + cmd.InitSDKConfig() +} + +// networkWithApplicationsAndSupplier creates a new network with a given number of supplier & application objects. +// It returns the network and a slice of the created supplier & application objects. +func networkWithApplicationsAndSupplier(t *testing.T, n int) (*network.Network, []sharedtypes.Supplier, []apptypes.Application) { + t.Helper() + cfg := network.DefaultConfig() + + // Prepare the application genesis state + applicationGenesisState := network.DefaultApplicationModuleGenesisState(t, n) + buf, err := cfg.Codec.MarshalJSON(applicationGenesisState) + require.NoError(t, err) + cfg.GenesisState[apptypes.ModuleName] = buf + + // Prepare the supplier genesis state + supplierGenesisState := network.DefaultSupplierModuleGenesisState(t, n) + buf, err = cfg.Codec.MarshalJSON(supplierGenesisState) + require.NoError(t, err) + cfg.GenesisState[suppliertypes.ModuleName] = buf + + // Start the network + return network.New(t, cfg), supplierGenesisState.SupplierList, applicationGenesisState.ApplicationList +} diff --git a/x/session/client/cli/query_get_session.go b/x/session/client/cli/query_get_session.go index 1c21aa881..41b441de7 100644 --- a/x/session/client/cli/query_get_session.go +++ b/x/session/client/cli/query_get_session.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "strconv" "github.com/cosmos/cosmos-sdk/client" @@ -12,26 +13,49 @@ import ( var _ = strconv.Itoa(0) -// TODO(@Olshansk): Implement the CLI component of `GetSession`. func CmdGetSession() *cobra.Command { cmd := &cobra.Command{ - Use: "get-session", + Use: "get-session [block_height]", Short: "Query get-session", - Args: cobra.ExactArgs(0), + Long: `Query the session data for a specific (app, service, height) tuple. + +[block_height] is optional. If unspecified, or set to 0, it defaults to the latest height of the node being queried. + +This is a query operation that will not result in a state transition but simply gives a view into the chain state. + +Example: +$ pocketd --home=$(POCKETD_HOME) q session get-session pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4 svc1 42 --node $(POCKET_NODE)`, + Args: cobra.RangeArgs(2, 3), RunE: func(cmd *cobra.Command, args []string) (err error) { + appAddressString := args[0] + serviceIdString := args[1] + blockHeightString := "0" // 0 will default to latest height + if len(args) == 3 { + blockHeightString = args[2] + } + + blockHeight, err := strconv.ParseInt(blockHeightString, 10, 64) + if err != nil { + return fmt.Errorf("couldn't convert block height to int: %s; (%v)", blockHeightString, err) + } + + getSessionReq := types.NewQueryGetSessionRequest(appAddressString, serviceIdString, blockHeight) + if err := getSessionReq.ValidateBasic(); err != nil { + return err + } + clientCtx, err := client.GetClientQueryContext(cmd) if err != nil { return err } queryClient := types.NewQueryClient(clientCtx) - req := &types.QueryGetSessionRequest{} - res, err := queryClient.GetSession(cmd.Context(), req) + getSessionRes, err := queryClient.GetSession(cmd.Context(), getSessionReq) if err != nil { return err } - return clientCtx.PrintProto(res) + return clientCtx.PrintProto(getSessionRes) }, } diff --git a/x/session/client/cli/query_get_session_test.go b/x/session/client/cli/query_get_session_test.go new file mode 100644 index 000000000..480bafa48 --- /dev/null +++ b/x/session/client/cli/query_get_session_test.go @@ -0,0 +1,197 @@ +package cli_test + +import ( + "fmt" + "testing" + + sdkerrors "cosmossdk.io/errors" + tmcli "github.com/cometbft/cometbft/libs/cli" + clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" + "github.com/gogo/status" + "github.com/stretchr/testify/require" + + "github.com/pokt-network/poktroll/x/session/client/cli" + sessiontypes "github.com/pokt-network/poktroll/x/session/types" +) + +func TestCLI_GetSession(t *testing.T) { + // Prepare the network + net, suppliers, applications := networkWithApplicationsAndSupplier(t, 2) + _, err := net.WaitForHeight(10) // Wait for a sufficiently high block height to ensure the staking transactions have been processed + require.NoError(t, err) + val := net.Validators[0] + ctx := val.ClientCtx + + // Sanity check the application configs are what we expect them to be + appSvc0 := applications[0] + appSvc1 := applications[1] + + require.Len(t, appSvc0.ServiceConfigs, 2) + require.Len(t, appSvc1.ServiceConfigs, 2) + + require.Equal(t, appSvc0.ServiceConfigs[0].Service.Id, "svc0") // svc0 has a supplier + require.Equal(t, appSvc0.ServiceConfigs[1].Service.Id, "svc00") // svc00 doesn't have a supplier + require.Equal(t, appSvc1.ServiceConfigs[0].Service.Id, "svc1") // svc1 has a supplier + require.Equal(t, appSvc1.ServiceConfigs[1].Service.Id, "svc11") // svc11 doesn't have a supplier + + // Sanity check the supplier configs are what we expect them to be + supplierSvc0 := suppliers[0] // supplier for svc0 + supplierSvc1 := suppliers[1] // supplier for svc1 + + require.Len(t, supplierSvc0.Services, 1) + require.Len(t, supplierSvc1.Services, 1) + + require.Equal(t, supplierSvc0.Services[0].Service.Id, "svc0") + require.Equal(t, supplierSvc1.Services[0].Service.Id, "svc1") + + // Prepare the test cases + tests := []struct { + desc string + + appAddress string + serviceId string + blockHeight int64 + + expectedErr *sdkerrors.Error + expectedNumSuppliers int + }{ + // Valid requests + { + desc: "valid - block height specified and is zero", + + appAddress: appSvc0.Address, + serviceId: "svc0", + blockHeight: 0, + + expectedErr: nil, + expectedNumSuppliers: 1, + }, + { + desc: "valid - block height specified and is greater than zero", + + appAddress: appSvc1.Address, + serviceId: "svc1", + blockHeight: 10, + + expectedErr: nil, + expectedNumSuppliers: 1, + }, + { + desc: "valid - block height unspecified and defaults to 0", + + appAddress: appSvc0.Address, + serviceId: "svc0", + // blockHeight: intentionally omitted, + + expectedErr: nil, + expectedNumSuppliers: 1, + }, + + // Invalid requests - incompatible state + { + desc: "invalid - app not staked for service", + + appAddress: appSvc0.Address, + serviceId: "svc9001", // appSvc0 is only staked for svc0 (has supplier) and svc00 (doesn't have supplier) and is not staked for service over 9000 + blockHeight: 0, + + expectedErr: sessiontypes.ErrSessionAppNotStakedForService, + }, + { + desc: "invalid - no suppliers staked for service", + + appAddress: appSvc0.Address, // dynamically getting address from applications + serviceId: "svc00", // appSvc0 is only staked for svc0 (has supplier) and svc00 (doesn't have supplier) + blockHeight: 0, + + expectedErr: sessiontypes.ErrSessionSuppliersNotFound, + }, + { + desc: "invalid - block height is in the future", + + appAddress: appSvc0.Address, // dynamically getting address from applications + serviceId: "svc0", + blockHeight: 9001, // block height over 9000 is greater than the context height of 10 + + expectedErr: sessiontypes.ErrSessionInvalidBlockHeight, + }, + + // Invalid requests - bad app address input + { + desc: "invalid - invalid appAddress", + + appAddress: "invalidAddress", // providing a deliberately invalid address + serviceId: "svc0", + blockHeight: 0, + + expectedErr: sessiontypes.ErrSessionInvalidAppAddress, + }, + { + desc: "invalid - missing appAddress", + // appAddress: intentionally omitted + serviceId: "svc0", + blockHeight: 0, + + expectedErr: sessiontypes.ErrSessionInvalidAppAddress, + }, + + // Invalid requests - bad serviceID input + { + desc: "invalid - invalid service ID", + appAddress: appSvc0.Address, // dynamically getting address from applications + serviceId: "invalidServiceId", + blockHeight: 0, + + expectedErr: sessiontypes.ErrSessionInvalidService, + }, + { + desc: "invalid - missing service ID", + appAddress: appSvc0.Address, // dynamically getting address from applications + // serviceId: intentionally omitted + blockHeight: 0, + + expectedErr: sessiontypes.ErrSessionInvalidService, + }, + } + + // We want to use the `--output=json` flag for all tests so it's easy to unmarshal below + common := []string{ + fmt.Sprintf("--%s=json", tmcli.OutputFlag), + } + + // Run the tests + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + // Prepare the arguments for the CLI command + args := []string{ + tt.appAddress, + tt.serviceId, + fmt.Sprintf("%d", tt.blockHeight), + } + args = append(args, common...) + + // Execute the command + getSessionOut, err := clitestutil.ExecTestCLICmd(ctx, cli.CmdGetSession(), args) + if tt.expectedErr != nil { + stat, ok := status.FromError(tt.expectedErr) + require.True(t, ok) + require.Contains(t, stat.Message(), tt.expectedErr.Error()) + return + } + require.NoError(t, err) + + var getSessionRes sessiontypes.QueryGetSessionResponse + err = net.Config.Codec.UnmarshalJSON(getSessionOut.Bytes(), &getSessionRes) + require.NoError(t, err) + require.NotNil(t, getSessionRes) + + session := getSessionRes.Session + require.NotNil(t, session) + + // Verify some data about the session + require.Equal(t, tt.appAddress, session.Application.Address) + require.Equal(t, tt.serviceId, session.Header.Service.Id) + require.Len(t, session.Suppliers, tt.expectedNumSuppliers) + }) + } +} diff --git a/x/session/keeper/query_get_session.go b/x/session/keeper/query_get_session.go index 7a69e5a8c..e8931b343 100644 --- a/x/session/keeper/query_get_session.go +++ b/x/session/keeper/query_get_session.go @@ -15,8 +15,23 @@ func (k Keeper) GetSession(goCtx context.Context, req *types.QueryGetSessionRequ return nil, status.Error(codes.InvalidArgument, "invalid request") } + if err := req.ValidateBasic(); err != nil { + return nil, err + } + ctx := sdk.UnwrapSDKContext(goCtx) + // If block height is not specified, use the current (context's latest) block height + // Note that `GetSession` is called via the `Query` service rather than the `Msg` server. + // The former is stateful but does not lead to state transitions, while the latter one + // does. The request height depends on how much the node has synched and only acts as a read, + // while the `Msg` server handles the code flow of the validator/sequencer when a new block + // is being proposed. + blockHeight := req.BlockHeight + if blockHeight == 0 { + blockHeight = ctx.BlockHeight() + } + sessionHydrator := NewSessionHydrator(req.ApplicationAddress, req.Service.Id, req.BlockHeight) session, err := k.HydrateSession(ctx, sessionHydrator) if err != nil { diff --git a/x/session/keeper/query_get_session_test.go b/x/session/keeper/query_get_session_test.go index ebf80b9cf..a8b8ecedb 100644 --- a/x/session/keeper/query_get_session_test.go +++ b/x/session/keeper/query_get_session_test.go @@ -8,6 +8,7 @@ import ( "github.com/pokt-network/poktroll/cmd/pocketd/cmd" keepertest "github.com/pokt-network/poktroll/testutil/keeper" + "github.com/pokt-network/poktroll/testutil/sample" "github.com/pokt-network/poktroll/x/session/types" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) @@ -22,6 +23,7 @@ func init() { func TestSession_GetSession_Success(t *testing.T) { keeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors wctx := sdk.WrapSDKContext(ctx) type test struct { @@ -45,7 +47,7 @@ func TestSession_GetSession_Success(t *testing.T) { blockHeight: 1, // Intentionally only checking a subset of the session metadata returned - expectedSessionId: "e1e51d087e447525d7beb648711eb3deaf016a8089938a158e6a0f600979370c", + expectedSessionId: "cf5bbdce56ee5a7c46c5d5482303907685a7e5dbb22703cd4a85df521b9ab6e9", expectedSessionNumber: 0, expectedNumSuppliers: 1, }, @@ -53,7 +55,6 @@ func TestSession_GetSession_Success(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := &types.QueryGetSessionRequest{ ApplicationAddress: tt.appAddr, Service: &sharedtypes.Service{ @@ -75,6 +76,7 @@ func TestSession_GetSession_Success(t *testing.T) { func TestSession_GetSession_Failure(t *testing.T) { keeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors wctx := sdk.WrapSDKContext(ctx) type test struct { @@ -91,20 +93,56 @@ func TestSession_GetSession_Failure(t *testing.T) { { name: "application address does not reflected a staked application", - appAddr: "some string that is not a valid app address", + appAddr: sample.AccAddress(), // a random (valid) app address that's not staked serviceId: keepertest.TestServiceId1, blockHeight: 1, - expectedErrContains: types.ErrAppNotFound.Error(), + expectedErrContains: types.ErrSessionAppNotFound.Error(), + }, + { + name: "application staked for service that has no available suppliers", + + appAddr: keepertest.TestApp1Address, + serviceId: keepertest.TestServiceId11, + blockHeight: 1, + + expectedErrContains: types.ErrSessionSuppliersNotFound.Error(), }, { - name: "service ID does not reflect one with staked suppliers", + name: "application is valid but not staked for the specified service", appAddr: keepertest.TestApp1Address, - serviceId: "some string that is not a valid service Id", + serviceId: "svc9001", // App1 is not staked for service over 9000 blockHeight: 1, - expectedErrContains: types.ErrSuppliersNotFound.Error(), + expectedErrContains: types.ErrSessionAppNotStakedForService.Error(), + }, + { + name: "application address is invalid format", + + appAddr: "invalid_app_address", + serviceId: keepertest.TestServiceId1, + blockHeight: 1, + + expectedErrContains: types.ErrSessionInvalidAppAddress.Error(), + }, + { + name: "service ID is invalid", + + appAddr: keepertest.TestApp1Address, + serviceId: "service_id_is_too_long_to_be_valid", + blockHeight: 1, + + expectedErrContains: "invalid service in session", + }, + { + name: "negative block height", + + appAddr: keepertest.TestApp1Address, + serviceId: keepertest.TestServiceId1, + blockHeight: -1, + + expectedErrContains: "invalid block height for session being retrieved", }, } @@ -112,13 +150,12 @@ func TestSession_GetSession_Failure(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - req := &types.QueryGetSessionRequest{ ApplicationAddress: tt.appAddr, Service: &sharedtypes.Service{ Id: tt.serviceId, }, - BlockHeight: 1, + BlockHeight: tt.blockHeight, } res, err := keeper.GetSession(wctx, req) diff --git a/x/session/keeper/session_hydrator.go b/x/session/keeper/session_hydrator.go index 2b71143d1..38f9d529e 100644 --- a/x/session/keeper/session_hydrator.go +++ b/x/session/keeper/session_hydrator.go @@ -12,6 +12,7 @@ import ( _ "golang.org/x/crypto/sha3" "github.com/pokt-network/poktroll/x/session/types" + sharedhelpers "github.com/pokt-network/poktroll/x/shared/helpers" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) @@ -61,22 +62,22 @@ func (k Keeper) HydrateSession(ctx sdk.Context, sh *sessionHydrator) (*types.Ses logger := k.Logger(ctx).With("method", "hydrateSession") if err := k.hydrateSessionMetadata(ctx, sh); err != nil { - return nil, sdkerrors.Wrapf(types.ErrHydratingSession, "failed to hydrate the session metadata: %v", err) + return nil, sdkerrors.Wrapf(types.ErrSessionHydration, "failed to hydrate the session metadata: %v", err) } logger.Debug("Finished hydrating session metadata") if err := k.hydrateSessionID(ctx, sh); err != nil { - return nil, sdkerrors.Wrapf(types.ErrHydratingSession, "failed to hydrate the session ID: %v", err) + return nil, sdkerrors.Wrapf(types.ErrSessionHydration, "failed to hydrate the session ID: %v", err) } logger.Info("Finished hydrating session ID: %s", sh.sessionHeader.SessionId) if err := k.hydrateSessionApplication(ctx, sh); err != nil { - return nil, sdkerrors.Wrapf(types.ErrHydratingSession, "failed to hydrate application for session: %v", err) + return nil, sdkerrors.Wrapf(types.ErrSessionHydration, "failed to hydrate application for session: %v", err) } logger.Debug("Finished hydrating session application: %+v", sh.session.Application) if err := k.hydrateSessionSuppliers(ctx, sh); err != nil { - return nil, sdkerrors.Wrapf(types.ErrHydratingSession, "failed to hydrate suppliers for session: %v", err) + return nil, sdkerrors.Wrapf(types.ErrSessionHydration, "failed to hydrate suppliers for session: %v", err) } logger.Debug("Finished hydrating session suppliers: %+v") @@ -90,6 +91,10 @@ func (k Keeper) HydrateSession(ctx sdk.Context, sh *sessionHydrator) (*types.Ses func (k Keeper) hydrateSessionMetadata(ctx sdk.Context, sh *sessionHydrator) error { // TODO_TECHDEBT: Add a test if `blockHeight` is ahead of the current chain or what this node is aware of + if sh.blockHeight > ctx.BlockHeight() { + return sdkerrors.Wrapf(types.ErrSessionHydration, "block height %d is ahead of the current block height %d", sh.blockHeight, ctx.BlockHeight()) + } + sh.session.NumBlocksPerSession = NumBlocksPerSession sh.session.SessionNumber = int64(sh.blockHeight / NumBlocksPerSession) sh.sessionHeader.SessionStartBlockHeight = sh.blockHeight - (sh.blockHeight % NumBlocksPerSession) @@ -107,8 +112,11 @@ func (k Keeper) hydrateSessionID(ctx sdk.Context, sh *sessionHydrator) error { appPubKeyBz := []byte(sh.sessionHeader.ApplicationAddress) // TODO_TECHDEBT: In the future, we will need to valid that the Service is a valid service depending on whether - // or not its permissioned or permissionless - // TODO(@Olshansk): Add a check to make sure `IsValidServiceName(Service.Id)` returns True + // or not its permissioned or permissionless + + if !sharedhelpers.IsValidService(sh.sessionHeader.Service) { + return sdkerrors.Wrapf(types.ErrSessionHydration, "invalid service: %v", sh.sessionHeader.Service) + } serviceIdBz := []byte(sh.sessionHeader.Service.Id) sessionHeightBz := make([]byte, 8) @@ -124,10 +132,17 @@ func (k Keeper) hydrateSessionID(ctx sdk.Context, sh *sessionHydrator) error { func (k Keeper) hydrateSessionApplication(ctx sdk.Context, sh *sessionHydrator) error { app, appIsFound := k.appKeeper.GetApplication(ctx, sh.sessionHeader.ApplicationAddress) if !appIsFound { - return sdkerrors.Wrapf(types.ErrAppNotFound, "could not find app with address: %s at height %d", sh.sessionHeader.ApplicationAddress, sh.sessionHeader.SessionStartBlockHeight) + return sdkerrors.Wrapf(types.ErrSessionAppNotFound, "could not find app with address: %s at height %d", sh.sessionHeader.ApplicationAddress, sh.sessionHeader.SessionStartBlockHeight) } - sh.session.Application = &app - return nil + + for _, appServiceConfig := range app.ServiceConfigs { + if appServiceConfig.Service.Id == sh.sessionHeader.Service.Id { + sh.session.Application = &app + return nil + } + } + + return sdkerrors.Wrapf(types.ErrSessionAppNotStakedForService, "application %s not staked for service %s", sh.sessionHeader.ApplicationAddress, sh.sessionHeader.Service.Id) } // hydrateSessionSuppliers finds the suppliers that are staked at the session height and populates the session with them @@ -154,7 +169,7 @@ func (k Keeper) hydrateSessionSuppliers(ctx sdk.Context, sh *sessionHydrator) er if len(candidateSuppliers) == 0 { logger.Error("[ERROR] no suppliers found for session") - return sdkerrors.Wrapf(types.ErrSuppliersNotFound, "could not find suppliers for service %s at height %d", sh.sessionHeader.Service, sh.sessionHeader.SessionStartBlockHeight) + return sdkerrors.Wrapf(types.ErrSessionSuppliersNotFound, "could not find suppliers for service %s at height %d", sh.sessionHeader.Service, sh.sessionHeader.SessionStartBlockHeight) } if len(candidateSuppliers) < NumSupplierPerSession { diff --git a/x/session/keeper/session_hydrator_test.go b/x/session/keeper/session_hydrator_test.go index 61d591900..50592f735 100644 --- a/x/session/keeper/session_hydrator_test.go +++ b/x/session/keeper/session_hydrator_test.go @@ -13,6 +13,7 @@ import ( func TestSession_HydrateSession_Success_BaseCase(t *testing.T) { sessionKeeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors blockHeight := int64(10) sessionHydrator := keeper.NewSessionHydrator(keepertest.TestApp1Address, keepertest.TestServiceId1, blockHeight) @@ -30,82 +31,99 @@ func TestSession_HydrateSession_Success_BaseCase(t *testing.T) { // Check the session require.Equal(t, int64(4), session.NumBlocksPerSession) - require.Equal(t, "23f037a10f9d51d020d27763c42dd391d7e71765016d95d0d61f36c4a122efd0", session.SessionId) + require.Equal(t, "5481d5ca2ddb15dc5edb792b8e20ba9c7d516a74475fc5feba6b6aeb95a26f58", session.SessionId) require.Equal(t, int64(2), session.SessionNumber) // Check the application app := session.Application require.Equal(t, keepertest.TestApp1Address, app.Address) - require.Len(t, app.ServiceConfigs, 2) + require.Len(t, app.ServiceConfigs, 3) // Check the suppliers suppliers := session.Suppliers require.Len(t, suppliers, 1) supplier := suppliers[0] require.Equal(t, keepertest.TestSupplierAddress, supplier.Address) - require.Len(t, supplier.Services, 2) + require.Len(t, supplier.Services, 3) } func TestSession_HydrateSession_Metadata(t *testing.T) { type test struct { - name string + desc string blockHeight int64 expectedNumBlocksPerSession int64 expectedSessionNumber int64 expectedSessionStartBlock int64 expectedSessionEndBlock int64 + errExpected error } // TODO_TECHDEBT: Extend these tests once `NumBlocksPerSession` is configurable. // Currently assumes NumBlocksPerSession=4 tests := []test{ { - name: "blockHeight = 0", + desc: "blockHeight = 0", blockHeight: 0, expectedNumBlocksPerSession: 4, expectedSessionNumber: 0, expectedSessionStartBlock: 0, expectedSessionEndBlock: 4, + errExpected: nil, }, { - name: "blockHeight = 1", + desc: "blockHeight = 1", blockHeight: 1, expectedNumBlocksPerSession: 4, expectedSessionNumber: 0, expectedSessionStartBlock: 0, expectedSessionEndBlock: 4, + errExpected: nil, }, { - name: "blockHeight = sessionHeight", + desc: "blockHeight = sessionHeight", blockHeight: 4, expectedNumBlocksPerSession: 4, expectedSessionNumber: 1, expectedSessionStartBlock: 4, expectedSessionEndBlock: 8, + errExpected: nil, }, { - name: "blockHeight != sessionHeight", + desc: "blockHeight != sessionHeight", blockHeight: 5, expectedNumBlocksPerSession: 4, expectedSessionNumber: 1, expectedSessionStartBlock: 4, expectedSessionEndBlock: 8, + errExpected: nil, + }, + { + desc: "blockHeight > contextHeight", + blockHeight: 9001, // block height over 9000 is too height given that the context height is 100 + + errExpected: types.ErrSessionHydration, }, } appAddr := keepertest.TestApp1Address serviceId := keepertest.TestServiceId1 sessionKeeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.desc, func(t *testing.T) { sessionHydrator := keeper.NewSessionHydrator(appAddr, serviceId, tt.blockHeight) session, err := sessionKeeper.HydrateSession(ctx, sessionHydrator) + + if tt.errExpected != nil { + require.ErrorIs(t, tt.errExpected, err) + return + } require.NoError(t, err) require.Equal(t, tt.expectedNumBlocksPerSession, session.NumBlocksPerSession) @@ -118,7 +136,7 @@ func TestSession_HydrateSession_Metadata(t *testing.T) { func TestSession_HydrateSession_SessionId(t *testing.T) { type test struct { - name string + desc string blockHeight1 int64 blockHeight2 int64 @@ -137,7 +155,7 @@ func TestSession_HydrateSession_SessionId(t *testing.T) { // Currently assumes NumBlocksPerSession=4 tests := []test{ { - name: "(app1, svc1): sessionId at first session block != sessionId at next session block", + desc: "(app1, svc1): sessionId at first session block != sessionId at next session block", blockHeight1: 4, blockHeight2: 8, @@ -148,11 +166,11 @@ func TestSession_HydrateSession_SessionId(t *testing.T) { serviceId1: keepertest.TestServiceId1, // svc1 serviceId2: keepertest.TestServiceId1, // svc1 - expectedSessionId1: "aabaa25668538f80395170be95ce1d1536d9228353ced71cc3b763171316fe39", - expectedSessionId2: "23f037a10f9d51d020d27763c42dd391d7e71765016d95d0d61f36c4a122efd0", + expectedSessionId1: "251665c7cf286a30fbd98acd983c63e9a34efc16496511373405e24eb02a8fb9", + expectedSessionId2: "5481d5ca2ddb15dc5edb792b8e20ba9c7d516a74475fc5feba6b6aeb95a26f58", }, { - name: "app1: sessionId for svc1 != sessionId for svc2", + desc: "app1: sessionId for svc1 != sessionId for svc12", blockHeight1: 4, blockHeight2: 4, @@ -160,14 +178,14 @@ func TestSession_HydrateSession_SessionId(t *testing.T) { appAddr1: keepertest.TestApp1Address, // app1 appAddr2: keepertest.TestApp1Address, // app1 - serviceId1: keepertest.TestServiceId1, // svc1 - serviceId2: keepertest.TestServiceId2, // svc2 + serviceId1: keepertest.TestServiceId1, // svc1 + serviceId2: keepertest.TestServiceId12, // svc12 - expectedSessionId1: "aabaa25668538f80395170be95ce1d1536d9228353ced71cc3b763171316fe39", - expectedSessionId2: "478d005769e5edf38d9bf2d8828a56d78b17348bb2c4796dd6d85b5d736a908a", + expectedSessionId1: "251665c7cf286a30fbd98acd983c63e9a34efc16496511373405e24eb02a8fb9", + expectedSessionId2: "44fce80205bece269429a5dc8b55f9d96e5bf7acdb9838f2ac9aa7216905a1cf", }, { - name: "svc1: sessionId for app1 != sessionId for app2", + desc: "svc12: sessionId for app1 != sessionId for app2", blockHeight1: 4, blockHeight2: 4, @@ -175,18 +193,19 @@ func TestSession_HydrateSession_SessionId(t *testing.T) { appAddr1: keepertest.TestApp1Address, // app1 appAddr2: keepertest.TestApp2Address, // app2 - serviceId1: keepertest.TestServiceId1, // svc1 - serviceId2: keepertest.TestServiceId1, // svc1 + serviceId1: keepertest.TestServiceId12, // svc12 + serviceId2: keepertest.TestServiceId12, // svc12 - expectedSessionId1: "aabaa25668538f80395170be95ce1d1536d9228353ced71cc3b763171316fe39", - expectedSessionId2: "b4b0d8747b1cf67050a7bfefd7e93ebbad80c534fa14fb3c69339886f2ed7061", + expectedSessionId1: "44fce80205bece269429a5dc8b55f9d96e5bf7acdb9838f2ac9aa7216905a1cf", + expectedSessionId2: "22328e12562532047c9d4200beaedc9be694cd99b38938ba64cf4cdca0a8ecba", }, } sessionKeeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.desc, func(t *testing.T) { sessionHydrator1 := keeper.NewSessionHydrator(tt.appAddr1, tt.serviceId1, tt.blockHeight1) session1, err := sessionKeeper.HydrateSession(ctx, sessionHydrator1) require.NoError(t, err) @@ -205,30 +224,48 @@ func TestSession_HydrateSession_SessionId(t *testing.T) { // TODO_TECHDEBT: Expand these tests to account for application joining/leaving the network at different heights as well changing the services they support func TestSession_HydrateSession_Application(t *testing.T) { type test struct { - name string - appAddr string + // Description + desc string + // Inputs + appAddr string + serviceId string + // Outputs expectedErr error } tests := []test{ { - name: "app is found", - appAddr: keepertest.TestApp1Address, + desc: "app is found", + + appAddr: keepertest.TestApp1Address, + serviceId: keepertest.TestServiceId1, expectedErr: nil, }, { - name: "app is not found", - appAddr: sample.AccAddress(), // Generating a random address on the fly + desc: "app is not found", - expectedErr: types.ErrHydratingSession, + appAddr: sample.AccAddress(), // Generating a random address on the fly + serviceId: keepertest.TestServiceId1, + + expectedErr: types.ErrSessionHydration, }, { - name: "invalid app address", - appAddr: "invalid", + desc: "invalid app address", + + appAddr: "invalid", + serviceId: keepertest.TestServiceId1, - expectedErr: types.ErrHydratingSession, + expectedErr: types.ErrSessionHydration, + }, + { + desc: "invalid - app not staked for service", + + appAddr: keepertest.TestApp1Address, // app1 + serviceId: "svc9001", // app1 is only stake for svc1 and svc11 + + expectedErr: types.ErrSessionHydration, }, // TODO_TECHDEBT: Add tests for when: // - Application join/leaves (stakes/unstakes) altogether @@ -236,13 +273,13 @@ func TestSession_HydrateSession_Application(t *testing.T) { // - Application increases stakes mid-session } - serviceId := keepertest.TestServiceId1 blockHeight := int64(10) sessionKeeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - sessionHydrator := keeper.NewSessionHydrator(tt.appAddr, serviceId, blockHeight) + t.Run(tt.desc, func(t *testing.T) { + sessionHydrator := keeper.NewSessionHydrator(tt.appAddr, tt.serviceId, blockHeight) _, err := sessionKeeper.HydrateSession(ctx, sessionHydrator) if tt.expectedErr != nil { require.Error(t, err) @@ -256,10 +293,14 @@ func TestSession_HydrateSession_Application(t *testing.T) { // TODO_TECHDEBT: Expand these tests to account for supplier joining/leaving the network at different heights as well changing the services they support func TestSession_HydrateSession_Suppliers(t *testing.T) { type test struct { - name string + // Description + desc string + + // Inputs appAddr string serviceId string + // Outputs numExpectedSuppliers int expectedErr error } @@ -268,15 +309,17 @@ func TestSession_HydrateSession_Suppliers(t *testing.T) { // Currently assumes NumSupplierPerSession=15 tests := []test{ { - name: "num_suppliers_available = 0", + desc: "num_suppliers_available = 0", + appAddr: keepertest.TestApp1Address, // app1 - serviceId: "svc_unknown", + serviceId: keepertest.TestServiceId11, numExpectedSuppliers: 0, - expectedErr: types.ErrSuppliersNotFound, + expectedErr: types.ErrSessionSuppliersNotFound, }, { - name: "num_suppliers_available < num_suppliers_per_session_param", + desc: "num_suppliers_available < num_suppliers_per_session_param", + appAddr: keepertest.TestApp1Address, // app1 serviceId: keepertest.TestServiceId1, // svc1 @@ -295,9 +338,10 @@ func TestSession_HydrateSession_Suppliers(t *testing.T) { blockHeight := int64(10) sessionKeeper, ctx := keepertest.SessionKeeper(t) + ctx = ctx.WithBlockHeight(100) // provide a sufficiently large block height to avoid errors for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) {}) + t.Run(tt.desc, func(t *testing.T) {}) sessionHydrator := keeper.NewSessionHydrator(tt.appAddr, tt.serviceId, blockHeight) session, err := sessionKeeper.HydrateSession(ctx, sessionHydrator) diff --git a/x/session/types/errors.go b/x/session/types/errors.go index 3bf90eae7..dde73c1e6 100644 --- a/x/session/types/errors.go +++ b/x/session/types/errors.go @@ -8,7 +8,11 @@ import ( // x/session module sentinel errors var ( - ErrHydratingSession = sdkerrors.Register(ModuleName, 1, "error during session hydration") - ErrAppNotFound = sdkerrors.Register(ModuleName, 2, "application not found") - ErrSuppliersNotFound = sdkerrors.Register(ModuleName, 3, "suppliers not found") + ErrSessionHydration = sdkerrors.Register(ModuleName, 1, "error during session hydration") + ErrSessionAppNotFound = sdkerrors.Register(ModuleName, 2, "application for session not found not found ") + ErrSessionAppNotStakedForService = sdkerrors.Register(ModuleName, 3, "application in session not staked for requested service") + ErrSessionSuppliersNotFound = sdkerrors.Register(ModuleName, 4, "no suppliers not found for session") + ErrSessionInvalidAppAddress = sdkerrors.Register(ModuleName, 5, "invalid application address for session") + ErrSessionInvalidService = sdkerrors.Register(ModuleName, 6, "invalid service in session") + ErrSessionInvalidBlockHeight = sdkerrors.Register(ModuleName, 7, "invalid block height for session") ) diff --git a/x/session/types/query_get_session_request.go b/x/session/types/query_get_session_request.go new file mode 100644 index 000000000..31b705f8e --- /dev/null +++ b/x/session/types/query_get_session_request.go @@ -0,0 +1,40 @@ +package types + +import ( + sdkerrors "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" + + sharedhelpers "github.com/pokt-network/poktroll/x/shared/helpers" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" +) + +// NOTE: Please note that `QueryGetSessionRequest` is not a `sdk.Msg`, and is therefore not a message/request +// that will be signable or invoke a state transition. However, following a similar `ValidateBasic` pattern +// allows us to localize & reuse validation logic. +func NewQueryGetSessionRequest(appAddress, serviceId string, blockHeight int64) *QueryGetSessionRequest { + return &QueryGetSessionRequest{ + ApplicationAddress: appAddress, + Service: &sharedtypes.Service{ + Id: serviceId, + }, + BlockHeight: blockHeight, + } +} + +func (query *QueryGetSessionRequest) ValidateBasic() error { + // Validate the application address + if _, err := sdk.AccAddressFromBech32(query.ApplicationAddress); err != nil { + return sdkerrors.Wrapf(ErrSessionInvalidAppAddress, "invalid app address for session being retrieved %s; (%v)", query.ApplicationAddress, err) + } + + // Validate the Service ID + if !sharedhelpers.IsValidService(query.Service) { + return sdkerrors.Wrapf(ErrSessionInvalidService, "invalid service for session being retrieved %s;", query.Service) + } + + // Validate the height for which a session is being retrieved + if query.BlockHeight < 0 { // Note that `0` defaults to the latest height rather than genesis + return sdkerrors.Wrapf(ErrSessionInvalidBlockHeight, "invalid block height for session being retrieved %d;", query.BlockHeight) + } + return nil +} diff --git a/x/shared/helpers/service.go b/x/shared/helpers/service.go index 760ee0162..9825f26d4 100644 --- a/x/shared/helpers/service.go +++ b/x/shared/helpers/service.go @@ -27,9 +27,12 @@ func init() { } -// IsValidService checks if the input service is valid +// IsValidService checks if the provided ServiceId struct has valid fields func IsValidService(service *sharedtypes.Service) bool { - return service != nil && IsValidServiceId(service.Id) && IsValidServiceName(service.Name) + // Check if service Id and Name are valid using the provided helper functions + return service != nil && + IsValidServiceId(service.Id) && + IsValidServiceName(service.Name) } // IsValidServiceId checks if the input string is a valid serviceId diff --git a/x/shared/helpers/service_test.go b/x/shared/helpers/service_test.go index 63009cf28..d74b77afa 100644 --- a/x/shared/helpers/service_test.go +++ b/x/shared/helpers/service_test.go @@ -12,60 +12,119 @@ func TestIsValidService(t *testing.T) { tests := []struct { desc string - id string - name string - expected bool + serviceId string + serviceName string + + expectedIsValid bool }{ { desc: "Valid ID and Name", - id: "Service1", - name: "Valid Service Name", - expected: true, + serviceId: "Service1", + serviceName: "Valid Service Name", + + expectedIsValid: true, }, { desc: "Valid ID and empty Name", - id: "Srv", - name: "", // Valid because the service name can be empty - expected: true, + serviceId: "Srv", + serviceName: "", // Valid because the service name can be empty + + expectedIsValid: true, }, { desc: "ID exceeds max length", - id: "TooLongId123", // Exceeds maxServiceIdLength - name: "Valid Name", - expected: false, + serviceId: "TooLongId123", // Exceeds maxServiceIdLength + serviceName: "Valid Name", + + expectedIsValid: false, }, { - desc: "Name exceeds max length", - id: "ValidID", - name: "This service name is way too long to be considered valid since it exceeds the max length", - expected: false, + desc: "Name exceeds max length", + + serviceId: "ValidID", + serviceName: "This service name is way too long to be considered valid since it exceeds the max length", + + expectedIsValid: false, }, { desc: "Empty ID is invalid", - id: "", // Invalid because the service ID cannot be empty - name: "Valid Name", - expected: false, + serviceId: "", // Invalid because the service ID cannot be empty + serviceName: "Valid Name", + + expectedIsValid: false, }, { desc: "Invalid characters in ID", - id: "ID@Invalid", // Invalid character '@' - name: "Valid Name", - expected: false, + serviceId: "ID@Invalid", // Invalid character '@' + serviceName: "Valid Name", + + expectedIsValid: false, }, } for _, test := range tests { t.Run(test.desc, func(t *testing.T) { service := &sharedtypes.Service{ - Id: test.id, - Name: test.name, + Id: test.serviceId, + Name: test.serviceName, } result := IsValidService(service) + require.Equal(t, test.expectedIsValid, result) + }) + } +} + +func TestIsValidServiceName(t *testing.T) { + tests := []struct { + desc string + input string + expected bool + }{ + { + desc: "Valid with hyphen and number", + input: "ValidName-1", + expected: true, + }, + { + desc: "Valid with space and underscore", + input: "Valid Name_1", + expected: true, + }, + { + desc: "Valid name with spaces", + input: "valid name with spaces", + expected: true, + }, + { + desc: "Invalid character '@'", + input: "invalid@name", + expected: false, + }, + { + desc: "Invalid character '.'", + input: "Valid.Name", + expected: false, + }, + { + desc: "Empty string", + input: "", + expected: true, + }, + { + desc: "Exceeds maximum length", + input: "validnamebuttoolongvalidnamebuttoolongvalidnamebuttoolong", + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + result := IsValidServiceName(test.input) require.Equal(t, test.expected, result) }) } diff --git a/x/supplier/client/cli/query_supplier.go b/x/supplier/client/cli/query_supplier.go index cfbc6b4ec..604c1182f 100644 --- a/x/supplier/client/cli/query_supplier.go +++ b/x/supplier/client/cli/query_supplier.go @@ -46,7 +46,7 @@ func CmdListSupplier() *cobra.Command { func CmdShowSupplier() *cobra.Command { cmd := &cobra.Command{ - Use: "show-supplier [address]", + Use: "show-supplier ", Short: "shows a supplier", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) (err error) { diff --git a/x/supplier/client/cli/tx_create_claim.go b/x/supplier/client/cli/tx_create_claim.go index db951891f..2869ce95d 100644 --- a/x/supplier/client/cli/tx_create_claim.go +++ b/x/supplier/client/cli/tx_create_claim.go @@ -2,9 +2,8 @@ package cli import ( "encoding/base64" - "strconv" - "encoding/json" + "strconv" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" @@ -21,7 +20,7 @@ var _ = strconv.Itoa(0) func CmdCreateClaim() *cobra.Command { cmd := &cobra.Command{ - Use: "create-claim [session-header] [root-hash-base64]", + Use: "create-claim ", Short: "Broadcast message create-claim", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) (err error) { diff --git a/x/supplier/client/cli/tx_stake_supplier.go b/x/supplier/client/cli/tx_stake_supplier.go index f74a874fa..83a1413e2 100644 --- a/x/supplier/client/cli/tx_stake_supplier.go +++ b/x/supplier/client/cli/tx_stake_supplier.go @@ -23,7 +23,7 @@ func CmdStakeSupplier() *cobra.Command { // TODO_HACK: For now we are only specifying the service IDs as a list of of strings separated by commas. // This needs to be expand to specify the full SupplierServiceConfig. Furthermore, providing a flag to // a file where SupplierServiceConfig specifying full service configurations in the CLI by providing a flag that accepts a JSON string - Use: "stake-supplier [amount] [svcId1;url1,svcId2;url2,...,svcIdN;urlN]", + Use: "stake-supplier ", Short: "Stake a supplier", Long: `Stake an supplier with the provided parameters. This is a broadcast operation that will stake the tokens and associate them with the supplier specified by the 'from' address. diff --git a/x/supplier/client/cli/tx_submit_proof.go b/x/supplier/client/cli/tx_submit_proof.go index 64c498026..0c3ba758f 100644 --- a/x/supplier/client/cli/tx_submit_proof.go +++ b/x/supplier/client/cli/tx_submit_proof.go @@ -2,9 +2,8 @@ package cli import ( "encoding/base64" - "strconv" - "encoding/json" + "strconv" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" @@ -15,13 +14,13 @@ import ( "github.com/pokt-network/poktroll/x/supplier/types" ) -var _ = strconv.Itoa(0) - // TODO(@bryanchriswhite): Add unit tests for the CLI command when implementing the business logic. +var _ = strconv.Itoa(0) + func CmdSubmitProof() *cobra.Command { cmd := &cobra.Command{ - Use: "submit-proof [session-header] [proof-base64]", + Use: "submit-proof ", Short: "Broadcast message submit-proof", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) (err error) {