Skip to content
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.

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

Add new fields to AddChainOptions to configure JSON-RPC limits #694

Merged
merged 5 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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