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

fix: recover from smoldot crashes #2162

Merged
merged 18 commits into from
May 22, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// plugins: [pluginNodeResolve(), pluginEsbuild()],
// }

import { start } from "smoldot"
import { start } from "@substrate/light-client-extension-helpers/smoldot"

const clientReferencesMaxLogLevel = 3

Expand Down
4 changes: 2 additions & 2 deletions packages/light-client-extension-helpers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
"./known-chain-specs/ksmcc3": "./dist/known-chain-specs/ksmcc3.mjs",
"./known-chain-specs/westend2": "./dist/known-chain-specs/westend2.mjs",
"./known-chain-specs/rococo_v2_2": "./dist/known-chain-specs/rococo_v2_2.mjs",
"./smoldot": "./dist/smoldot/smoldot.mjs"
"./smoldot": "./dist/smoldot/index.mjs"
},
"files": [
"dist"
],
"scripts": {
"build": "tsc --noEmit && tsup-node src/background/background-helper.ts src/content-script/content-script-helper.ts src/extension-page/extension-page-helper.ts src/web-page/web-page-helper.ts src/known-chain-specs/index.ts src/known-chain-specs/polkadot.ts src/known-chain-specs/ksmcc3.ts src/known-chain-specs/westend2.ts src/known-chain-specs/rococo_v2_2.ts src/utils/index.ts src/smoldot/smoldot.ts --clean --sourcemap --platform neutral --target=es2020 --format esm --dts",
"build": "tsc --noEmit && tsup-node src/background/background-helper.ts src/content-script/content-script-helper.ts src/extension-page/extension-page-helper.ts src/web-page/web-page-helper.ts src/known-chain-specs/index.ts src/known-chain-specs/polkadot.ts src/known-chain-specs/ksmcc3.ts src/known-chain-specs/westend2.ts src/known-chain-specs/rococo_v2_2.ts src/utils/index.ts src/smoldot/index.ts --clean --sourcemap --platform neutral --target=es2020 --format esm --dts",
"dev": "pnpm build --watch",
"lint": "prettier --check README.md \"src/**/*.{js,jsx,ts,tsx,json,md}\""
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
} from "@polkadot-api/substrate-client"
import { getObservableClient } from "@polkadot-api/observable-client"
import { getSyncProvider } from "@polkadot-api/json-rpc-provider-proxy"
import type { AddChainOptions, Chain, Client } from "smoldot"
import {
Observable,
firstValueFrom,
Expand All @@ -24,6 +23,7 @@ import { ALARM, PORT, isSubstrateConnectToExtensionMessage } from "@/shared"
import { isRpcMessage, type RpcMessage } from "@/utils"
import * as storage from "@/storage"
import { createBackgroundRpc } from "./createBackgroundRpc"
import { Client, Chain, AddChainOptions, supervise } from "../smoldot"

export type * from "./types"

Expand All @@ -40,6 +40,8 @@ export const register = ({
if (isRegistered) throw new Error("helper already registered")
isRegistered = true

supervise(smoldotClient, { onError: console.error })

const wellKnownChainSpecsPromise: Promise<Record<string, string>> =
getWellKnownChainSpecs().then(async (chainSpecs) =>
chainSpecs instanceof Array
Expand Down Expand Up @@ -375,15 +377,15 @@ export const register = ({
disableJsonRpc: false,
potentialRelayChains: chain.relayChainGenesisHash
? [
await smoldotClient.addChain({
{
chainSpec:
chains[chain.relayChainGenesisHash].chainSpec,
disableJsonRpc: true,
databaseContent: await storage.get({
type: "databaseContent",
genesisHash: chain.relayChainGenesisHash,
}),
}),
},
]
: [],
databaseContent: await storage.get({
Expand All @@ -399,19 +401,22 @@ export const register = ({
disableJsonRpc: false,
potentialRelayChains: chains[relayChainGenesisHashOrChainId]
? [
await smoldotClient.addChain({
{
chainSpec:
chains[relayChainGenesisHashOrChainId].chainSpec,
disableJsonRpc: true,
databaseContent: await storage.get({
type: "databaseContent",
genesisHash: relayChainGenesisHashOrChainId,
}),
}),
},
]
: msg.potentialRelayChainIds
.filter((chainId) => activeChains[tabId][chainId])
.map((chainId) => activeChains[tabId][chainId].chain),
.map((chainId) => ({
chainSpec: activeChains[tabId][chainId].chainSpec,
disableJsonRpc: true,
})),
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import type * as smoldot from "../smoldot"

import type { JsonRpcConnection } from "@polkadot-api/json-rpc-provider"
import { getSyncProvider } from "@polkadot-api/json-rpc-provider-proxy"

export type Client = Pick<smoldot.Client, "addChain">
export type AddChainOptions = smoldot.AddChainOptions

/**
* Adds a chain to check if the add chain options are valid and then
* subsequently removes it, so that it is not included in smoldot's internal
* reference count.
*
*/
const validateAddChainOptions = async (
client: Client,
options: AddChainOptions,
) => {
const chain = await client.addChain(options)
try {
chain.remove()
} catch {}
}

/**
* Creates a new JSON RPC provider using a client and options.
*
* This provider will automatically re-connect to the chain if the
* connection is lost.
ryanleecode marked this conversation as resolved.
Show resolved Hide resolved
*
* @param client - The client to use for creating the provider.
* @param options - The options for adding a chain.
*/
export const make = async (client: Client, options: AddChainOptions) => {
// The chain that is added in this function is removed only to be added
// again when the provider is created. This doesn't matter from a performance
// perspective, and is done to keep things simple rather than keep a separate
// variable to track whether it is the first time the chain is being added.
await validateAddChainOptions(client, options)

const provider = getSyncProvider(async () => {
const chain = await client.addChain(options)

return (onMessage, onError) => {
let connected = true
;(async () => {
while (connected) {
try {
const message = await chain.nextJsonRpcResponse()
onMessage(message)
} catch {
connected = false
try {
onError()
} catch {}
break
}
}
})()

const send: JsonRpcConnection["send"] = (message) => {
try {
chain.sendJsonRpc(message)
} catch {}
}

const disconnect: JsonRpcConnection["disconnect"] = () => {
connected = false
try {
chain.remove()
} catch {}
}

return {
send,
disconnect,
}
}
})

return provider
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { JsonRpcProvider } from "@polkadot-api/json-rpc-provider"
import type { AddChainOptions, Client } from "smoldot"
import type { Client, AddChainOptions } from "../smoldot"
import { make as makeJsonRpcProvider } from "./json-rpc-provider"

type SmoldotProviderOptions =
| { smoldotClient: Client; addChainOptions: AddChainOptions }
Expand All @@ -15,62 +16,25 @@ export const smoldotProvider = async ({
smoldotClient,
...options
}: SmoldotProviderOptions): Promise<JsonRpcProvider> => {
const chain = await smoldotClient.addChain(
const provider = await makeJsonRpcProvider(
smoldotClient,
"addChainOptions" in options
? options.addChainOptions
: {
chainSpec: options.chainSpec,
disableJsonRpc: false,
potentialRelayChains: options.relayChainSpec
? [
await smoldotClient.addChain({
{
chainSpec: options.relayChainSpec,
disableJsonRpc: true,
databaseContent: options.relayChainDatabaseContent,
}),
},
]
: [],
databaseContent: options.databaseContent,
},
)
return (onMessage) => {
let initialized = false
return {
disconnect() {
try {
chain.remove()
} catch (error) {
console.error("error removing chain", error)
}
},
send(msg: string) {
if (!initialized) {
initialized = true
;(async () => {
while (true) {
let jsonRpcResponse
try {
jsonRpcResponse = await chain.nextJsonRpcResponse()
} catch (_) {
break
}

// `nextJsonRpcResponse` throws an exception if we pass `disableJsonRpc: true` in the
// config. We pass `disableJsonRpc: true` if `jsonRpcCallback` is undefined. Therefore,
// this code is never reachable if `jsonRpcCallback` is undefined.
try {
onMessage(jsonRpcResponse)
} catch (error) {
console.error(
"JSON-RPC callback has thrown an exception:",
error,
)
}
}
})()
}
chain.sendJsonRpc(msg)
},
}
}
return provider
}
62 changes: 62 additions & 0 deletions packages/light-client-extension-helpers/src/smoldot/client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import * as smoldot from "smoldot"
import { AddChainOptions, Client, ClientOptions } from "./types"

/**
* Wraps a smoldot client to add restart functionality.
*
* The key difference from the standard smoldot client is how relay chains are
* specified. Instead of using `Chain` objects for `potentialRelayChains`,
* we use `AddChainOptions`. This is necessary because the `Chain` objects
* become invalid after a restart and can't be used in a replay `addChain` call.
*
* With `AddChainOptions`, we can easily re-add a parachain after a restart by
* reusing the same options for each relay chain. Before adding the parachain,
* we add the relay chains using their `AddChainOptions` and then remove them
* after the parachain is created.
*
* This ensures the parachain can always be added successfully, as its relay
* chain is always added first.
*/
export const start = (options?: ClientOptions): Client => {
let client = smoldot.start(options)
let terminated = false

const addChain = async (options: AddChainOptions) => {
const potentialRelayChains = await Promise.all(
options.potentialRelayChains?.map((options) =>
client.addChain(options),
) ?? [],
)

const newChain = await client.addChain({
...options,
potentialRelayChains,
})

await Promise.all(potentialRelayChains.map((chain) => chain.remove()))

return newChain
}

const terminate = () => {
terminated = true
return client.terminate()
}

const restart = async () => {
if (terminated) {
throw new Error("Cannot restart a terminated client")
}

try {
await client.terminate()
} catch {}
client = smoldot.start(options)
}

return {
addChain,
restart,
terminate,
}
}
3 changes: 3 additions & 0 deletions packages/light-client-extension-helpers/src/smoldot/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./client"
export * from "./types"
export * from "./tasks"
Comment on lines +1 to +3
Copy link
Contributor

@jsdw jsdw May 20, 2024

Choose a reason for hiding this comment

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

I know that this pattern exists throughout the repo, but IMO I think it'd be better practice to be explicit about the exports from a package rather than use * exports (just so the library interface is obvious just looking at this root file and not needing to traverse other stuff :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah but I find its really easy to forget to add a new type that should get re-exported and then it gets missed during code review because people don't really pay attention to that.

For anything that should be kept 100% internal I like to use the @internal jsdoc tag and then with stripeInternal set true in your tsconfig, these definitions won't be included in the d.ts`

https://www.typescriptlang.org/tsconfig/#stripInternal

Copy link
Contributor

@jsdw jsdw May 21, 2024

Choose a reason for hiding this comment

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

Yup totally true; we forget to export types in Rust a fair bit, but personally I much prefer to forget exporting a type than to accidentally export a type I didn't mean to; it's easier to go one way than the other! But mainly it just makes it super easy at a glance to see what's being provided by the package.

TIL about @internal; that's cool to know about! Ultimately I'm not super fussed about this so happy to stick with the *'s + @internal's if you prefer that way around!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Gonna opt for the YOLO approach and just use @internal.

But if you want to actually see what the publicly export types are u can run corepack pnpm turbo build then open the dist folder and each of the the subpath exports will have a d.ts file in it showing the public api.

image

This file was deleted.

Loading
Loading