Skip to content

[experimental] A types-only RPC implementation using JavaScript proxies #1190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 9, 2023
Merged

[experimental] A types-only RPC implementation using JavaScript proxies #1190

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0bc8528
Boilerplate code for `@solana/rpc-core`
steveluscher Mar 9, 2023
e3f892d
Type definition for two simple RPC methods: `getBlockHeight` and `get…
steveluscher Mar 9, 2023
b2a8a82
Implementation of the RPC using JavaScript proxies
steveluscher Mar 9, 2023
b87de1a
Type definition of a more complicated RPC method, `getAccountInfo`
steveluscher Mar 9, 2023
1cd7604
Rename `Finality` to `Commitment`
steveluscher Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/rpc-core/src/types/jsonRpcApi.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { GetBlockHeightApi } from './rpc-methods/getBlockHeight';
import { GetBlocksApi } from './rpc-methods/getBlocks';

declare interface JsonRpcApi extends GetBlockHeightApi, GetBlocksApi {}
6 changes: 6 additions & 0 deletions packages/rpc-core/src/types/rpc-methods/common.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// TODO: Eventually move this into whatever package implements transactions
declare type Finality = 'confirmed' | 'finalized' | 'processed';
Copy link
Contributor

@joncinque joncinque Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is Finality the new term? If so, we should probably update the docs and all that. I'd prefer to keep this Commitment to make mental mapping easier, but I won't put up a big fight about it.

Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh. I thought these three were what we're calling finality, as opposed to the olde deprecated suite of commitments.

From web3.js today:

https://github.com/solana-labs/solana-web3.js/blob/ad23683e8a42c726995cf0c1f0f903b20152854f/packages/library-legacy/src/connection.ts#L476-L501

So yeah, I have this wrong, somehow. I'll rename it.


declare type Slot =
// TODO(solana-labs/solana/issues/30341) Represent as bigint
number;
Comment on lines +9 to +11
Copy link
Contributor

@joncinque joncinque Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we've talked about this before, so pardon asking it again here... with the BYO-Transport model, is it possible to require that transports properly handle u64s and make this a bigint?

Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, fast forward to when everything is bigint (or rewind, and have represented all u64s as JavaScript strings in the first place) and all that the transport has to do is to serialize bigints as string over the wire.

We can't do this today, because the Solana RPC doesn't accept them.

# params is normally [5, 10] but here I'm representing them as ["5", "10"]
curl https://api.mainnet-beta.solana.com -X POST -H "Content-Type: application/json" -d '
  {
    "jsonrpc": "2.0", "id": 1,
    "method": "getBlocks",
    "params": ["5", "10"]
  }
'
{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params: invalid type: string \"5\", expected u64."},"id":1}

We need to change the server to parse strings as u64 before we can do that on the client.

Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I see what you're saying. You're saying: make it a bigint now at the outer part of the interface, and have the transport (temporarily) downcast that to a JavaScript number, maybe optionally warning when it overflows. Is that right?

Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this!

Copy link
Contributor

@joncinque joncinque Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering it if it would be possible at the transport level to use bigint everywhere, but I forgot that the problem is at the RPC level, and not web3.js. But yeah, since all of these interfaces are new, it might be worth doing everything right and use bigints everywhere.

20 changes: 20 additions & 0 deletions packages/rpc-core/src/types/rpc-methods/getBlockHeight.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { IJsonRpcTransport } from '@solana/rpc-transport';

type GetBlockHeightApiResponse =
// TODO(solana-labs/solana/issues/30341) Represent as bigint
number;

declare interface GetBlockHeightApi {
/**
* Returns the current block height of the node
*/
getBlockHeight(
transport: IJsonRpcTransport,
config?: readonly {
// Defaults to `finalized`
commitment?: Finality;
// The minimum slot that the request can be evaluated at
minContextSlot?: Slot;
}
Copy link
Contributor

@joncinque joncinque Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these levels of optionals... I can swee why graphql would make this so much neater

): Promise<GetBlockHeightApiResponse>;
Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://docs.solana.com/api/http#getblockheight.

}
18 changes: 18 additions & 0 deletions packages/rpc-core/src/types/rpc-methods/getBlocks.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { IJsonRpcTransport } from '@solana/rpc-transport';

type GetBlocksApiResponse = Slot[];

declare interface GetBlocksApi {
/**
* Returns a list of confirmed blocks between two slots
*/
getBlocks(
transport: IJsonRpcTransport,
startSlot: Slot,
endSlotInclusive?: Slot,
config?: readonly {
// Defaults to `finalized`
commitment?: Exclude<Finality, 'processed'>;
}
): Promise<GetBlocksApiResponse>;
Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://docs.solana.com/api/http#getblocks.

Copy link
Contributor

@joncinque joncinque Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the docs are wrong on this one :-\

Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed them! I don't know why the change hasn't deployed yet. https://github.com/solana-labs/solana/pull/30351/files

Copy link
Contributor Author

@steveluscher steveluscher Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah… https://discord.com/channels/428295358100013066/966782389839876096/1083467864650039347

Copy link
Contributor

@joncinque joncinque Mar 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes. Anyway, thanks for fixing!

}