Skip to content

Commit

Permalink
Make RPC transports responsible for preparing payloads
Browse files Browse the repository at this point in the history
  • Loading branch information
lorisleiva committed Sep 3, 2024
1 parent efdef44 commit dd8d6b7
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 145 deletions.
7 changes: 7 additions & 0 deletions .changeset/orange-yaks-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@solana/rpc-transport-http': patch
'@solana/rpc-spec': patch
'@solana/rpc': patch
---

Make RPC transports responsible for preparing RPC payloads from requests
13 changes: 5 additions & 8 deletions packages/rpc-spec/src/__tests__/rpc-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { createRpcMessage } from '@solana/rpc-spec-types';

import { createRpc, Rpc } from '../rpc';
import { RpcApi, RpcApiRequestPlan } from '../rpc-api';
import { RpcTransport } from '../rpc-transport';
Expand Down Expand Up @@ -28,7 +26,8 @@ describe('JSON-RPC 2.0', () => {
it('sends a request to the transport', () => {
rpc.someMethod(123).send();
expect(makeHttpRequest).toHaveBeenCalledWith({
payload: { ...createRpcMessage('someMethod', [123]), id: expect.any(Number) },
methodName: 'someMethod',
params: [123],
});
});
it('returns results from the transport', async () => {
Expand Down Expand Up @@ -59,13 +58,11 @@ describe('JSON-RPC 2.0', () => {
transport: makeHttpRequest,
});
});
it('converts the returned request to a JSON-RPC 2.0 message and sends it to the transport', () => {
it('passes the method name and parameters to the transport', () => {
rpc.someMethod(123).send();
expect(makeHttpRequest).toHaveBeenCalledWith({
payload: {
...createRpcMessage('someMethodAugmented', [123, 'augmented', 'params']),
id: expect.any(Number),
},
methodName: 'someMethodAugmented',
params: [123, 'augmented', 'params'],
});
});
});
Expand Down
9 changes: 4 additions & 5 deletions packages/rpc-spec/src/rpc-transport.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { RpcResponse } from './rpc-shared';
import { RpcRequest, RpcResponse } from './rpc-shared';

type RpcTransportRequest = Readonly<{
payload: unknown;
signal?: AbortSignal;
}>;
export type RpcTransportRequest<TParams = unknown> = RpcRequest<TParams> & {
readonly signal?: AbortSignal;
};

export type RpcTransport = {
<TResponse>(request: RpcTransportRequest): Promise<RpcResponse<TResponse>>;
Expand Down
24 changes: 11 additions & 13 deletions packages/rpc-spec/src/rpc.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
import {
Callable,
createRpcMessage,
Flatten,
OverloadImplementations,
UnionToIntersection,
} from '@solana/rpc-spec-types';
import { Callable, Flatten, OverloadImplementations, UnionToIntersection } from '@solana/rpc-spec-types';

import { RpcApi, RpcApiRequestPlan } from './rpc-api';
import { RpcTransport } from './rpc-transport';
Expand Down Expand Up @@ -74,12 +68,16 @@ function createPendingRpcRequest<TRpcMethods, TRpcTransport extends RpcTransport
return {
async send(options?: RpcSendOptions): Promise<TResponse> {
const { methodName, params, responseTransformer } = pendingRequest;
const request = Object.freeze({ methodName, params });
const rawResponse = await rpcConfig.transport<TResponse>({
payload: createRpcMessage(methodName, params),
signal: options?.abortSignal,
});
return responseTransformer ? responseTransformer(rawResponse, request) : rawResponse;
const rawResponse = await rpcConfig.transport<TResponse>(
Object.freeze({
methodName,
params,
signal: options?.abortSignal,
}),
);
return responseTransformer
? responseTransformer(rawResponse, Object.freeze({ methodName, params }))
: rawResponse;
},
};
}
12 changes: 4 additions & 8 deletions packages/rpc-transport-http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import { createHttpTransport } from '@solana/rpc-transport-http';

const transport = createHttpTransport({ url: 'https://api.mainnet-beta.solana.com' });
const response = await transport({
payload: { id: 1, jsonrpc: '2.0', method: 'getSlot' },
methodName: 'getSlot',
params: [],
});
const data = await response.json();
```
Expand Down Expand Up @@ -66,16 +67,11 @@ const transport = createHttpTransport({
dispatcher_NODE_ONLY: dispatcher,
url: 'https://mypool',
});
let id = 0;
const balances = await Promise.allSettled(
accounts.map(async account => {
const response = await transport({
payload: {
id: ++id,
jsonrpc: '2.0',
method: 'getBalance',
params: [account],
},
methodName: 'getBalance',
params: [account],
});
return await response.json();
}),
Expand Down
1 change: 1 addition & 0 deletions packages/rpc-transport-http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"dependencies": {
"@solana/errors": "workspace:*",
"@solana/rpc-spec": "workspace:*",
"@solana/rpc-spec-types": "workspace:*",
"undici-types": "^6.19.8"
},
"devDependencies": {
Expand Down
12 changes: 4 additions & 8 deletions packages/rpc-transport-http/src/__benchmarks__/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@ function createDispatcher(options: Agent.Options) {
});
}

let id = 0;
function getTestPayload() {
function getTestRequest() {
return {
id: ++id,
jsonrpc: '2.0',
method: 'getLatestBlockhash',
methodName: 'getLatestBlockhash',
params: [],
};
}

Expand All @@ -53,9 +51,7 @@ async function makeConcurrentRequests(num: number = NUM_CONCURRENT_REQUESTS) {
createHttpTransport({
dispatcher_NODE_ONLY: dispatcher,
url: VALIDATOR_URL,
})({
payload: getTestPayload(),
}),
})(getTestRequest()),
),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('createHttpTransport and `AbortSignal`', () => {
describe('when invoked with an already-aborted `AbortSignal`', () => {
it('rejects with an `AbortError` when no reason is specified', async () => {
expect.assertions(3);
const sendPromise = makeHttpRequest({ payload: 123, signal: AbortSignal.abort() });
const sendPromise = makeHttpRequest({ methodName: 'foo', params: 123, signal: AbortSignal.abort() });
await expect(sendPromise).rejects.toThrow();
await expect(sendPromise).rejects.toBeInstanceOf(DOMException);
await expect(sendPromise).rejects.toHaveProperty('name', 'AbortError');
Expand All @@ -21,7 +21,8 @@ describe('createHttpTransport and `AbortSignal`', () => {
it("rejects with the `AbortSignal's` reason", async () => {
expect.assertions(1);
const sendPromise = makeHttpRequest({
payload: 123,
methodName: 'foo',
params: 123,
signal: AbortSignal.abort('Already aborted'),
});
await expect(sendPromise).rejects.toBe('Already aborted');
Expand All @@ -37,7 +38,7 @@ describe('createHttpTransport and `AbortSignal`', () => {
});
it('rejects with an `AbortError` when no reason is specified', async () => {
expect.assertions(1);
const sendPromise = makeHttpRequest({ payload: 123, signal: abortSignal });
const sendPromise = makeHttpRequest({ methodName: 'foo', params: 123, signal: abortSignal });
abortController.abort();
await expect(sendPromise).rejects.toThrow();
});
Expand All @@ -46,7 +47,7 @@ describe('createHttpTransport and `AbortSignal`', () => {
if (!__BROWSER__) {
it("rejects with with the `AbortSignal's` reason", async () => {
expect.assertions(1);
const sendPromise = makeHttpRequest({ payload: 123, signal: abortSignal });
const sendPromise = makeHttpRequest({ methodName: 'foo', params: 123, signal: abortSignal });
abortController.abort('I got bored waiting');
await expect(sendPromise).rejects.toBe('I got bored waiting');
});
Expand Down Expand Up @@ -74,7 +75,7 @@ describe('createHttpTransport and `AbortSignal`', () => {
json: () => Promise.resolve({ ok: true }),
ok: true,
} as unknown as Response);
const sendPromise = makeHttpRequest({ payload: 123, signal: abortSignal });
const sendPromise = makeHttpRequest({ methodName: 'foo', params: 123, signal: abortSignal });
abortController.abort('I got bored waiting');
await expect(sendPromise).resolves.toMatchObject({ ok: true });
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { SOLANA_ERROR__RPC__TRANSPORT_HTTP_HEADER_FORBIDDEN, SolanaError } from '@solana/errors';
import { RpcTransport } from '@solana/rpc-spec';
import { createRpcMessage } from '@solana/rpc-spec-types';

import { assertIsAllowedHttpRequestHeaders } from '../http-transport-headers';

Expand Down Expand Up @@ -81,7 +82,7 @@ describe('createHttpRequest with custom headers', () => {
headers: { aCcEpT: 'text/html' },
url: 'http://localhost',
});
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
Expand All @@ -96,12 +97,13 @@ describe('createHttpRequest with custom headers', () => {
headers: { 'cOnTeNt-LeNgTh': '420' },
url: 'http://localhost',
});
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
const expectedContentLength = JSON.stringify(createRpcMessage('foo', 123)).length;
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
headers: expect.objectContaining({
'content-length': '3',
'content-length': expectedContentLength.toString(),
}),
}),
);
Expand All @@ -111,7 +113,7 @@ describe('createHttpRequest with custom headers', () => {
headers: { 'cOnTeNt-TyPe': 'text/html' },
url: 'http://localhost',
});
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
Expand Down
30 changes: 18 additions & 12 deletions packages/rpc-transport-http/src/__tests__/http-transport-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, SolanaError } from '@solana/errors';
import { RpcTransport } from '@solana/rpc-spec';
import { createRpcMessage } from '@solana/rpc-spec-types';

// HACK: Pierce the veil of `jest.isolateModules` so that the modules inside get the same version of
// `@solana/errors` that is imported above.
Expand Down Expand Up @@ -28,7 +29,7 @@ describe('createHttpTransport', () => {
});
it('throws HTTP errors', async () => {
expect.assertions(1);
const requestPromise = makeHttpRequest({ payload: 123 });
const requestPromise = makeHttpRequest({ methodName: 'foo', params: 123 });
await expect(requestPromise).rejects.toThrow(
new SolanaError(SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, {
message: 'We looked everywhere',
Expand All @@ -43,7 +44,9 @@ describe('createHttpTransport', () => {
});
it('passes the exception through', async () => {
expect.assertions(1);
await expect(makeHttpRequest({ payload: 123 })).rejects.toThrow(new TypeError('Failed to fetch'));
await expect(makeHttpRequest({ methodName: 'foo', params: 123 })).rejects.toThrow(
new TypeError('Failed to fetch'),
);
});
});
describe('when the endpoint returns a well-formed JSON response', () => {
Expand All @@ -54,20 +57,20 @@ describe('createHttpTransport', () => {
});
});
it('calls fetch with the specified URL', () => {
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
expect(fetchSpy).toHaveBeenCalledWith('http://localhost', expect.anything());
});
it('sets the `body` to a stringfied version of the payload', () => {
makeHttpRequest({ payload: { ok: true } });
makeHttpRequest({ methodName: 'foo', params: { ok: true } });
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
body: JSON.stringify({ ok: true }),
body: JSON.stringify(createRpcMessage('foo', { ok: true })),
}),
);
});
it('sets the accept header to `application/json`', () => {
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
Expand All @@ -78,7 +81,7 @@ describe('createHttpTransport', () => {
);
});
it('sets the content type header to `application/json; charset=utf-8`', () => {
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
Expand All @@ -89,8 +92,9 @@ describe('createHttpTransport', () => {
);
});
it('sets the content length header to the length of the JSON-stringified payload', () => {
makeHttpRequest({
payload:
const request = {
methodName: 'foo',
params:
// Shruggie: https://emojipedia.org/person-shrugging/
'\xAF\\\x5F\x28\u30C4\x29\x5F\x2F\xAF' +
' ' +
Expand All @@ -99,18 +103,20 @@ describe('createHttpTransport', () => {
' ' +
// https://tinyurl.com/bdemuf3r
'\u{1F469}\u{1F3FB}\u200D\u2764\uFE0F\u200D\u{1F469}\u{1F3FF}',
});
};
makeHttpRequest(request);
const expectedContentLength = JSON.stringify(createRpcMessage(request.methodName, request.params)).length;
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
headers: expect.objectContaining({
'content-length': '30',
'content-length': expectedContentLength.toString(),
}),
}),
);
});
it('sets the `method` to `POST`', () => {
makeHttpRequest({ payload: 123 });
makeHttpRequest({ methodName: 'foo', params: 123 });
expect(fetchSpy).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
Expand Down
5 changes: 4 additions & 1 deletion packages/rpc-transport-http/src/http-transport.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { SOLANA_ERROR__RPC__TRANSPORT_HTTP_ERROR, SolanaError } from '@solana/errors';
import { RpcResponse, RpcTransport } from '@solana/rpc-spec';
import { createRpcMessage } from '@solana/rpc-spec-types';
import type Dispatcher from 'undici-types/dispatcher';

import {
Expand Down Expand Up @@ -42,9 +43,11 @@ export function createHttpTransport(config: Config): RpcTransport {
}
const customHeaders = headers && normalizeHeaders(headers);
return async function makeHttpRequest<TResponse>({
payload,
methodName,
params,
signal,
}: Parameters<RpcTransport>[0]): Promise<RpcResponse<TResponse>> {
const payload = createRpcMessage(methodName, params);
const body = JSON.stringify(payload);
const requestInfo = {
...dispatcherConfig,
Expand Down
Loading

0 comments on commit dd8d6b7

Please sign in to comment.