[Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups"

.github/workflows-helpers/run-e2e-test-job-template.yaml

@@ -31,6 +31,8 @@ spec:
  name: celestia-secret
  - name: POCKET_NODE
  value: tcp://${NAMESPACE}-sequencer:36657
+ - name: SEQUENCER_RPC_ENDPOINT
+ value: ${NAMESPACE}-sequencer:36657
  - name: E2E_DEBUG_OUTPUT
  value: "false" # Flip to true to see the command and result of the execution
  - name: POKTROLLD_HOME

Dockerfile.dev

@@ -23,6 +23,5 @@ RUN ignite chain init --skip-proto
 EXPOSE 8545
 EXPOSE 8546
 EXPOSE 8547
-EXPOSE 8548
 
 ENTRYPOINT ["ignite"]

Tiltfile

@@ -33,6 +33,7 @@ if localnet_config["helm_chart_local_repo"]["enabled"]:
  print("Using local helm chart repo " + helm_chart_local_repo)
  chart_prefix = helm_chart_local_repo + "/charts/"
 
+
 # Import files into Kubernetes ConfigMap
 def read_files_from_directory(directory):
  files = listdir(directory)
@@ -158,7 +159,7 @@ k8s_resource(
  "relayminers",
  labels=["blockchains"],
  resource_deps=["sequencer"],
- port_forwards=["8548", "40005"],
+ port_forwards=["8545", "40005"],
 )
 k8s_resource(
  "appgateservers",

app/app.go

@@ -110,6 +110,8 @@ import (
  ibckeeper "github.com/cosmos/ibc-go/v7/modules/core/keeper"
  solomachine "github.com/cosmos/ibc-go/v7/modules/light-clients/06-solomachine"
  ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
+ "github.com/spf13/cast"
+
  appparams "github.com/pokt-network/poktroll/app/params"
  "github.com/pokt-network/poktroll/docs"
  applicationmodule "github.com/pokt-network/poktroll/x/application"
@@ -133,7 +135,6 @@ import (
  tokenomicsmodule "github.com/pokt-network/poktroll/x/tokenomics"
  tokenomicsmodulekeeper "github.com/pokt-network/poktroll/x/tokenomics/keeper"
  tokenomicsmoduletypes "github.com/pokt-network/poktroll/x/tokenomics/types"
- "github.com/spf13/cast"
 )
 
 const (

e2e/tests/node.go

@@ -118,9 +118,9 @@ func (p *pocketdBin) runCurlPostCmd(rpcUrl string, service string, data string,
  dataStr := fmt.Sprintf("%s", data)
  urlStr := fmt.Sprintf("%s/%s", rpcUrl, service)
  base := []string{
- "-v",  // verbose output
- "-sS",  // silent with error
- "POST",   // HTTP method
+ "-v", // verbose output
+ "-sS", // silent with error
+ "-X", "POST", // HTTP method
  "-H", "Content-Type: application/json", // HTTP headers
  "--data", dataStr, urlStr, // POST data
  }

e2e/tests/relay.feature

@@ -1,5 +1,7 @@
 Feature: Relay Namespace
 
+ # TODO_TECHDEBT(@Olshansk, #180): This test requires you to run `make supplier1_stake && make app1_stake` first
+ # As a shorter workaround, we can also add steps that stake the application and supplier as part of the scenario.
  Scenario: App can send relay to Supplier
  Given the user has the pocketd binary installed
  And the application "app1" is staked for service "anvil"

e2e/tests/session_steps_test.go

@@ -11,20 +11,19 @@ import (
  "time"
 
  abci "github.com/cometbft/cometbft/abci/types"
- "github.com/stretchr/testify/require"
-
  "github.com/pokt-network/poktroll/pkg/client/events"
  "github.com/pokt-network/poktroll/pkg/either"
  "github.com/pokt-network/poktroll/pkg/observable"
  "github.com/pokt-network/poktroll/pkg/observable/channel"
  "github.com/pokt-network/poktroll/testutil/testclient"
  suppliertypes "github.com/pokt-network/poktroll/x/supplier/types"
+ "github.com/stretchr/testify/require"
 )
 
 const (
  createClaimTimeoutDuration = 10 * time.Second
  eitherEventsReplayBufferSize = 100
- msgClaimSenderQueryFmt = "tm.event='Tx' AND message.sender='%s'"
+ msgClaimSenderQueryFmt = "tm.event='Tx' AND message.sender='%s' AND message.action='/pocket.supplier.MsgCreateClaim'"
  testServiceId = "anvil"
  eitherEventsBzReplayObsKey = "eitherEventsBzReplayObsKey"
  preExistingClaimsKey = "preExistingClaimsKey"
@@ -86,17 +85,21 @@ func (s *suite) AfterTheSupplierCreatesAClaimForTheSessionForServiceForApplicati
 func (s *suite) TheClaimCreatedBySupplierForServiceForApplicationShouldBePersistedOnchain(supplierName, serviceId, appName string) {
  ctx := context.Background()
 
- claimsRes, err := s.supplierQueryClient.AllClaims(ctx, &suppliertypes.QueryAllClaimsRequest{
+ allClaimsRes, err := s.supplierQueryClient.AllClaims(ctx, &suppliertypes.QueryAllClaimsRequest{
  Filter: &suppliertypes.QueryAllClaimsRequest_SupplierAddress{
  SupplierAddress: accNameToAddrMap[supplierName],
  },
  })
  require.NoError(s, err)
- require.NotNil(s, claimsRes)
+ require.NotNil(s, allClaimsRes)
 
  // Assert that the number of claims has increased by one.
  preExistingClaims := s.scenarioState[preExistingClaimsKey].([]suppliertypes.Claim)
- require.Len(s, claimsRes.Claim, len(preExistingClaims)+1)
+ // NB: We are avoiding the use of require.Len here because it provides unreadable output
+ // TODO_TECHDEBT: Due to the speed of the blocks of the LocalNet sequencer, along with the small number
+ // of blocks per session, multiple claims may be created throughout the duration of the test. Until
+ // these values are appropriately adjusted
+ require.Greater(s, len(allClaimsRes.Claim), len(preExistingClaims), "number of claims must have increased")
 
  // TODO_IMPROVE: assert that the root hash of the claim contains the correct
  // SMST sum. The sum can be retrieved by parsing the last 8 bytes as a
@@ -106,7 +109,7 @@ func (s *suite) TheClaimCreatedBySupplierForServiceForApplicationShouldBePersist
  // TODO_IMPROVE: add assertions about serviceId and appName and/or incorporate
  // them into the scenarioState key(s).
 
- claim := claimsRes.Claim[0]
+ claim := allClaimsRes.Claim[0]
  require.Equal(s, accNameToAddrMap[supplierName], claim.SupplierAddress)
 }
 
@@ -118,9 +121,9 @@ func (s *suite) TheSupplierHasServicedASessionWithRelaysForServiceForApplication
 
  // Query for any existing claims so that we can compensate for them in the
  // future assertions about changes in on-chain claims.
- claimsRes, err := s.supplierQueryClient.AllClaims(ctx, &suppliertypes.QueryAllClaimsRequest{})
+ allClaimsRes, err := s.supplierQueryClient.AllClaims(ctx, &suppliertypes.QueryAllClaimsRequest{})
  require.NoError(s, err)
- s.scenarioState[preExistingClaimsKey] = claimsRes.Claim
+ s.scenarioState[preExistingClaimsKey] = allClaimsRes.Claim
 
  // Construct an events query client to listen for tx events from the supplier.
  msgSenderQuery := fmt.Sprintf(msgClaimSenderQueryFmt, accNameToAddrMap[supplierName])

go.mod

@@ -24,6 +24,7 @@ require (
  github.com/athanorlabs/go-dleq v0.1.0
  github.com/cometbft/cometbft v0.37.2
  github.com/cometbft/cometbft-db v0.8.0
+ github.com/cosmos/cosmos-proto v1.0.0-beta.2
  github.com/cosmos/cosmos-sdk v0.47.3
  github.com/cosmos/gogoproto v1.4.11
  github.com/cosmos/ibc-go/v7 v7.1.0
@@ -45,6 +46,7 @@ require (
  go.uber.org/multierr v1.11.0
  golang.org/x/crypto v0.15.0
  golang.org/x/sync v0.5.0
+ google.golang.org/genproto/googleapis/api v0.0.0-20230913181813-007df8e322eb
  google.golang.org/grpc v1.59.0
  gopkg.in/yaml.v2 v2.4.0
 )
@@ -88,7 +90,6 @@ require (
  github.com/containerd/cgroups v1.1.0 // indirect
  github.com/coreos/go-systemd/v22 v22.5.0 // indirect
  github.com/cosmos/btcutil v1.0.5 // indirect
- github.com/cosmos/cosmos-proto v1.0.0-beta.2 // indirect
  github.com/cosmos/go-bip39 v1.0.0 // indirect
  github.com/cosmos/gogogateway v1.2.0 // indirect
  github.com/cosmos/iavl v0.20.0 // indirect
@@ -284,7 +285,6 @@ require (
  google.golang.org/api v0.143.0 // indirect
  google.golang.org/appengine v1.6.7 // indirect
  google.golang.org/genproto v0.0.0-20230913181813-007df8e322eb // indirect
- google.golang.org/genproto/googleapis/api v0.0.0-20230913181813-007df8e322eb // indirect
  google.golang.org/genproto/googleapis/rpc v0.0.0-20230920204549-e6e6cdab5c13 // indirect
  google.golang.org/protobuf v1.31.0 // indirect
  gopkg.in/ini.v1 v1.67.0 // indirect

localnet/kubernetes/values-relayminer.yaml

@@ -1,4 +1,3 @@
 config:
  query_node_url: tcp://sequencer-poktroll-sequencer:36657
  network_node_url: tcp://sequencer-poktroll-sequencer:36657
-

pkg/client/block/block.go

@@ -27,9 +27,9 @@ func (blockEvent *cometBlockEvent) Hash() []byte {
  return blockEvent.Block.LastBlockID.Hash.Bytes()
 }
 
-// newCometBlockEventFactoryFn is a factory function that returns a functon
+// newCometBlockEventFactoryFn is a factory function that returns a function
 // that attempts to deserialize the given bytes into a comet block.
-// if the resulting block has a height of zero, assume the event was not a block
+// If the resulting block has a height of zero, assume the event was not a block
 // event and return an ErrUnmarshalBlockEvent error.
 func newCometBlockEventFactoryFn() events.NewEventsFn[client.Block] {
  return func(blockMsgBz []byte) (client.Block, error) {
@@ -38,7 +38,9 @@ func newCometBlockEventFactoryFn() events.NewEventsFn[client.Block] {
  return nil, err
  }
 
- // If msg does not match the expected format then the block's height has a zero value.
+ // The header height should never be zero. If it is, it means that blockMsg
+ // does not match the expected format which led unmarshaling to fail,
+ // and blockHeader.height to have a default value.
  if blockMsg.Block.Header.Height == 0 {
  return nil, events.ErrEventsUnmarshalEvent.
  Wrapf("with block data: %s", string(blockMsgBz))

pkg/client/block/client.go

@@ -15,11 +15,11 @@ const (
  // the chain.
  // See: https://docs.cosmos.network/v0.47/learn/advanced/events#default-events
  committedBlocksQuery = "tm.event='NewBlock'"
- // TODO_TECHDEBT/TODO_FUTURE: add a `blocksReplayLimit` field to the block
- // client struct that defaults to this but can be overridden via an option
- // in future work.
+
  // defaultBlocksReplayLimit is the number of blocks that the replay
  // observable returned by LastNBlocks() will be able to replay.
+ // TODO_TECHDEBT/TODO_FUTURE: add a `blocksReplayLimit` field to the blockClient
+ // struct that defaults to this but can be overridden via an option.
  defaultBlocksReplayLimit = 100
 )
 

pkg/client/delegation/client.go

@@ -15,11 +15,11 @@ const (
  // See: https://docs.cosmos.network/v0.47/learn/advanced/events#subscribing-to-events
  // And: https://docs.cosmos.network/v0.47/learn/advanced/events#default-events
  delegationEventQuery = "message.action='pocket.application.EventRedelegation'"
- // TODO_TECHDEBT/TODO_FUTURE: add a `blocksReplayLimit` field to the block
- // client struct that defaults to this but can be overridden via an option
- // in future work.
+
  // defaultRedelegationsReplayLimit is the number of redelegations that the
  // replay observable returned by LastNRedelegations() will be able to replay.
+ // TODO_TECHDEBT/TODO_FUTURE: add a `redelegationsReplayLimit` field to the `delegationClient`
+ // struct that defaults to this but can be overridden via an option.
  defaultRedelegationsReplayLimit = 100
 )
 

pkg/client/events/replay_client_example_test.go

@@ -6,7 +6,6 @@ import (
  "fmt"
 
  "cosmossdk.io/depinject"
-
  "github.com/pokt-network/poktroll/pkg/client/events"
  "github.com/pokt-network/poktroll/pkg/observable"
  "github.com/pokt-network/poktroll/pkg/polylog"
@@ -23,6 +22,8 @@ const (
  replayObsBufferSize = 1
 )
 
+var _ EventType = (*eventType)(nil)
+
 // Define an interface to represent the onchain event
 type EventType interface {
  GetName() string // Illustrative only; arbitrary interfaces are supported.

pkg/client/interface.go

@@ -111,7 +111,7 @@ type Block interface {
 
 // Redelegation is an interface which wraps the EventRedelegation event
 // emitted by the application module.
-// See: proto/pocket/application/types/event.proto#EventRedelegatio
+// See: proto/pocket/application/types/event.proto#EventRedelegation
 type Redelegation interface {
  GetAppAddress() string
  GetGatewayAddress() string

pkg/client/supplier/client.go

@@ -9,6 +9,7 @@ import (
 
  "github.com/pokt-network/poktroll/pkg/client"
  "github.com/pokt-network/poktroll/pkg/client/keyring"
+ "github.com/pokt-network/poktroll/pkg/polylog"
  sessiontypes "github.com/pokt-network/poktroll/x/session/types"
  suppliertypes "github.com/pokt-network/poktroll/x/supplier/types"
 )
@@ -66,6 +67,8 @@ func (sClient *supplierClient) SubmitProof(
  sessionHeader sessiontypes.SessionHeader,
  proof *smt.SparseMerkleClosestProof,
 ) error {
+ logger := polylog.Ctx(ctx)
+
  proofBz, err := proof.Marshal()
  if err != nil {
  return err
@@ -82,6 +85,16 @@ func (sClient *supplierClient) SubmitProof(
  return err
  }
 
+ // TODO_IMPROVE: log details related to what & how much is being proven
+ logger.Info().
+ Fields(map[string]any{
+ "supplier_addr": sClient.signingKeyAddr.String(),
+ "app_addr": sessionHeader.ApplicationAddress,
+ "session_id": sessionHeader.SessionId,
+ "service": sessionHeader.Service.Id,
+ }).
+ Msg("submitted a new proof")
+
  return <-errCh
 }
 
@@ -93,6 +106,8 @@ func (sClient *supplierClient) CreateClaim(
  sessionHeader sessiontypes.SessionHeader,
  rootHash []byte,
 ) error {
+ logger := polylog.Ctx(ctx)
+
  msg := &suppliertypes.MsgCreateClaim{
  SupplierAddress: sClient.signingKeyAddr.String(),
  SessionHeader: &sessionHeader,
@@ -104,8 +119,17 @@ func (sClient *supplierClient) CreateClaim(
  return err
  }
 
- err = <-errCh
- return err
+ // TODO_IMPROVE: log details related to how much is claimed
+ logger.Info().
+ Fields(map[string]any{
+ "supplier_addr": sClient.signingKeyAddr.String(),
+ "app_addr": sessionHeader.ApplicationAddress,
+ "session_id": sessionHeader.SessionId,
+ "service": sessionHeader.Service.Id,
+ }).
+ Msg("created a new claim")
+
+ return <-errCh
 }
 
 // validateConfigAndSetDefaults attempts to get the address from the keyring

pkg/client/supplier/client_test.go

@@ -8,19 +8,21 @@ import (
 
  "cosmossdk.io/depinject"
  "github.com/golang/mock/gomock"
- "github.com/pokt-network/smt"
- "github.com/stretchr/testify/require"
-
- "github.com/pokt-network/poktroll/testutil/mockclient"
-
  "github.com/pokt-network/poktroll/pkg/client/keyring"
  "github.com/pokt-network/poktroll/pkg/client/supplier"
+ "github.com/pokt-network/poktroll/testutil/mockclient"
  "github.com/pokt-network/poktroll/testutil/testclient/testkeyring"
  "github.com/pokt-network/poktroll/testutil/testclient/testtx"
  sessiontypes "github.com/pokt-network/poktroll/x/session/types"
+ sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
+ "github.com/pokt-network/smt"
+ "github.com/stretchr/testify/require"
 )
 
-var testSigningKeyName = "test_signer"
+const (
+ testSigningKeyName = "test_signer"
+ testService = "test_service"
+)
 
 func TestNewSupplierClient(t *testing.T) {
  ctrl := gomock.NewController(t)
@@ -85,7 +87,7 @@ func TestSupplierClient_CreateClaim(t *testing.T) {
  require.NoError(t, err)
 
  txCtxMock, _ := testtx.NewAnyTimesTxTxContext(t, keyring)
- txClientMock := testtx.NewOneTimeDelayedSignAndBroadcastTxClient(t, signAndBroadcastDelay)
+ txClientMock := testtx.NewOneTimeDelayedSignAndBroadcastTxClient(t, ctx, signAndBroadcastDelay)
 
  signingKeyOpt := supplier.WithSigningKeyName(testAppKey.Name)
  deps := depinject.Supply(
@@ -102,6 +104,9 @@ func TestSupplierClient_CreateClaim(t *testing.T) {
  ApplicationAddress: testAppAddr.String(),
  SessionStartBlockHeight: 0,
  SessionId: "",
+ Service: &sharedtypes.Service{
+ Id: testService,
+ },
  }
 
  go func() {
@@ -141,7 +146,7 @@ func TestSupplierClient_SubmitProof(t *testing.T) {
  require.NoError(t, err)
 
  txCtxMock, _ := testtx.NewAnyTimesTxTxContext(t, keyring)
- txClientMock := testtx.NewOneTimeDelayedSignAndBroadcastTxClient(t, signAndBroadcastDelay)
+ txClientMock := testtx.NewOneTimeDelayedSignAndBroadcastTxClient(t, ctx, signAndBroadcastDelay)
 
  signingKeyOpt := supplier.WithSigningKeyName(testAppKey.Name)
  deps := depinject.Supply(
@@ -157,6 +162,9 @@ func TestSupplierClient_SubmitProof(t *testing.T) {
  ApplicationAddress: testAppAddr.String(),
  SessionStartBlockHeight: 0,
  SessionId: "",
+ Service: &sharedtypes.Service{
+ Id: testService,
+ },
  }
 
  kvStore, err := smt.NewKVStore("")

pkg/relayer/proxy/synchronous.go

@@ -8,6 +8,7 @@ import (
  "net/url"
 
  sdkerrors "cosmossdk.io/errors"
+
  "github.com/pokt-network/poktroll/pkg/polylog"
  "github.com/pokt-network/poktroll/pkg/relayer"
  "github.com/pokt-network/poktroll/x/service/types"