From 29821df246b14eb41dd4606913f44fac40183957 Mon Sep 17 00:00:00 2001
From: Callum McIntyre <callum.mcintyre@anza.xyz>
Date: Thu, 18 Jul 2024 13:23:40 +0100
Subject: [PATCH] RPC: Remove JSON RPC specific behaviour from rpc-spec, move
 into default transformer (#2950)

* Remove JSON RPC specific behaviour from rpc-spec, move into default transformer

- Update the default RPC transformer to unpack the `result` property
- Remove JSON RPC specific result and error fields from rpc-spec
- Add the JSON RPC error handling to the default transformer

* Add changeset
---
 .changeset/metal-bees-lick.md                 |  6 +++++
 packages/rpc-spec/package.json                |  1 -
 packages/rpc-spec/src/__tests__/rpc-test.ts   | 16 +++++--------
 packages/rpc-spec/src/rpc-api.ts              |  2 +-
 packages/rpc-spec/src/rpc.ts                  |  8 +------
 .../src/__tests__/rpc-subscription-test.ts    |  6 ++---
 .../src/rpc-subscriptions.ts                  |  2 +-
 packages/rpc-transformers/package.json        |  5 ++--
 .../__tests__/response-transformer-test.ts    | 24 ++++++++++++++++---
 .../src/response-transformer.ts               | 18 ++++++++++----
 pnpm-lock.yaml                                |  6 ++---
 11 files changed, 59 insertions(+), 35 deletions(-)
 create mode 100644 .changeset/metal-bees-lick.md

diff --git a/.changeset/metal-bees-lick.md b/.changeset/metal-bees-lick.md
new file mode 100644
index 000000000000..40b158dbaf4c
--- /dev/null
+++ b/.changeset/metal-bees-lick.md
@@ -0,0 +1,6 @@
+---
+'@solana/rpc-transformers': patch
+'@solana/rpc-spec': patch
+---
+
+Refactor rpc-spec to remove requirement for transports to implement parts of JSON RPC spec
diff --git a/packages/rpc-spec/package.json b/packages/rpc-spec/package.json
index bb32653eba3b..298606321a04 100644
--- a/packages/rpc-spec/package.json
+++ b/packages/rpc-spec/package.json
@@ -63,7 +63,6 @@
         "maintained node versions"
     ],
     "dependencies": {
-        "@solana/errors": "workspace:*",
         "@solana/rpc-spec-types": "workspace:*"
     },
     "peerDependencies": {
diff --git a/packages/rpc-spec/src/__tests__/rpc-test.ts b/packages/rpc-spec/src/__tests__/rpc-test.ts
index 37f60459afcf..c37b09e5be52 100644
--- a/packages/rpc-spec/src/__tests__/rpc-test.ts
+++ b/packages/rpc-spec/src/__tests__/rpc-test.ts
@@ -1,4 +1,3 @@
-import { SOLANA_ERROR__JSON_RPC__PARSE_ERROR, SolanaError } from '@solana/errors';
 import { createRpcMessage } from '@solana/rpc-spec-types';
 
 import { createRpc, Rpc } from '../rpc';
@@ -35,19 +34,16 @@ describe('JSON-RPC 2.0', () => {
     });
     it('returns results from the transport', async () => {
         expect.assertions(1);
-        (makeHttpRequest as jest.Mock).mockResolvedValueOnce({ result: 123 });
+        (makeHttpRequest as jest.Mock).mockResolvedValueOnce(123);
         const result = await rpc.someMethod().send();
         expect(result).toBe(123);
     });
     it('throws errors from the transport', async () => {
         expect.assertions(1);
-        (makeHttpRequest as jest.Mock).mockResolvedValueOnce({
-            error: { code: SOLANA_ERROR__JSON_RPC__PARSE_ERROR, message: 'o no' },
-        });
+        const transportError = new Error('o no');
+        (makeHttpRequest as jest.Mock).mockRejectedValueOnce(transportError);
         const sendPromise = rpc.someMethod().send();
-        await expect(sendPromise).rejects.toThrow(
-            new SolanaError(SOLANA_ERROR__JSON_RPC__PARSE_ERROR, { __serverMessage: 'o no' }),
-        );
+        await expect(sendPromise).rejects.toThrow(transportError);
     });
     describe('when calling a method having a concrete implementation', () => {
         let rpc: Rpc<TestRpcMethods>;
@@ -94,13 +90,13 @@ describe('JSON-RPC 2.0', () => {
         });
         it('calls the response transformer with the response from the JSON-RPC 2.0 endpoint', async () => {
             expect.assertions(1);
-            (makeHttpRequest as jest.Mock).mockResolvedValueOnce({ result: 123 });
+            (makeHttpRequest as jest.Mock).mockResolvedValueOnce(123);
             await rpc.someMethod().send();
             expect(responseTransformer).toHaveBeenCalledWith(123, 'someMethod');
         });
         it('returns the processed response', async () => {
             expect.assertions(1);
-            (makeHttpRequest as jest.Mock).mockResolvedValueOnce({ result: 123 });
+            (makeHttpRequest as jest.Mock).mockResolvedValueOnce(123);
             const result = await rpc.someMethod().send();
             expect(result).toBe('123 processed response');
         });
diff --git a/packages/rpc-spec/src/rpc-api.ts b/packages/rpc-spec/src/rpc-api.ts
index 4da9c41c87b5..f0ed256a00cb 100644
--- a/packages/rpc-spec/src/rpc-api.ts
+++ b/packages/rpc-spec/src/rpc-api.ts
@@ -4,7 +4,7 @@ import { RpcRequest } from './rpc-request';
 
 export type RpcApiConfig = Readonly<{
     parametersTransformer?: <T extends unknown[]>(params: T, methodName: string) => unknown;
-    responseTransformer?: <T>(response: unknown, methodName: string) => T;
+    responseTransformer?: <T>(response: unknown, methodName?: string) => T;
 }>;
 
 export type RpcApi<TRpcMethods> = {
diff --git a/packages/rpc-spec/src/rpc.ts b/packages/rpc-spec/src/rpc.ts
index 3d44e86ff0d1..dec275b7f66d 100644
--- a/packages/rpc-spec/src/rpc.ts
+++ b/packages/rpc-spec/src/rpc.ts
@@ -1,4 +1,3 @@
-import { getSolanaErrorFromJsonRpcError } from '@solana/errors';
 import {
     Callable,
     createRpcMessage,
@@ -74,12 +73,7 @@ function createPendingRpcRequest<TRpcMethods, TRpcTransport extends RpcTransport
                 payload,
                 signal: options?.abortSignal,
             });
-            if ('error' in response) {
-                throw getSolanaErrorFromJsonRpcError(response.error);
-            }
-            return (
-                responseTransformer ? responseTransformer(response.result, methodName) : response.result
-            ) as TResponse;
+            return (responseTransformer ? responseTransformer(response, methodName) : response) as TResponse;
         },
     };
 }
