Skip to content

Commit

Permalink
Add new fields to AddChainOptions to configure JSON-RPC limits (#694)
Browse files Browse the repository at this point in the history
* Add new fields to AddChainOptions to configure JSON-RPC limits

* `jsonRpcMaxSubscriptions == 0` is valid

* Fix overflow in debug_assert

* Rustfmt

* PR link
  • Loading branch information
tomaka authored Jun 8, 2023
1 parent 48adb01 commit cf21110
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 23 deletions.
13 changes: 9 additions & 4 deletions lib/src/json_rpc/requests_subscriptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,18 @@ impl<TSubMsg: Send + Sync + 'static> RequestsSubscriptions<TSubMsg> {
total_requests_in_fly: AtomicUsize::new(0),
guarded: Mutex::new(ClientInnerGuarded {
pending_requests: hashbrown::HashSet::with_capacity_and_hasher(
self.max_requests_per_client,
cmp::min(self.max_requests_per_client, 64),
Default::default(),
),
responses_send_back: VecDeque::with_capacity(self.max_requests_per_client),
responses_send_back: VecDeque::with_capacity(cmp::min(
self.max_requests_per_client,
64,
)),
notification_messages: BTreeMap::new(),
responses_send_back_pushed_or_dead: event_listener::Event::new(),
notification_messages_popped_or_dead: event_listener::Event::new(),
active_subscriptions: hashbrown::HashMap::with_capacity_and_hasher(
self.max_subscriptions_per_client,
cmp::min(self.max_subscriptions_per_client, 128),
Default::default(),
),
num_inactive_alive_subscriptions: 0,
Expand Down Expand Up @@ -400,7 +403,9 @@ impl<TSubMsg: Send + Sync + 'static> RequestsSubscriptions<TSubMsg> {

debug_assert!(
guarded_lock.responses_send_back.len()
<= self.max_requests_per_client + guarded_lock.notification_messages.len(),
<= self
.max_requests_per_client
.saturating_add(guarded_lock.notification_messages.len()),
);

match guarded_lock.responses_send_back.pop_front() {
Expand Down
1 change: 1 addition & 0 deletions wasm-node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Add support for child tries, meaning that errors will no longer be returned when performing runtime calls on chains that use child tries. In practice, this typically concerns contracts chains. ([#680](https://github.com/smol-dot/smoldot/pull/680), [#684](https://github.com/smol-dot/smoldot/pull/684))
- The checksum of the SS58 address passed to the `system_accountNextIndex` JSON-RPC function is now verified. Note that its prefix isn't compared against the one of the current chain, because there is no straight-forward way for smoldot to determine the SS58 prefix of the chain that it is running. ([#691](https://github.com/smol-dot/smoldot/pull/691))
- Add `AddChainOptions.jsonRpcMaxPendingRequests` and `AddChainOptions.jsonRpcMaxSubscriptions`, allowing you to specify the maximum number of pending JSON-RPC requests and the maximum number of active JSON-RPC subscriptions. These two limits were previously implicitly set to respectively 128 and 1024. Not passing any value defaults to "infinity". You are strongly encouraged to specify a value for these two options if the source of the JSON-RPC requests is untrusted. ([#694](https://github.com/smol-dot/smoldot/pull/694))

### Fixed

Expand Down
24 changes: 23 additions & 1 deletion wasm-node/javascript/src/internals/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,13 +491,35 @@ export function start(options: ClientOptions, wasmModule: SmoldotBytecode | Prom
}
}

// Sanitize `jsonRpcMaxPendingRequests`.
let jsonRpcMaxPendingRequests = options.jsonRpcMaxPendingRequests === undefined ? Infinity : options.jsonRpcMaxPendingRequests;
jsonRpcMaxPendingRequests = Math.floor(jsonRpcMaxPendingRequests);
if (jsonRpcMaxPendingRequests <= 0 || isNaN(jsonRpcMaxPendingRequests)) {
throw new AddChainError("Invalid value for `jsonRpcMaxPendingRequests`");
}
if (jsonRpcMaxPendingRequests > 0xffffffff) {
jsonRpcMaxPendingRequests = 0xffffffff
}

// Sanitize `jsonRpcMaxSubscriptions`.
let jsonRpcMaxSubscriptions = options.jsonRpcMaxSubscriptions === undefined ? Infinity : options.jsonRpcMaxSubscriptions;
jsonRpcMaxSubscriptions = Math.floor(jsonRpcMaxSubscriptions);
if (jsonRpcMaxSubscriptions < 0 || isNaN(jsonRpcMaxSubscriptions)) {
throw new AddChainError("Invalid value for `jsonRpcMaxSubscriptions`");
}
if (jsonRpcMaxSubscriptions > 0xffffffff) {
jsonRpcMaxSubscriptions = 0xffffffff
}

const promise = new Promise<{ success: true, chainId: number } | { success: false, error: string }>((resolve) => state.addChainResults.push(resolve));

state.instance.instance.addChain(
options.chainSpec,
typeof options.databaseContent === 'string' ? options.databaseContent : "",
potentialRelayChainsIds,
!!options.disableJsonRpc
!!options.disableJsonRpc,
jsonRpcMaxPendingRequests,
jsonRpcMaxSubscriptions
);

const outcome = await promise;
Expand Down
14 changes: 10 additions & 4 deletions wasm-node/javascript/src/internals/local-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export type ParsedMultiaddr =
export interface Instance {
request: (request: string, chainId: number) => number,
peekJsonRpcResponse: (chainId: number) => string | null,
addChain: (chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean) => void,
addChain: (chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean, jsonRpcMaxPendingRequests: number, jsonRpcMaxSubscriptions: number) => void,
removeChain: (chainId: number) => void,
/**
* Notifies the background executor that it should stop. Once it has effectively stopped,
Expand Down Expand Up @@ -579,12 +579,18 @@ export async function startLocalInstance(config: Config, wasmModule: WebAssembly
}
},

addChain: (chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean) => {
addChain: (chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean, jsonRpcMaxPendingRequests: number, jsonRpcMaxSubscriptions: number) => {
if (!state.instance) {
eventCallback({ ty: "add-chain-result", success: false, error: "Smoldot has crashed" });
return;
}

// The caller is supposed to avoid this situation.
console.assert(
disableJsonRpc || jsonRpcMaxPendingRequests != 0,
"invalid jsonRpcMaxPendingRequests value passed to local-instance::addChain"
);

// `add_chain` unconditionally allocates a chain id. If an error occurs, however, this chain
// id will refer to an *erroneous* chain. `chain_is_ok` is used below to determine whether it
// has succeeeded or not.
Expand All @@ -595,7 +601,7 @@ export async function startLocalInstance(config: Config, wasmModule: WebAssembly
buffer.writeUInt32LE(potentialRelayChainsEncoded, idx * 4, potentialRelayChains[idx]!);
}
state.bufferIndices[2] = potentialRelayChainsEncoded
const chainId = state.instance.exports.add_chain(0, 1, disableJsonRpc ? 0 : 1, 2);
const chainId = state.instance.exports.add_chain(0, 1, disableJsonRpc ? 0 : jsonRpcMaxPendingRequests, jsonRpcMaxSubscriptions, 2);

delete state.bufferIndices[0]
delete state.bufferIndices[1]
Expand Down Expand Up @@ -702,7 +708,7 @@ interface SmoldotWasmExports extends WebAssembly.Exports {
memory: WebAssembly.Memory,
init: (maxLogLevel: number) => void,
advance_execution: () => void,
add_chain: (chainSpecBufferIndex: number, databaseContentBufferIndex: number, jsonRpcRunning: number, potentialRelayChainsBufferIndex: number) => number;
add_chain: (chainSpecBufferIndex: number, databaseContentBufferIndex: number, jsonRpcMaxPendingRequests: number, jsonRpcMaxSubscriptions: number, potentialRelayChainsBufferIndex: number) => number;
remove_chain: (chainId: number) => void,
chain_is_ok: (chainId: number) => number,
chain_error_len: (chainId: number) => number,
Expand Down
8 changes: 4 additions & 4 deletions wasm-node/javascript/src/internals/remote-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ export async function connectToInstanceServer(config: ConnectConfig): Promise<in
};

return {
async addChain(chainSpec, databaseContent, potentialRelayChains, disableJsonRpc) {
const msg: ClientToServer = { ty: "add-chain", chainSpec, databaseContent, potentialRelayChains, disableJsonRpc };
async addChain(chainSpec, databaseContent, potentialRelayChains, disableJsonRpc, jsonRpcMaxPendingRequests, jsonRpcMaxSubscriptions) {
const msg: ClientToServer = { ty: "add-chain", chainSpec, databaseContent, potentialRelayChains, disableJsonRpc, jsonRpcMaxPendingRequests, jsonRpcMaxSubscriptions };
portToServer.postMessage(msg);
},

Expand Down Expand Up @@ -341,7 +341,7 @@ export async function startInstanceServer(config: ServerConfig, initPortToClient

switch (message.ty) {
case "add-chain": {
state.instance!.addChain(message.chainSpec, message.databaseContent, message.potentialRelayChains, message.disableJsonRpc);
state.instance!.addChain(message.chainSpec, message.databaseContent, message.potentialRelayChains, message.disableJsonRpc, message.jsonRpcMaxPendingRequests, message.jsonRpcMaxSubscriptions);
break;
}
case "remove-chain": {
Expand Down Expand Up @@ -442,7 +442,7 @@ type ServerToClient = Exclude<instance.Event, { ty: "json-rpc-responses-non-empt
{ ty: "json-rpc-response", chainId: number, response: string };

type ClientToServer =
{ ty: "add-chain", chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean } |
{ ty: "add-chain", chainSpec: string, databaseContent: string, potentialRelayChains: number[], disableJsonRpc: boolean, jsonRpcMaxPendingRequests: number, jsonRpcMaxSubscriptions: number } |
{ ty: "remove-chain", chainId: number } |
{ ty: "request", chainId: number, request: string } |
{ ty: "accept-more-json-rpc-answers", chainId: number } |
Expand Down
23 changes: 23 additions & 0 deletions wasm-node/javascript/src/public-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,4 +423,27 @@ export interface AddChainOptions {
* this chain.
*/
disableJsonRpc?: boolean,

/**
* Maximum number of JSON-RPC requests that haven't been answered yet. Any further request will
* be rejected.
*
* This field is ignored if {@link AddChainOptions.disableJsonRpc} is `true`.
*
* A zero, negative or NaN value is invalid.
*
* If this value is not set, it means that there is no maximum.
*/
jsonRpcMaxPendingRequests?: number,

/**
* Maximum number of active JSON-RPC subscriptions. Any further subscription will be rejected.
*
* This field is ignored if {@link AddChainOptions.disableJsonRpc} is `true`.
*
* A negative or NaN value is invalid.
*
* If this value is not set, it means that there is no maximum.
*/
jsonRpcMaxSubscriptions?: number
}
15 changes: 11 additions & 4 deletions wasm-node/rust/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,13 @@ pub extern "C" fn advance_execution() {
/// ids. If the chain specification refer to a parachain, these chain ids are the ones that will be
/// looked up to find the corresponding relay chain.
///
/// If `json_rpc_running` is 0, then no JSON-RPC service will be started and it is forbidden to
/// send JSON-RPC requests targeting this chain. This can be used to save up resources.
/// `json_rpc_max_pending_requests` indicates the size of the queue of JSON-RPC requests that
/// haven't been answered yet.
/// If `json_rpc_max_pending_requests` is 0, then no JSON-RPC service will be started and it is
/// forbidden to send JSON-RPC requests targeting this chain. This can be used to save up
/// resources.
/// If `json_rpc_max_pending_requests` is 0, then the value of `json_rpc_max_subscriptions` is
/// ignored.
///
/// If an error happens during the creation of the chain, a chain id will be allocated
/// nonetheless, and must later be de-allocated by calling [`remove_chain`]. This allocated chain,
Expand All @@ -329,13 +334,15 @@ pub extern "C" fn advance_execution() {
pub extern "C" fn add_chain(
chain_spec_buffer_index: u32,
database_content_buffer_index: u32,
json_rpc_running: u32,
json_rpc_max_pending_requests: u32,
json_rpc_max_subscriptions: u32,
potential_relay_chains_buffer_index: u32,
) -> u32 {
super::add_chain(
get_buffer(chain_spec_buffer_index),
get_buffer(database_content_buffer_index),
json_rpc_running,
json_rpc_max_pending_requests,
json_rpc_max_subscriptions,
get_buffer(potential_relay_chains_buffer_index),
)
}
Expand Down
15 changes: 9 additions & 6 deletions wasm-node/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ fn init(max_log_level: u32) {
fn add_chain(
chain_spec: Vec<u8>,
database_content: Vec<u8>,
json_rpc_running: u32,
json_rpc_max_pending_requests: u32,
json_rpc_max_subscriptions: u32,
potential_relay_chains: Vec<u8>,
) -> u32 {
let mut client_lock = CLIENT.lock().unwrap();
Expand Down Expand Up @@ -97,14 +98,16 @@ fn add_chain(
.unwrap_or_else(|_| panic!("non-utf8 chain spec")),
database_content: str::from_utf8(&database_content)
.unwrap_or_else(|_| panic!("non-utf8 database content")),
json_rpc: if json_rpc_running == 0 {
smoldot_light::AddChainConfigJsonRpc::Disabled
} else {
json_rpc: if let Some(json_rpc_max_pending_requests) =
NonZeroU32::new(json_rpc_max_pending_requests)
{
smoldot_light::AddChainConfigJsonRpc::Enabled {
max_pending_requests: NonZeroU32::new(128).unwrap(),
max_pending_requests: json_rpc_max_pending_requests,
// Note: the PolkadotJS UI is very heavy in terms of subscriptions.
max_subscriptions: 1024,
max_subscriptions: json_rpc_max_subscriptions,
}
} else {
smoldot_light::AddChainConfigJsonRpc::Disabled
},
potential_relay_chains: potential_relay_chains.into_iter(),
}) {
Expand Down

0 comments on commit cf21110

Please sign in to comment.