Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
Use string for RPC message IDs (#3507)
Browse files Browse the repository at this point in the history
- Still an autoincrementing integer, but sent to the server as a string
- This means the server will return it to us as a string, and we don't need to worry about number/bigint upcasting
- This fixes the bug in subscriptions, caused by upcasting the ID to a bigint

<img width="329" alt="Screenshot 2024-10-31 at 16 06 51" src="https://github.com/user-attachments/assets/2fc3ccb8-34a9-449d-9fd9-536ebce29b8b">
<img width="426" alt="Screenshot 2024-10-31 at 16 07 32" src="https://github.com/user-attachments/assets/9e107fdc-dcd2-47d0-91be-8cadd98ec85e">
  • Loading branch information
mcintyre94 authored Oct 31, 2024
1 parent bd1cd50 commit 45df702
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 11 deletions.
7 changes: 7 additions & 0 deletions .changeset/funny-weeks-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@solana/rpc-subscriptions': patch
'@solana/rpc-spec-types': patch
'@solana/rpc-spec': patch
---

Fixed a bug where the subcription server's response would not be detected, because of a mismatch in the format of the `id`. Now all RPC message ids are strings, for avoidance of doubt.
4 changes: 2 additions & 2 deletions packages/rpc-spec-types/src/__tests__/rpc-message-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ describe('createRpcMessage', () => {
const request = { methodName: 'foo', params: 'bar' };
const { id: firstId } = createRpcMessage(request);
const { id: secondId } = createRpcMessage(request);
expect(secondId - firstId).toBe(1);
expect(Number(secondId) - Number(firstId)).toBe(1);
});
it('returns a well-formed JSON-RPC 2.0 message', () => {
const request = { methodName: 'someMethod', params: [1, 2, 3] };
expect(createRpcMessage(request)).toStrictEqual({
id: expect.any(Number),
id: expect.any(String),
jsonrpc: '2.0',
method: 'someMethod',
params: [1, 2, 3],
Expand Down
8 changes: 4 additions & 4 deletions packages/rpc-spec-types/src/rpc-message.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { RpcRequest } from './rpc-request';

let _nextMessageId = 0;
function getNextMessageId() {
let _nextMessageId = 0n;
function getNextMessageId(): string {
const id = _nextMessageId;
_nextMessageId = (_nextMessageId + 1) % Number.MAX_SAFE_INTEGER;
return id;
_nextMessageId++;
return id.toString();
}

export function createRpcMessage<TParams>(request: RpcRequest<TParams>) {
Expand Down
2 changes: 1 addition & 1 deletion packages/rpc-spec-types/src/rpc-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export type RpcResponseTransformer<TResponse = unknown> = {
};

interface IHasIdentifier {
readonly id: number;
readonly id: string;
}

type RpcErrorResponsePayload = Readonly<{
Expand Down
4 changes: 2 additions & 2 deletions packages/rpc-spec/src/__tests__/rpc-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('JSON-RPC 2.0', () => {
.send()
.catch(() => {});
expect(makeHttpRequest).toHaveBeenCalledWith({
payload: { ...createRpcMessage({ methodName: 'someMethod', params: [123] }), id: expect.any(Number) },
payload: { ...createRpcMessage({ methodName: 'someMethod', params: [123] }), id: expect.any(String) },
});
});
it('returns results from the transport', async () => {
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('JSON-RPC 2.0', () => {
expect(makeHttpRequest).toHaveBeenCalledWith({
payload: {
...createRpcMessage({ methodName: 'someMethodAugmented', params: [123, 'augmented', 'params'] }),
id: expect.any(Number),
id: expect.any(String),
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ describe('accountNotifications', () => {
rpcSubscriptions = createLocalhostSolanaRpcSubscriptions();
});

// eslint-disable-next-line jest/no-disabled-tests
it.skip('can subscribe to account notifications', async () => {
it('can subscribe to account notifications', async () => {
expect.hasAssertions();
const abortSignal = new AbortController().signal;
const subscriptionPromise = rpcSubscriptions
Expand Down

0 comments on commit 45df702

Please sign in to comment.