diff --git a/packages/rpc-subscriptions-spec/src/__tests__/rpc-subscription-test.ts b/packages/rpc-subscriptions-spec/src/__tests__/rpc-subscription-test.ts
index 18857731135e..35f115855b4a 100644
--- a/packages/rpc-subscriptions-spec/src/__tests__/rpc-subscription-test.ts
+++ b/packages/rpc-subscriptions-spec/src/__tests__/rpc-subscription-test.ts
@@ -215,7 +215,7 @@ describe('JSON-RPC 2.0 Subscriptions', () => {
             .thingNotifications()
             .subscribe({ abortSignal: new AbortController().signal });
         const iterator = thingNotifications[Symbol.asyncIterator]();
-        await expect(iterator.next()).resolves.toHaveProperty('value', 456);
+        await expect(iterator.next()).resolves.toHaveProperty('value', { result: 456, subscription: 42 });
     });
     it.each([null, undefined])(
         'fatals when the subscription id returned from the server is `%s`',
@@ -324,7 +324,7 @@ describe('JSON-RPC 2.0 Subscriptions', () => {
         let responseTransformer: jest.Mock;
         let rpc: RpcSubscriptions<TestRpcSubscriptionNotifications>;
         beforeEach(() => {
-            responseTransformer = jest.fn(response => `${response} processed response`);
+            responseTransformer = jest.fn(response => `${response.result} processed response`);
             rpc = createSubscriptionRpc({
                 api: {
                     thingNotifications(...params: unknown[]): RpcSubscriptionsRequest<unknown> {
@@ -349,7 +349,7 @@ describe('JSON-RPC 2.0 Subscriptions', () => {
                 .thingNotifications()
                 .subscribe({ abortSignal: new AbortController().signal });
             await thingNotifications[Symbol.asyncIterator]().next();
-            expect(responseTransformer).toHaveBeenCalledWith(123, 'thingSubscribe');
+            expect(responseTransformer).toHaveBeenCalledWith({ result: 123, subscription: 42 }, 'thingSubscribe');
         });
         it('returns the processed response', async () => {
             expect.assertions(1);
diff --git a/packages/rpc-subscriptions-spec/src/rpc-subscriptions.ts b/packages/rpc-subscriptions-spec/src/rpc-subscriptions.ts
index 96948c0474a3..694fe0df9396 100644
--- a/packages/rpc-subscriptions-spec/src/rpc-subscriptions.ts
+++ b/packages/rpc-subscriptions-spec/src/rpc-subscriptions.ts
@@ -180,7 +180,7 @@ function createPendingRpcSubscription<
                         if (!('params' in message) || message.params.subscription !== subscriptionId) {
                             continue;
                         }
-                        const notification = message.params.result as TNotification;
+                        const notification = message.params as TNotification;
                         yield responseTransformer
                             ? responseTransformer(notification, subscribeMethodName)
                             : notification;
diff --git a/packages/rpc-transformers/package.json b/packages/rpc-transformers/package.json
index bc77be72643c..00bb8702faf8 100644
--- a/packages/rpc-transformers/package.json
+++ b/packages/rpc-transformers/package.json
@@ -63,10 +63,11 @@
         "maintained node versions"
     ],
     "dependencies": {
+        "@solana/errors": "workspace:*",
         "@solana/functional": "workspace:*",
-        "@solana/rpc-types": "workspace:*",
         "@solana/rpc-spec": "workspace:*",
-        "@solana/rpc-subscriptions-spec": "workspace:*"
+        "@solana/rpc-subscriptions-spec": "workspace:*",
+        "@solana/rpc-types": "workspace:*"
     },
     "peerDependencies": {
         "typescript": ">=5"
diff --git a/packages/rpc-transformers/src/__tests__/response-transformer-test.ts b/packages/rpc-transformers/src/__tests__/response-transformer-test.ts
index e454ecdceff5..0e0db01d49bb 100644
--- a/packages/rpc-transformers/src/__tests__/response-transformer-test.ts
+++ b/packages/rpc-transformers/src/__tests__/response-transformer-test.ts
@@ -1,12 +1,15 @@
+import { SOLANA_ERROR__JSON_RPC__PARSE_ERROR, SolanaError } from '@solana/errors';
+
 import { getDefaultResponseTransformerForSolanaRpc } from '../response-transformer';
 import { KEYPATH_WILDCARD } from '../tree-traversal';
 
 describe('getDefaultResponseTransformerForSolanaRpc', () => {
     describe('given an array as input', () => {
         const input = [10, 10n, '10', ['10', [10n, 10], 10]] as const;
+        const response = { result: input };
         it('casts the numbers in the array to a `bigint`, recursively', () => {
             const transformer = getDefaultResponseTransformerForSolanaRpc();
-            expect(transformer(input)).toStrictEqual([
+            expect(transformer(response)).toStrictEqual([
                 BigInt(input[0]),
                 input[1],
                 input[2],
@@ -16,9 +19,11 @@ describe('getDefaultResponseTransformerForSolanaRpc', () => {
     });
     describe('given an object as input', () => {
         const input = { a: 10, b: 10n, c: { c1: '10', c2: 10 } } as const;
+        const response = { result: input };
+
         it('casts the numbers in the object to `bigints`, recursively', () => {
             const transformer = getDefaultResponseTransformerForSolanaRpc();
-            expect(transformer(input)).toStrictEqual({
+            expect(transformer(response)).toStrictEqual({
                 a: BigInt(input.a),
                 b: input.b,
                 c: { c1: input.c.c1, c2: BigInt(input.c.c2) },
@@ -39,8 +44,21 @@ describe('getDefaultResponseTransformerForSolanaRpc', () => {
                 const transformer = getDefaultResponseTransformerForSolanaRpc({
                     allowedNumericKeyPaths: { getFoo: allowedKeyPaths },
                 });
-                expect(transformer(input, 'getFoo')).toStrictEqual(expectation);
+                const response = { result: input };
+                expect(transformer(response, 'getFoo')).toStrictEqual(expectation);
             },
         );
     });
+    describe('given a JSON RPC error as input', () => {
+        const input = {
+            error: { code: SOLANA_ERROR__JSON_RPC__PARSE_ERROR, message: 'o no' },
+        };
+
+        it('throws it as a SolanaError', () => {
+            const transformer = getDefaultResponseTransformerForSolanaRpc();
+            expect(() => transformer(input)).toThrow(
+                new SolanaError(SOLANA_ERROR__JSON_RPC__PARSE_ERROR, { __serverMessage: 'o no' }),
+            );
+        });
+    });
 });
diff --git a/packages/rpc-transformers/src/response-transformer.ts b/packages/rpc-transformers/src/response-transformer.ts
index f4b961ab7d3b..1911c02581e8 100644
--- a/packages/rpc-transformers/src/response-transformer.ts
+++ b/packages/rpc-transformers/src/response-transformer.ts
@@ -1,3 +1,6 @@
+import { getSolanaErrorFromJsonRpcError } from '@solana/errors';
+import { RpcApiConfig } from '@solana/rpc-spec';
+
 import { AllowedNumericKeypaths } from './response-transformer-allowed-numeric-values';
 import { getBigIntUpcastVisitor } from './response-transformer-bigint-upcast';
 import { getTreeWalker } from './tree-traversal';
@@ -6,14 +9,21 @@ export type ResponseTransformerConfig<TApi> = Readonly<{
     allowedNumericKeyPaths?: AllowedNumericKeypaths<TApi>;
 }>;
 
-export function getDefaultResponseTransformerForSolanaRpc<TApi>(config?: ResponseTransformerConfig<TApi>) {
-    return <T>(rawResponse: unknown, methodName?: keyof TApi): T => {
+type JsonRpcResponse = { error: Parameters<typeof getSolanaErrorFromJsonRpcError>[0] } | { result: unknown };
+
+export function getDefaultResponseTransformerForSolanaRpc<TApi>(
+    config?: ResponseTransformerConfig<TApi>,
+): NonNullable<RpcApiConfig['responseTransformer']> {
+    return (<T>(rawResponse: JsonRpcResponse, methodName?: keyof TApi): T => {
+        if ('error' in rawResponse) {
+            throw getSolanaErrorFromJsonRpcError(rawResponse.error);
+        }
         const keyPaths =
             config?.allowedNumericKeyPaths && methodName ? config.allowedNumericKeyPaths[methodName] : undefined;
         const traverse = getTreeWalker([getBigIntUpcastVisitor(keyPaths ?? [])]);
         const initialState = {
             keyPath: [],
         };
-        return traverse(rawResponse, initialState) as T;
-    };
+        return traverse(rawResponse.result, initialState) as T;
+    }) as NonNullable<RpcApiConfig['responseTransformer']>;
 }
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 31bc1e208ba2..c2b47266a05b 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -912,9 +912,6 @@ importers:
 
   packages/rpc-spec:
     dependencies:
-      '@solana/errors':
-        specifier: workspace:*
-        version: link:../errors
       '@solana/rpc-spec-types':
         specifier: workspace:*
         version: link:../rpc-spec-types
@@ -1025,6 +1022,9 @@ importers:
 
   packages/rpc-transformers:
     dependencies:
+      '@solana/errors':
+        specifier: workspace:*
+        version: link:../errors
       '@solana/functional':
         specifier: workspace:*
         version: link:../functional