Refactor: move blocks client (#10)

* refactor: remove redundant application client * refactor: move the BlockClient to top level * remove the redundant clients/block.go * fix spacing in the comment * Refactor: simplify the ShannonSDK struct by droping the HeightClient * address review comments * address review comment Co-authored-by: Daniel Olshansky <[email protected]> * address review comments * update comment on PoktNodeStatusFetcher * Review & update README * Update comment Co-authored-by: Daniel Olshansky <[email protected]> * Update README.md with interface design details * address review comments: s/height/queryHeight * fix: Use deterministic map marshaling * Update README.md Co-authored-by: Daniel Olshansky <[email protected]> * Update README.md Co-authored-by: Daniel Olshansky <[email protected]> --------- Co-authored-by: Daniel Olshansky <[email protected]> Co-authored-by: Redouane Lakrache <[email protected]>
pokt-network · Jun 20, 2024 · d6cb20a · d6cb20a
1 parent c338413
commit d6cb20a
Show file tree

Hide file tree

Showing 9 changed files with 226 additions and 93 deletions.
diff --git a/README.md b/README.md
@@ -17,7 +17,12 @@ to [dev.poktroll.com/category/actors](https://dev.poktroll.com/category/actors).
   - [Get Gateway Delegating Applications](#get-gateway-delegating-applications)
   - [Send Relay](#send-relay)
   - [Helper functions](#helper-functions)
-- [ShannonSDK Internals](#shannonsdk-internals)
+- [ShannonSDK Internals \& Design (for developers only)](#shannonsdk-internals--design-for-developers-only)
+  - [Code Organization](#code-organization)
+    - [Interface Design](#interface-design)
+    - [Exposed Concrete Types](#exposed-concrete-types)
+    - [sdk.go](#sdkgo)
+    - [Public vs Private Fields](#public-vs-private-fields)
   - [Implementation Details](#implementation-details)
   - [Error Handling](#error-handling)
   - [Dependencies implementation](#dependencies-implementation)
@@ -188,7 +193,110 @@ poktHTTPRequest, requestBz, err := sdktypes.SerializeHTTPRequest(request)
 serviceResponse, err := sdktypes.DeserializeHTTPResponse(relayResponse.Payload)
 ```
 
-## ShannonSDK Internals
+## ShannonSDK Internals & Design (for developers only)
+
+### Code Organization
+
+The following is the top-level structure the SDK repo is moving towards:
+
+```bash
+application.go
+block.go
+relay.go
+session.go
+sign.go
+```
+
+_TODO_DOCUMENT: Add the output of `tree -L 2` once the above structure is implemented._
+_TODO_DOCUMENT: Add a mermaid diagram of the exposed types once complete._
+
+#### Interface Design
+
+The `SDK` **IS NOT DESIGNED** to provide interfaces to the consumer.
+
+The `SDK` **IS DESIGNED** to consume functionality from other packages via interfaces.
+
+This follows Golang's best practices for interfaces as described [here](https://go.dev/wiki/CodeReviewComments#interfaces).
+
+As a concrete example, the `Client` struct is exported directly from the `net/http` package
+
+```go
+type Client struct {
+    // Transport specifies the mechanism by which individual
+    // HTTP requests are made.
+    // If nil, DefaultTransport is used.
+    Transport RoundTripper
+```
+
+Note that the above `Client` struct has multiple public methods, yet no code exists to force it to fulfill a specific interface.
+
+As a concrete example of keeping interfaces on the consumer side, the above `Client` consumes a `RoundTripper` interface.
+As the godoc page for `net/http` specifies: `For control over proxies, TLS configuration, keep-alives, compression, and other settings, create a Transport`:
+
+```go
+tr := &http.Transport{
+    MaxIdleConns:       10,
+    IdleConnTimeout:    30 * time.Second,
+    DisableCompression: true,
+}
+client := &http.Client{Transport: tr}
+```
+
+Example of a helper function used for overriding the default `RoundTripper`:
+
+```func NewFileTransport(fs FileSystem) RoundTripper```
+
+#### Exposed Concrete Types
+
+Each file (in the top level directory) will have a client implemented and returned
+as a concrete struct.
+
+For example, `ApplicationClient` is a `struct` that will be returned by `application.go`
+rather than an interface.
+
+#### sdk.go
+
+**NOTE: If you are reading this and the documentation is outdated, please update it!**
+
+The `sdk.go` is a **TEMPORARY** that needs to be split file needs to be split into
+`application.go`, `supplier.go`, etc...
+
+A `ShannonSDK` struct was defined initially but is non-ideal. It forces the
+user/developer to construct the entire struct even if they need a small fraction
+of the functionality.
+
+The following is an example of using a small subset of the SDK:
+
+```go
+session, err := sessionClient.CurrentSession()
+if err != nil {
+   return nil, err
+}
+
+endpoints := sdk.Endpoints(session, serviceID)
+```
+
+#### Public vs Private Fields
+
+The goal of this `SDK` is to make all fields of concrete types public to the user
+if there is a potential need for the user to set them directly.
+
+**IT SHOULD** be possible for the user to initialize any component of the SDK by
+creating a struct and setting the bare minimum necessary fields.
+
+For example, the SDK biases towards the following design:
+
+```go
+c := SessionClient {
+     HttpClient:  myCustomHttpTransport
+}
+```
+
+Instead of the following design:
+
+```go
+c := NewSessionClient(nil, nil, myCustomHttpTransport, nil, nil)
+```
 
 ### Implementation Details
 
@@ -213,4 +321,4 @@ Go, and they can be used as a reference for building more complex ones.
 The SDK relies on the `poktroll` repository for the `types` package, which
 acts as a single source of truth for the data structures used by the SDK.
 This design choice ensures consistency across the various components of the
-POKT ecosystem.
+POKT ecosystem.
diff --git a/block.go b/block.go
@@ -0,0 +1,64 @@
+// Package sdk implements utility functions for interacting with POKT full nodes.
+package sdk
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	ctypes "github.com/cometbft/cometbft/rpc/core/types"
+	cosmos "github.com/cosmos/cosmos-sdk/client"
+)
+
+// TODO_IDEA: The BlockClient could leverage websockets to get notified about new blocks
+//  and cache the latest block height to avoid querying the blockchain for it every time.
+
+// BlockClient is a concrete types used to interact with the on-chain block module.
+// For example, it can be used to get the latest block height.
+//
+// For obtaining the latest height, BlockClient uses a POKT full node's status
+// which contains the latest block height. This is done to avoid fetching the
+// entire latest block just to extract the block height.
+type BlockClient struct {
+	// PoktNodeStatusFetcher specifies the functionality required by the
+	// BlockClient to interact with a POKT full node.
+	PoktNodeStatusFetcher
+}
+
+// LatestBlockHeight returns the height of the latest committed block in the blockchain.
+func (bc *BlockClient) LatestBlockHeight(ctx context.Context) (height int64, err error) {
+	if bc.PoktNodeStatusFetcher == nil {
+		return 0, errors.New("LatestBlockHeight: nil PoktNodeStatusFetcher")
+	}
+
+	nodeStatus, err := bc.PoktNodeStatusFetcher.Status(ctx)
+	if err != nil {
+		return 0, err
+	}
+
+	return nodeStatus.SyncInfo.LatestBlockHeight, nil
+}
+
+// NewPoktNodeStatusFetcher returns the default implementation of the PoktNodeStatusFetcher interface.
+// It connects, through a cometbft RPC HTTP client, to a POKT full node to get its status.
+func NewPoktNodeStatusFetcher(queryNodeRpcUrl string) (PoktNodeStatusFetcher, error) {
+	// TODO_IMPROVE: drop the cosmos dependency and directly use cometbft rpchttp.New, once the latter publishes a release that includes this functionality.
+	// Directly using the cometbft will simplify the code by both reducing imported repos and removing the cosmos wrapper which we don't use.
+	// This can be done once there is a cometbft release that includes the following version: github.com/cometbft/cometbft v1.0.0-alpha.2.0.20240530055211-ae27f7eb3c08
+	statusFetcher, err := cosmos.NewClientFromNode(queryNodeRpcUrl)
+	if err != nil {
+		return nil, fmt.Errorf("error constructing a default POKT full node status fetcher: %w", err)
+	}
+
+	return statusFetcher, nil
+}
+
+// PoktNodeStatusFetcher interface is used by the BlockClient to get the status of a POKT full node.
+// The BlokClient extracts the latest height from this status struct.
+//
+// Most users can rely on the default implementation provided by NewPoktNodeStatusFetcher function.
+// A custom implementation of this interface can be used to gain more granular control over the interactions
+// of the BlockClient with the POKT full node.
+type PoktNodeStatusFetcher interface {
+	Status(ctx context.Context) (*ctypes.ResultStatus, error)
+}
diff --git a/block_test.go b/block_test.go
@@ -0,0 +1,26 @@
+package sdk
+
+import (
+	"context"
+	"fmt"
+)
+
+func ExampleLatestBlockHeight() {
+	poktFullNode, err := NewPoktNodeStatusFetcher("pokt-full-node-URL")
+	if err != nil {
+		fmt.Printf("Erorr creating a connection to POKT full node: %v\n", err)
+		return
+	}
+
+	bc := BlockClient{
+		PoktNodeStatusFetcher: poktFullNode,
+	}
+
+	queryHeight, err := bc.LatestBlockHeight(context.Background())
+	if err != nil {
+		fmt.Printf("Erorr fetching latest block height: %v\n", err)
+		return
+	}
+
+	fmt.Printf("Latest block height: %d\n", queryHeight)
+}
diff --git a/clients/block.go b/clients/block.go
diff --git a/sdk/interface.go b/sdk/interface.go
@@ -33,18 +33,6 @@ type SharedParamsClient interface {
 	GetParams(ctx context.Context) (params *sharedtypes.Params, err error)
 }
 
-// BlockClient is the interface to interact with the on-chain block module.
-//
-// For example, it can be used to get the latest block height.
-//
-// The implementations of this interface could leverage websockets to get notified
-// about new blocks and cache the latest block height to avoid querying the blockchain
-// for it every time.
-type BlockClient interface {
-	// GetLatestBlockHeight returns the height of the latest block.
-	GetLatestBlockHeight(ctx context.Context) (height int64, err error)
-}
-
 // RelayClient is the interface used to send Relays to suppliers.
 //
 // It is transport agnostic and could be implemented using the required protocols.