Repository: celestiaorg/celestia-node
Pull request #404 (2022-01-31 13:52:28)

core: Remove client retry test
Resolves https://github.com/celestiaorg/celestia-node/issues/400


Patch:
diff --git a/docs/adr/adr-004-state-interaction.md b/docs/adr/adr-004-state-interaction.md
new file mode 100644
index 00000000..cbc38bd0
--- /dev/null
+++ b/docs/adr/adr-004-state-interaction.md
@@ -0,0 +1,121 @@
+# ADR #004: Celestia-Node State Interaction for March 2022 Testnet
+
+<hr style="border:3px solid gray"> </hr>
+
+## Authors
+
+@renaynay
+
+## Changelog
+
+* 2022-01-14: initial draft
+
+<hr style="border:2px solid gray"> </hr>
+
+## Context
+
+Currently, celestia-node lacks a way for users to submit a transaction to celestia-core as well as access its own state
+and public state of others.
+
+## Decision
+
+Both celestia **light** and **full** nodes will run `StateService`. 
+`StateService` will be responsible for managing the RPC endpoints provided by `StateAccessor`.
+
+`StateAccessor` will be an interface for interacting with celestia-core in order to retrieve account-related information
+as well as submit transactions.
+
+### `StateService`
+
+`StateService` will expose several higher-level RPC endpoints for users to access the methods provided by the 
+`StateAccessor`.
+
+```go
+type StateService struct {
+    accessor StateAccessor
+}
+``` 
+
+### `StateAccessor`
+
+`StateAccessor` is defined by the two methods listed below and may be expanded in the future to include more methods 
+related to accounts and transactions in later iterations.
+
+```go
+type StateAccessor interface {
+    // AccountBalance returns the Celestia Balance for the given Account
+    func AccountBalance(context.Context, Account) (Balance, error)
+	// SubmitTx submits the given Tx, where Tx implements sdk.Msg
+    func SubmitTx(context.Context, Tx) error
+}
+```
+
+`StateAccessor` will have 3 separate implementations under the hood, the first of which will be outlined in this ADR: 
+1. **CORE**: RPC connection with a celestia-core endpoint handed to the node upon initialisation
+(*required for this iteration*) 
+2. **P2P**: discovery of a **bridge** node or other node that is able to provide state (*nice-to-have for this iteration*)
+3. **LOCAL**: eventually, **full** nodes will be able to provide state to the network, and can therefore execute and 
+respond to the queries without relaying queries to celestia-core (*to be scoped out and implemented in later iterations*)
+
+### 1. Core Implementation of `StateAccessor`: `CoreAccess`
+
+`CoreAccess` will be the direct RPC connection implementation of `StateAccessor`. It will use the [lens implementation of ChainClient](https://github.com/strangelove-ventures/lens/blob/main/client/chain_client.go#L23)
+under the hood. `lens.ChainClient` provides a nice wrapper around the Cosmos-SDK APIs. 
+
+Upon node initialisation, the node will receive the endpoint of a trusted celestia-core node. The constructor for 
+`CoreAccess` will construct a new instance of `CoreAccess` with the `lens.ChainClient` pointing at the given 
+celestia-core endpoint.
+
+```go
+type CoreAccess struct {
+    cc *lens.ChainClient
+}
+
+func NewCoreAccess(cc *lens.ChainClient) (*CoreAccess, error) {
+	return &CoreAccess{
+        cc: cc,		
+    }, nil   
+}
+
+// where Account wraps account information and Balance 
+func (ca *CoreAccess) AccountBalance(ctx context.Context, acct Account) (Balance, error) {
+	coins, err := da.cc.QueryBalanceWithAddress(acct.Address)
+	if err != nil {
+		return err
+    }   
+    // returning first index as we only care for account's balance on celestia chain	
+	return coins[0], nil 
+}
+
+// where Tx implements sdk.Msg
+func (ca *CoreAccess) SubmitTx(ctx context.Context, tx Tx) (TxResponse, error) {
+	return da.cc.SendMsg(ctx, tx)
+}
+```
+
+### 2. P2P Implementation of `StateAccessor`: `P2PAccess`
+
+While it is not necessary to detail how `P2PAccess` will be implemented in this ADR, it will still conform to the 
+`StateAccessor` interface, but instead of being provided a core endpoint to connect to via RPC and using `lens.ChainClient`
+for state-related queries, `P2PAccess` will perform service discovery of state-providing nodes in the network and perform
+the state queries via libp2p streams. More details of the p2p implementation will be described in a separate dedicated ADR.
+
+```go
+type P2PAccess struct {
+    // for discovery and communication
+    // with state-providing nodes
+    host host.Host
+    // for managing keys
+    keybase        keyring.Keyring
+    keyringOptions []keyring.Option
+}
+```
+
+#### `StateProvider`
+
+A **bridge** node will run a `StateProvider` (server-side of `P2PAccessor`). The `StateProvider` will be responsible for
+relaying the state-related queries through to its trusted celestia-core node.
+
+The `StateProvider` will be initialised with a `lens.ChainClient` that will be used under the hood for communication 
+with its already-configured celestia-core RPC connection. It will listen for inbound state-related queries from its 
+peers and relay the received payloads to celestia-core.


Test patch:
diff --git a/core/client_test.go b/core/client_test.go
index c67f35de..c6651fc1 100644
--- a/core/client_test.go
+++ b/core/client_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -103,21 +102,3 @@ func TestRemoteClient_StartBlockSubscription_And_GetBlock(t *testing.T) {
 	require.NoError(t, client.Stop())
 	require.NoError(t, remote.Stop())
 }
-
-// TestRemoteClient_RetryDial ensures 3 additional attempts
-// are made to dial the remote node.
-func TestRemoteClient_RetryDial(t *testing.T) {
-	remote := StartMockNode(CreateKvStore(defaultRetainBlocks))
-	protocol, ip := getRemoteEndpoint(remote)
-
-	// deliberately stop remote
-	err := remote.Stop()
-	require.NoError(t, err)
-
-	client, err := NewRemote(protocol, ip)
-	require.NoError(t, err)
-
-	_, err = client.Block(context.Background(), nil)
-	require.Error(t, err)
-	assert.ErrorContains(t, err, "giving up after 3 attempt(s)")
-}
diff --git a/ipld/read_test.go b/ipld/read_test.go
index f935ca28..9b4d60a6 100644
--- a/ipld/read_test.go
+++ b/ipld/read_test.go
@@ -250,7 +250,7 @@ func TestGetLeavesByNamespace(t *testing.T) {
 
 			// choose random nID from rand shares
 			expected := tt.rawData[len(tt.rawData)/2]
-			nID := expected[:8]
+			nID := expected[:NamespaceSize]
 
 			// change rawData to contain several shares with same nID
 			tt.rawData[(len(tt.rawData)/2)+1] = expected
@@ -275,13 +275,132 @@ func TestGetLeavesByNamespace(t *testing.T) {
 				for _, node := range nodes {
 					// TODO @renaynay: nID is prepended twice for some reason.
 					share := node.RawData()[1:]
-					assert.Equal(t, expected, share[8:])
+					assert.Equal(t, expected, share[NamespaceSize:])
 				}
 			}
 		})
 	}
 }
 
+func TestGetLeavesByNamespace_AbsentNamespaceId(t *testing.T) {
+	rawData := RandNamespacedShares(t, 16).Raw()
+
+	minNid := make([]byte, NamespaceSize)
+	midNid := make([]byte, NamespaceSize)
+	maxNid := make([]byte, NamespaceSize)
+
+	numberOfShares := len(rawData)
+
+	copy(minNid, rawData[0][:NamespaceSize])
+	copy(maxNid, rawData[numberOfShares-1][:NamespaceSize])
+	copy(midNid, rawData[numberOfShares/2][:NamespaceSize])
+
+	// create min nid missing data by replacing first namespace id with second
+	minNidMissingData := make([][]byte, len(rawData))
+	copy(minNidMissingData, rawData)
+	copy(minNidMissingData[0][:NamespaceSize], rawData[1][:NamespaceSize])
+
+	// create max nid missing data by replacing last namespace id with second last
+	maxNidMissingData := make([][]byte, len(rawData))
+	copy(maxNidMissingData, rawData)
+	copy(maxNidMissingData[numberOfShares-1][:NamespaceSize], rawData[numberOfShares-2][:NamespaceSize])
+
+	// create mid nid missing data by replacing middle namespace id with the one after
+	midNidMissingData := make([][]byte, len(rawData))
+	copy(midNidMissingData, rawData)
+	copy(midNidMissingData[numberOfShares/2][:NamespaceSize], rawData[(numberOfShares/2)+1][:NamespaceSize])
+
+	var tests = []struct {
+		name       string
+		data       [][]byte
+		missingNid []byte
+	}{
+		{name: "Namespace id less than the minimum namespace in data", data: minNidMissingData, missingNid: minNid},
+		{name: "Namespace id greater than the maximum namespace in data", data: maxNidMissingData, missingNid: maxNid},
+		{name: "Namespace id in range but still missing", data: midNidMissingData, missingNid: midNid},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dag, dah := putErasuredDataToDag(t, tt.data)
+
+			assertNoRowContainsNID(t, dag, dah, tt.missingNid)
+		})
+	}
+}
+
+func TestGetLeavesByNamespace_MultipleRowsContainingSameNamespaceId(t *testing.T) {
+	t.Run("Same namespace id across multiple rows", func(t *testing.T) {
+		rawData := RandNamespacedShares(t, 16).Raw()
+
+		// set all shares to the same namespace and data but the last one
+		nid := rawData[0][:NamespaceSize]
+		commonNamespaceData := rawData[0]
+
+		for i, nspace := range rawData {
+			if i == len(rawData)-1 {
+				break
+			}
+
+			copy(nspace, commonNamespaceData)
+		}
+
+		dag, dah := putErasuredDataToDag(t, rawData)
+
+		rowRootCids, err := rowRootsByNamespaceID(nid, &dah)
+		require.NoError(t, err)
+
+		for _, rowRootCid := range rowRootCids {
+			nodes, err := GetLeavesByNamespace(context.Background(), dag, rowRootCid, nid)
+			require.NoError(t, err)
+
+			for _, node := range nodes {
+				// test that the data returned by GetLeavesByNamespace for nid
+				// matches the commonNamespaceData that was copied across almost all data
+				share := node.RawData()[1:]
+				assert.Equal(t, commonNamespaceData, share[NamespaceSize:])
+			}
+		}
+	})
+
+}
+
+func putErasuredDataToDag(t *testing.T, rawData [][]byte) (format.DAGService, da.DataAvailabilityHeader) {
+	// calc square size
+	squareSize := uint64(math.Sqrt(float64(len(rawData))))
+
+	// generate DAH
+	eds, err := da.ExtendShares(squareSize, rawData)
+	require.NoError(t, err)
+	dah := da.NewDataAvailabilityHeader(eds)
+
+	// put raw data in DAG
+	dag := mdutils.Mock()
+	_, err = PutData(context.Background(), rawData, dag)
+	require.NoError(t, err)
+
+	return dag, dah
+}
+
+func assertNoRowContainsNID(
+	t *testing.T,
+	dag format.DAGService,
+	dah da.DataAvailabilityHeader,
+	nID namespace.ID,
+) {
+	// get all row root cids
+	rowRootCIDs := make([]cid.Cid, len(dah.RowsRoots))
+	for i, rowRoot := range dah.RowsRoots {
+		rowRootCIDs[i] = plugin.MustCidFromNamespacedSha256(rowRoot)
+	}
+
+	// for each row root cid check if the minNID exists
+	for _, rowCID := range rowRootCIDs {
+		_, err := GetLeavesByNamespace(context.Background(), dag, rowCID, nID)
+		assert.Equal(t, ErrNotFoundInRange, err)
+	}
+}
+
 // rowRootsByNamespaceID is a convenience method that finds the row root(s)
 // that contain the given namespace ID.
 func rowRootsByNamespaceID(nID namespace.ID, dah *da.DataAvailabilityHeader) ([]cid.Cid, error) {